Message-ID: <20140928073610.GD5015@linux.vnet.ibm.com>
Date:	Sun, 28 Sep 2014 00:36:10 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Andreea-Cristina Bernat <bernat.ada@...il.com>, paulus@...ba.org,
	mingo@...hat.com, acme@...nel.org, linux-kernel@...r.kernel.org,
	stephen@...workplumber.org, eric.dumazet@...il.com
Subject: Re: [PATCH] kernel: events: core: Replace rcu_assign_pointer() with
 RCU_INIT_POINTER()

On Fri, Sep 26, 2014 at 04:53:35PM +0200, Peter Zijlstra wrote:
> On Fri, Sep 12, 2014 at 09:30:55AM -0700, Paul E. McKenney wrote:
> > On Wed, Sep 10, 2014 at 03:12:51PM +0200, Peter Zijlstra wrote:
> > > On Tue, Sep 09, 2014 at 09:16:48AM -0700, Paul E. McKenney wrote:
> > > > On Tue, Sep 09, 2014 at 11:42:35AM +0200, Peter Zijlstra wrote:
> > > 
> > > > > Paul, why not do something like the below and do away with all this
> > > > > nonsense?
> > > > 
> > > > We used to do that, but the compilers became uncooperative, despite
> > > > Stephen Hemminger's best efforts.
> > > 
> > > Happen to have a link handy to that thread so I can educate myself?
> > 
> > After a bit of software archeology, here you go...
> > 
> > Commit d322f45ceed52 (rcu: Make rcu_assign_pointer() unconditionally
> > insert a memory barrier) made this change.  According to the commit
> > log:
> > 
> >     Recent changes to gcc give warning messages on rcu_assign_pointers()'s
> >     checks that allow it to determine when it is OK to omit the memory
> >     barrier.  Stephen Hemminger tried a number of gcc tricks to silence
> >     this warning, but #pragmas and CPP macros do not work together in the
> >     way that would be required to make this work.
> > 
> > This was applied in 2011, and searching LKML during that time gives
> > the following:  https://lkml.org/lkml/2011/7/29/305
> > 
> > And in this LKML post, Stephen Hemminger wrote:
> > 
> > 	Gcc now generates warnings from rcu_assign_pointer when passed the
> > 	address of something for example:
> > 		rcu_assign_pointer(dev_queue->qdisc, &noop_qdisc);
> > 	This warning is harmless and should be surpressed but there maybe
> > 	other cases where we want that Gcc warning.
> > 
> > 	I tried various combinations of in rcu_assign_pointer macro
> > 	  #pragma GCC diagnostic push
> > 	  #pragma GCC diagnostic ignored "-Wlogical-op"
> > 	  ...
> > 	  #pragma GCC diagnostic pop
> > 	but macro's and pragma's don't nest with the correct scope for
> > 	this.
> > 
> > 	Maybe some one with more Gcc foo and time to waste could take
> > 	a crack at it.
> > 
> > Adding Stephen on CC in case he remembers more.
> 
> So I read all that and am still left wondering WTF the problem was :/
> 
> I googled a bit and found this thread:
> 
>   https://lkml.org/lkml/2014/3/24/39
> 
> Where Michael actually has two 'fixes' to the problem. Instead we
> continue clutter the API with this silly RCU_INIT_POINTER stuff, totally
> sad.

Well, there are the other two use cases for RCU_INIT_POINTER().

But yes, most of the actual uses assign NULL.

> But nowhere have I found the actual warning GCC gives, I suppose I can
> go change my local tree and compile some code to obtain it, but that
> seems backwards. The patch 'working' around that should have mentioned
> it and explained why the warning is bogus or not.

Indeed, I should have had a better commit log on the original change.

							Thanx, Paul

--
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/

Powered by blists - more mailing lists