Re: [PATCH v6 2/3] xen/evtchn: rework per event channel lock

[Julien Grall <julien@xxxxxxx>]

Hi,

On 09/11/2020 16:48, Jan Beulich wrote:
> On 09.11.2020 17:38, Juergen Gross wrote:
> > Currently the lock for a single event channel needs to be taken with
> > interrupts off, which causes deadlocks in some cases.
> >
> > Rework the per event channel lock to be non-blocking for the case of
> > sending an event and removing the need for disabling interrupts for
> > taking the lock.
> >
> > The lock is needed for avoiding races between event channel state
> > changes (creation, closing, binding) against normal operations (set
> > pending, [un]masking, priority changes).
> >
> > Use a rwlock, but with some restrictions:
> >
> > - Changing the state of an event channel (creation, closing, binding)
> >    needs to use write_lock(), with ASSERT()ing that the lock is taken as
> >    writer only when the state of the event channel is either before or
> >    after the locked region appropriate (either free or unbound).
> >
> > - Sending an event needs to use read_trylock() mostly, in case of not
> >    obtaining the lock the operation is omitted. This is needed as
> >    sending an event can happen with interrupts off (at least in some
> >    cases).
> >
> > - Dumping the event channel state for debug purposes is using
> >    read_trylock(), too, in order to avoid blocking in case the lock is
> >    taken as writer for a long time.
> >
> > - All other cases can use read_lock().
> >
> > Fixes: e045199c7c9c54 ("evtchn: address races with evtchn_reset()")
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> > V4:
> > - switch to rwlock
> > - add ASSERT() to verify correct write_lock() usage
> >
> > V3:
> > - corrected a copy-and-paste error (Jan Beulich)
> > - corrected unlocking in two cases (Jan Beulich)
> > - renamed evtchn_read_trylock() (Jan Beulich)
> > - added some comments and an ASSERT() for evtchn_write_lock()
> > - set EVENT_WRITE_LOCK_INC to INT_MIN
> >
> > V2:
> > - added needed barriers
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> I'll give it overnight for others to possibly comment, but I'd
> like to get this in tomorrow if at all possible.
IIRC, Citrix originally reported the issues. I would like to give them 
an opportunity to test the patches and confirm this effectively fixed 
there issues.
@Roger, @Andrew, could you give a try to confirm this unblock the vm 
event issue?
Cheers,

--
Julien Grall