Re: [PATCH v3 1/5] evtchn: drop acquiring of per-channel lock from send_guest_{global,vcpu}_virq()

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

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.
The only reason I would be willing to diverge from statement is if the 
performance are significantly improved.
Cheers,

--
Julien Grall