[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 03/13] vpci: move lock outside of struct vpci
On 07.02.2022 16:11, Oleksandr Andrushchenko wrote: > > > On 07.02.22 16:35, Oleksandr Andrushchenko wrote: >> >> On 07.02.22 16:27, Roger Pau Monné wrote: >>> On Mon, Feb 07, 2022 at 03:11:03PM +0100, Jan Beulich wrote: >>>> On 07.02.2022 14:53, Oleksandr Andrushchenko wrote: >>>>> On 07.02.22 14:46, Roger Pau Monné wrote: >>>>>> I think the per-domain rwlock seems like a good option. I would do >>>>>> that as a pre-patch. >>>>> It is. But it seems it won't solve the thing we started this adventure >>>>> for: >>>>> >>>>> With per-domain read lock and still ABBA in modify_bars (hope the below >>>>> is correctly seen with a monospace font): >>>>> >>>>> cpu0: vpci_write-> d->RLock -> pdev1->lock -> >>>>> rom_write -> modify_bars: tmp (pdev2) ->lock >>>>> cpu1: vpci_write-> d->RLock pdev2->lock -> cmd_write -> >>>>> modify_bars: tmp (pdev1) ->lock >>>>> >>>>> There is no API to upgrade read lock to write lock in modify_bars which >>>>> could help, >>>>> so in both cases vpci_write should take write lock. >>>> Hmm, yes, I think you're right: It's not modify_bars() itself which needs >>>> to acquire the write lock, but its (perhaps indirect) caller. Effectively >>>> vpci_write() would need to take the write lock if the range written >>>> overlaps the BARs or the command register. >>> I'm confused. If we use a per-domain rwlock approach there would be no >>> need to lock tmp again in modify_bars, because we should hold the >>> rwlock in write mode, so there's no ABBA? >> this is only possible with what you wrote below: >>> We will have however to drop the per domain read and vpci locks and >>> pick the per-domain lock in write mode. >> I think this is going to be unreliable. We need a reliable way to >> upgrade read lock to write lock. >> Then, we can drop pdev->vpci_lock at all, because we are always >> protected with d->rwlock and those who want to free pdev->vpci >> will use write lock. >> >> So, per-domain rwlock with write upgrade implemented minus pdev->vpci >> should do the trick > Linux doesn't implement write upgrade and it seems for a reason [1]: > "Also, you cannot “upgrade” a read-lock to a write-lock, so if you at _any_ > time > need to do any changes (even if you don’t do it every time), you have to get > the write-lock at the very beginning." > > So, I am not sure we can have the same for Xen... > > At the moment I see at least two possible ways to solve the issue: > 1. Make vpci_write use write lock, thus make all write accesses synchronized > for the given domain, read are fully parallel 1b. Make vpci_write use write lock for writes to command register and BARs only; keep using the read lock for all other writes. Jan > 2. Re-implement pdev/tmp overlapping detection with something which won't > require pdev->vpci_lock/tmp->vpci_lock > > 3. Drop read and acquire write lock in modify_bars... but this is not reliable > and will hide a free(pdev->vpci) bug > > @Roger, @Jan: Any other suggestions? > > Thank you, > Oleksandr > > [1] > https://www.kernel.org/doc/html/latest/locking/spinlocks.html#lesson-2-reader-writer-spinlocks
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |