netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Waiman Long <waiman.long@hpe.com>
To: <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	<linux-kernel@vger.kernel.org>, <torvalds@linux-foundation.org>,
	<manfred@colorfullife.com>, <dave@stgolabs.net>,
	<will.deacon@arm.com>, <boqun.feng@gmail.com>, <tj@kernel.org>,
	<pablo@netfilter.org>, <kaber@trash.net>, <davem@davemloft.net>,
	<oleg@redhat.com>, <netfilter-devel@vger.kernel.org>,
	<sasha.levin@oracle.com>, <hofrat@osadl.org>
Subject: Re: [RFC][PATCH 1/3] locking: Introduce smp_acquire__after_ctrl_dep
Date: Wed, 25 May 2016 11:20:42 -0400	[thread overview]
Message-ID: <5745C2CA.4040003@hpe.com> (raw)
In-Reply-To: <20160525045329.GQ4148@linux.vnet.ibm.com>

On 05/25/2016 12:53 AM, 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@infradead.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
> 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
>

The smp_rmb() in Linux should be a compiler barrier. So the compiler 
should not recorder it above smp_rmb. However, what I am wondering is 
whether a condition + rmb combination can be considered a real acquire 
memory barrier from the CPU point of view which requires that it cannot 
reorder the data store in S3 above S1 and S2. This is where I am not so 
sure about.

Cheers,
Longman

  parent reply	other threads:[~2016-05-25 15:20 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-24 14:27 [RFC][PATCH 0/3] spin_unlock_wait and assorted borkage Peter Zijlstra
2016-05-24 14:27 ` [RFC][PATCH 1/3] locking: Introduce smp_acquire__after_ctrl_dep Peter Zijlstra
     [not found]   ` <57451581.6000700@hpe.com>
2016-05-25  4:53     ` Paul E. McKenney
2016-05-25  5:39       ` Boqun Feng
2016-05-25 14:29         ` Paul E. McKenney
2016-05-25 15:20       ` Waiman Long [this message]
2016-05-25 15:57         ` Paul E. McKenney
2016-05-25 16:28           ` Peter Zijlstra
2016-05-25 16:54             ` Linus Torvalds
2016-05-25 18:59               ` Paul E. McKenney
2016-06-03  9:18           ` Vineet Gupta
2016-06-03  9:38             ` Peter Zijlstra
2016-06-03 12:08               ` Paul E. McKenney
2016-06-03 12:23                 ` Peter Zijlstra
2016-06-03 12:27                   ` Peter Zijlstra
2016-06-03 13:33                     ` Paul E. McKenney
2016-06-03 13:32                   ` Paul E. McKenney
2016-06-03 13:45                     ` Will Deacon
2016-06-04 15:29                       ` Paul E. McKenney
2016-06-06 17:28                         ` Paul E. McKenney
2016-06-07  7:15                           ` Peter Zijlstra
2016-06-07 12:41                             ` Hannes Frederic Sowa
2016-06-07 13:06                               ` Paul E. McKenney
2016-06-07 14:59                                 ` Hannes Frederic Sowa
2016-06-07 15:23                                   ` Paul E. McKenney
2016-06-07 17:48                                     ` Peter Zijlstra
2016-06-07 18:44                                       ` Paul E. McKenney
2016-06-07 18:01                                     ` Will Deacon
2016-06-07 18:44                                       ` Paul E. McKenney
2016-06-07 18:54                                       ` Paul E. McKenney
2016-06-07 18:37                                     ` Hannes Frederic Sowa
2016-05-24 14:27 ` [RFC][PATCH 2/3] locking: Annotate spin_unlock_wait() users Peter Zijlstra
2016-05-24 16:17   ` Linus Torvalds
2016-05-24 16:22     ` Tejun Heo
2016-05-24 16:58       ` Peter Zijlstra
2016-05-25 19:28         ` Tejun Heo
2016-05-24 16:57     ` Peter Zijlstra
2016-05-24 14:27 ` [RFC][PATCH 3/3] locking,netfilter: Fix nf_conntrack_lock() Peter Zijlstra
2016-05-24 14:42   ` Peter Zijlstra
     [not found]   ` <3e1671fc-be0f-bc95-4fbb-6bfc56e6c15b@colorfullife.com>
2016-05-26 13:54     ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5745C2CA.4040003@hpe.com \
    --to=waiman.long@hpe.com \
    --cc=boqun.feng@gmail.com \
    --cc=dave@stgolabs.net \
    --cc=davem@davemloft.net \
    --cc=hofrat@osadl.org \
    --cc=kaber@trash.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manfred@colorfullife.com \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=pablo@netfilter.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=sasha.levin@oracle.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).