Re: [Xen-devel] [PATCH 2/2] xen/spinlocks: fix placement of preempt_[dis|en]able()

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

On 13.03.20 09:55, Jan Beulich wrote:
> On 13.03.2020 09:05, Juergen Gross wrote:
> > @@ -199,10 +199,10 @@ unsigned long _spin_lock_irqsave(spinlock_t *lock)
> >   void _spin_unlock(spinlock_t *lock)
> >   {
> >       arch_lock_release_barrier();
> > -    preempt_enable();
> >       LOCK_PROFILE_REL;
> >       rel_lock(&lock->debug);
> >       add_sized(&lock->tickets.head, 1);
> > +    preempt_enable();
> >       arch_lock_signal();
> >   }
> arch_lock_signal() is a barrier on Arm, hence just like for patch 1
> I wonder whether the insertion wouldn't better come after it.
Either way is fine for me. It should be noted that preemption is only
relevant on the local cpu. So this is about trading lock state
visibility against preemption disabled time, and I agree the visible
time of the lock held should be minimized at higher priority than the
preemption disabled time.

I'll modify my patches accordingly, adding a note in this regard to
the commit message.


Juergen

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