Re: [PATCH 1/4] spinlock: Document memory barrier rules

[Manfred Spraul]

Hi Peter,

On 08/29/2016 12:48 PM, Peter Zijlstra wrote:
> On Sun, Aug 28, 2016 at 01:56:13PM +0200, Manfred Spraul wrote:
> > Right now, the spinlock machinery tries to guarantee barriers even for
> > unorthodox locking cases, which ends up as a constant stream of updates
> > as the architectures try to support new unorthodox ideas.
> >
> > The patch proposes to reverse that:
> > spin_lock is ACQUIRE, spin_unlock is RELEASE.
> > spin_unlock_wait is also ACQUIRE.
> > Code that needs further guarantees must use appropriate explicit barriers.
> >
> > Architectures that can implement some barriers for free can define the
> > barriers as NOPs.
> >
> > As the initial step, the patch converts ipc/sem.c to the new defines:
> > - no more smp_rmb() after spin_unlock_wait(), that is part of
> >    spin_unlock_wait()
> > - smp_mb__after_spin_lock() instead of a direct smp_mb().
> Why? This does not explain why..

Which explanation is missing?

- removal of the smb_rmb() after spin_unlock_wait?

What about:

> - With commit 2c6100227116
>   ("locking/qspinlock: Fix spin_unlock_wait() some more"),
>   (and the commits for the other archs), spin_unlock_wait() is an
>   ACQUIRE.
>   Therefore the smp_rmb() after spin_unlock_wait() can be removed.
> - smp_mb__after_spin_lock() instead of a direct smp_mb().
>   This allows that architectures override it with a less expensive
>   barrier if this is sufficient for their hardware.
- Why smp_mb is required after spin_lock? See Patch 02, I added the race 
that exists on real hardware.

Exactly the same issue exists for sem.c

- Why introduce a smp_mb__after_spin_lock()?

The other options would be:
- same as RCU, i.e. add CONFIG_PPC into sem.c and nf_contrack_core.c
- overhead for all archs by added an unconditional smp_mb()

--
    Manfred