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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <968e4c62-4486-a6aa-8fdf-67ff9b05a330@colorfullife.com>
Date:   Mon, 29 Aug 2016 14:54:54 +0200
From:   Manfred Spraul <manfred@...orfullife.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     benh@...nel.crashing.org, paulmck@...ux.vnet.ibm.com,
        Ingo Molnar <mingo@...e.hu>, Boqun Feng <boqun.feng@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>, 1vier1@....de,
        Davidlohr Bueso <dave@...olabs.net>
Subject: Re: [PATCH 1/4] spinlock: Document memory barrier rules

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