linux-kernel - Re: Behaviour of smp_mb__{before,after}

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]

Date:	Wed, 14 Jan 2015 11:31:47 +0000
From:	Will Deacon <will.deacon@....com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	"paulmck@...ux.vnet.ibm.com" <paulmck@...ux.vnet.ibm.com>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
	"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>
Subject: Re: Behaviour of smp_mb__{before,after}_spin* and acquire/release

Hi Oleg,

On Tue, Jan 13, 2015 at 06:45:10PM +0000, Oleg Nesterov wrote:
> On 01/13, Will Deacon wrote:
> >
> >   1. Does smp_mb__before_spinlock actually have to order prior loads
> >      against later loads and stores? Documentation/memory-barriers.txt
> >      says it does, but that doesn't match the comment
> 
> The comment says that smp_mb__before_spinlock() + spin_lock() should
> only serialize STOREs with LOADs. This is because it was added to ensure
> that the setting of condition can't race with ->state check in ttwu().

Yup, that makes sense. The comment is consistent with the code, and I think
the code is doing what it's supposed to do.

> But since we use wmb() it obviously serializes STOREs with STORES. I do
> not know if this should be documented, but we already have another user
> which seems to rely on this fact: set_tlb_flush_pending().

In which case, it's probably a good idea to document that too.

> As for "prior loads", this doesn't look true...

Agreed. I'd propose something like the diff below, but it also depends on
my second question since none of this is true for smp_load_acquire.

Will

--->8

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 70a09f8a0383..9c0e3c45a807 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1724,10 +1724,9 @@ for each construct.  These operations all imply certain barriers:
 
      Memory operations issued before the ACQUIRE may be completed after
      the ACQUIRE operation has completed.  An smp_mb__before_spinlock(),
-     combined with a following ACQUIRE, orders prior loads against
-     subsequent loads and stores and also orders prior stores against
-     subsequent stores.  Note that this is weaker than smp_mb()!  The
-     smp_mb__before_spinlock() primitive is free on many architectures.
+     combined with a following ACQUIRE, orders prior stores against
+     subsequent loads and stores. Note that this is weaker than smp_mb()!
+     The smp_mb__before_spinlock() primitive is free on many architectures.
 
  (2) RELEASE operation implication:
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/