On 09.12.2020 10:53, Julien Grall wrote: > On 03/12/2020 09:46, Jan Beulich wrote: >> On 02.12.2020 20:03, Julien Grall wrote: >>> On 23/11/2020 13:28, Jan Beulich wrote: >>>> The per-vCPU virq_lock, which is being held anyway, together with there >>>> not being any call to evtchn_port_set_pending() when v->virq_to_evtchn[] >>>> is zero, provide sufficient guarantees. >>> >>> I agree that the per-vCPU virq_lock is going to be sufficient, however >>> dropping the lock also means the event channel locking is more complex >>> to understand (the long comment that was added proves it). >>> >>> In fact, the locking in the event channel code was already proven to be >>> quite fragile, therefore I think this patch is not worth the risk. >> >> I agree this is a very reasonable position to take. I probably >> would even have remained silent if in the meantime the >> spin_lock()s there hadn't changed to read_trylock()s. I really >> think we want to limit this unusual locking model to where we >> strictly need it. > > While I appreciate that the current locking is unusual, we should follow > the same model everywhere rather than having a dozen of way to lock the > same structure. > > The rationale is quite simple, if you have one way to lock a structure, > then there are less chance to screw up.
If only this all was consistent prior to this change. It's not, and hence I don't see how things get so much more unusual than it was before. In fact one "unusual" (the trylock) gets treated for another one (the specific lock protecting the sending of VIRQ events). Jan
