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

[Jürgen Groß <jgross@xxxxxxxx>]

On 09.11.20 18:46, Julien Grall wrote:
> 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?
It should be noted that this series ought to fix current unstable
failures of XSM tests, as those are triggering the advanced lock checks
of rwlocks due to the per-event channel lock disabling interrupts.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature