Re: sem_lock() vs qspinlocks

[Boqun Feng]

On Mon, May 23, 2016 at 10:52:09AM -0700, Linus Torvalds wrote:
> On Mon, May 23, 2016 at 5:25 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > Paul has smp_mb__after_unlock_lock() for the RCpc 'upgrade'. How about
> > something like:
> >
> >         smp_mb__after_lock()
> 
> I'd much rather make the naming be higher level. It's not necessarily

Speak of higher level, I realize that problem here is similar to the
problem we discussed last year:

http://lkml.kernel.org/r/20151112070915.GC6314@xxxxxxxxxxxxxxxxxxxxxxx

the problem here is about synchronization between two spinlocks and that
problem is about synchronization between a spinlock and ordinary
variables.

(One result of this similarity is that qspinlock on x86 may be also
broken in the do_exit() code as spinlocks on AARCH64 and PPC. Because
a variable LOAD inside a qspinlock critical section could be reordered
before the STORE part of a qspinlock acquisition.)


For the problem we found last year, the current solution for AARCH64 and
PPC is to have a little heavy weight spin_unlock_wait() to pair with
spin_lock():

AARCH64: http://lkml.kernel.org/r/1448624646-15863-1-git-send-email-will.deacon@xxxxxxx
PPC: http://lkml.kernel.org/r/1461130033-70898-1-git-send-email-boqun.feng@xxxxxxxxx (not merged yet)

Another solution works on PPC is what Paul Mckenney suggested, using
smp_mb__after_unlock_lock():

http://lkml.kernel.org/r/20151112144004.GU3972@xxxxxxxxxxxxxxxxxx

, which is petty much the same as the spinlock synchronization
primitive we are discussing about here.


So I'm thinking, if we are going to introduce some primitives for
synchronizing two spinlocks (or even a spinlock and a mutex) anyway,
could we be a little more higher level, to reuse/invent primitives to
solve the synchronzing problem we have between
spinlocks(spin_unlock_wait()) and normal variables?

One benefit of this is that we could drop the complex implementations of
spin_unlock_wait() on AARCH64 and PPC.

Thoughts?

Regards,
Boqun

> going to be a "mb", and while the problem is about smp, the primitives
> it is synchronizing aren't actually smp-specific (ie you're
> synchronizing a lock that is relevant on UP too).
> 
> So I'd just call it something like
> 
>         spin_lock_sync_after_lock();
> 
> because different locks might have different levels of serialization
> (ie maybe a spinlock needs one thing, and a mutex needs another - if
> we start worrying about ordering between spin_lock and
> mutex_is_locked(), for example, or between mutex_lock() and
> spin_is_locked()).
> 
> Hmm?
> 
>                      Linus

Attachment: signature.asc
Description: PGP signature