On 07.02.2022 17:08, Roger Pau Monné wrote: > On Mon, Feb 07, 2022 at 04:26:56PM +0100, Jan Beulich wrote: >> 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. > > We do not support writing to the BARs with memory decoding enabled > currently for dom0, so we would only need to pick the lock in write > mode for the command register and ROM BAR write handler AFAICT.
Oh, right - this then makes for even less contention due to needing to acquire the lock in write mode. Jan
