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

Manfred Spraul
2016-08-29T12:55:11Z

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