Re: [PATCH tip/core/rcu 01/14] rcu: Protect rcu_boost() lockless accesses with ACCESS_ONCE()

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

----- Original Message -----
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> To: "Peter Zijlstra" <peterz@infradead.org>
> Cc: "Davidlohr Bueso" <dave@stgolabs.net>, linux-kernel@vger.kernel.org, mingo@kernel.org, laijs@cn.fujitsu.com,
> dipankar@in.ibm.com, akpm@linux-foundation.org, "mathieu desnoyers" <mathieu.desnoyers@efficios.com>,
> josh@joshtriplett.org, tglx@linutronix.de, rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com,
> dvhart@linux.intel.com, fweisbec@gmail.com, oleg@redhat.com, "bobby prani" <bobby.prani@gmail.com>,
> borntraeger@de.ibm.com
> Sent: Friday, January 9, 2015 9:07:34 AM
> Subject: Re: [PATCH tip/core/rcu 01/14] rcu: Protect rcu_boost() lockless accesses with ACCESS_ONCE()
> 
> On Fri, Jan 09, 2015 at 02:56:14PM +0100, Peter Zijlstra wrote:
> > On Fri, Jan 09, 2015 at 05:49:54AM -0800, Paul E. McKenney wrote:
> > > > That reminds me, I think the new conversion for stores will most likely
> > > > introduce silly arg bugs:
> > > > 
> > > > -       ACCESS_ONCE(a) = b;
> > > > +       ASSIGN_ONCE(b, a);
> > > 
> > > I was planning to do mine by hand for this sort of reason.
> > > 
> > > Or are you thinking of something more subtle than the case where
> > > "b" is an unparenthesized comma-separated expression?
> > 
> > I think he's revering to the wrong way around-ness of the thing.
> > 
> > Its a bit of a mixed bag on assignments, but for instance
> > rcu_assign_pointer() takes them the right way around, as does
> > atomic_set().
> > 
> > So yes, I think the ASSIGN_ONCE() thing got the arguments the wrong way
> > around.
> 
> Ah, yes, I had forgotten about this point.
> 
> > We could maybe still change it, before its in too long ?
> 
> I would be in favor.  Any objections?  ;-)

Agreed. The target argument should be the first parameter. Also,
do you plan to make ASSIGN_ONCE() evaluate to something or void ?
In userland liburcu CMM_STORE_SHARED(), which evaluates to the value
used as input, we hit warnings with static checkers due to unused
value, which is unfortunate. Not sure if the checkers should be
thought better, or if evaluating to void is the right approach there.

Thanks,

Mathieu


> 
> 							Thanx, Paul
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com