Re: [Xen-devel] [PATCH 1/2] xen/rwlocks: call preempt_disable() when taking a rwlock

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

On 13.03.20 11:02, Julien Grall wrote:
> Hi Juergen,
>
> On 13/03/2020 08:05, Juergen Gross wrote:
> > Similar to spinlocks preemption should be disabled while holding a
> > rwlock.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   xen/include/xen/rwlock.h | 18 +++++++++++++++++-
> >   1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/include/xen/rwlock.h b/xen/include/xen/rwlock.h
> > index 1c221dd0d9..4ee341a182 100644
> > --- a/xen/include/xen/rwlock.h
> > +++ b/xen/include/xen/rwlock.h
> > @@ -2,6 +2,7 @@
> >   #define __RWLOCK_H__
> >   #include <xen/percpu.h>
> > +#include <xen/preempt.h>
> >   #include <xen/smp.h>
> >   #include <xen/spinlock.h>
> > @@ -57,10 +58,12 @@ static inline int _read_trylock(rwlock_t *lock)
> >       cnts = atomic_read(&lock->cnts);
> >       if ( likely(_can_read_lock(cnts)) )
> >       {
> If you get preempted here, then it means the check below is likely going 
> to fail. So I think it would be best to disable preemption before, to 
> give more chance to succeed.
As preemption probability at this very point should be much lower than
that of held locks I think that is optimizing the wrong path. I'm not
opposed doing the modification you are requesting, but would like to
hear a second opinion on that topic, especially as I'd need to add
another preempt_enable() call when following your advice.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel