linux-kernel - Re: [RFC][PATCH 1/3] locking: Introduce smp_acquire__after_ctrl

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, 25 May 2016 07:29:35 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Boqun Feng <boqun.feng@...il.com>
Cc:	Waiman Long <waiman.long@....com>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	manfred@...orfullife.com, dave@...olabs.net, will.deacon@....com,
	tj@...nel.org, pablo@...filter.org, kaber@...sh.net,
	davem@...emloft.net, oleg@...hat.com,
	netfilter-devel@...r.kernel.org, sasha.levin@...cle.com,
	hofrat@...dl.org
Subject: Re: [RFC][PATCH 1/3] locking: Introduce smp_acquire__after_ctrl_dep

On Wed, May 25, 2016 at 01:39:30PM +0800, Boqun Feng wrote:
> On Tue, May 24, 2016 at 09:53:29PM -0700, Paul E. McKenney wrote:
> > On Tue, May 24, 2016 at 11:01:21PM -0400, Waiman Long wrote:
> > > On 05/24/2016 10:27 AM, Peter Zijlstra wrote:
> > > >Introduce smp_acquire__after_ctrl_dep(), this construct is not
> > > >uncommen, but the lack of this barrier is.
> > > >
> > > >Signed-off-by: Peter Zijlstra (Intel)<peterz@...radead.org>
> > > >---
> > > >  include/linux/compiler.h |   14 ++++++++++----
> > > >  ipc/sem.c                |   14 ++------------
> > > >  2 files changed, 12 insertions(+), 16 deletions(-)
> > > >
> > > >--- a/include/linux/compiler.h
> > > >+++ b/include/linux/compiler.h
> > > >@@ -305,20 +305,26 @@ static __always_inline void __write_once
> > > >  })
> > > >
> > > >  /**
> > > >+ * smp_acquire__after_ctrl_dep() - Provide ACQUIRE ordering after a control dependency
> > > >+ *
> > > >+ * A control dependency provides a LOAD->STORE order, the additional RMB
> > > >+ * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
> > > >+ * aka. ACQUIRE.
> > > >+ */
> > > >+#define smp_acquire__after_ctrl_dep()		smp_rmb()
> > > >+
> > > >+/**
> > > >   * smp_cond_acquire() - Spin wait for cond with ACQUIRE ordering
> > > >   * @cond: boolean expression to wait for
> > > >   *
> > > >   * Equivalent to using smp_load_acquire() on the condition variable but employs
> > > >   * the control dependency of the wait to reduce the barrier on many platforms.
> > > >   *
> > > >- * The control dependency provides a LOAD->STORE order, the additional RMB
> > > >- * provides LOAD->LOAD order, together they provide LOAD->{LOAD,STORE} order,
> > > >- * aka. ACQUIRE.
> > > >   */
> > > >  #define smp_cond_acquire(cond)	do {		\
> > > >  	while (!(cond))				\
> > > >  		cpu_relax();			\
> > > >-	smp_rmb(); /* ctrl + rmb := acquire */	\
> > > >+	smp_acquire__after_ctrl_dep();		\
> > > >  } while (0)
> > > >
> > > >
> > > 
> > > I have a question about the claim that control dependence + rmb is
> > > equivalent to an acquire memory barrier. For example,
> > > 
> > > S1:    if (a)
> > > S2:       b = 1;
> > >        smp_rmb()
> > > S3:    c = 2;
> > > 
> > > Since c is independent of both a and b, is it possible that the cpu
> > > may reorder to execute store statement S3 first before S1 and S2?
> > 
> > The CPUs I know of won't do, nor should the compiler, at least assuming
> > "a" (AKA "cond") includes READ_ONCE().  Ditto "b" and WRITE_ONCE().
> > Otherwise, the compiler could do quite a few "interesting" things,
> > especially if it knows the value of "b".  For example, if the compiler
> > knows that b==1, without the volatile casts, the compiler could just
> > throw away both S1 and S2, eliminating any ordering.  This can get
> > quite tricky -- see memory-barriers.txt for more mischief.
> > 
> > The smp_rmb() is not needed in this example because S3 is a write, not
> 
> but S3 needs to be an WRITE_ONCE(), right? IOW, the following code can
> result in reordering:
> 
> S1:	if (READ_ONCE(a))
> S2:		WRITE_ONCE(b, 1);
> 	
> S3:	c = 2; // this can be reordered before READ_ONCE(a)
> 
> but if we change S3 to WRITE_ONCE(c, 2), the reordering can not happen
> for the CPUs you are aware of, right?

Yes, if you remove the smp_rmb(), you also need a WRITE_ONCE() for S3.

Even with the smp_rmb(), you have to be careful.

In general, if you don't tell the compiler otherwise, it is within its
rights to assume that nothing else is reading from or writing to the
variables in question.  That means that it can split and fuse loads
and stores.  Or keep some of the variables in registers, so that it
never loads and stores them.  ;-)

							Thanx, Paul

> Regards,
> Boqun
> 
> > a read.  Perhaps you meant something more like this:
> > 
> > 	if (READ_ONCE(a))
> > 		WRITE_ONCE(b, 1);
> > 	smp_rmb();
> > 	r1 = READ_ONCE(c);
> > 
> > This sequence would guarantee that "a" was read before "c".
> > 
> > 							Thanx, Paul
> >