linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Andreea-Cristina Bernat <bernat.ada@gmail.com>,
	paulus@samba.org, mingo@redhat.com, acme@kernel.org,
	linux-kernel@vger.kernel.org, stephen@networkplumber.org,
	eric.dumazet@gmail.com
Subject: Re: [PATCH] kernel: events: core: Replace rcu_assign_pointer() with RCU_INIT_POINTER()
Date: Sun, 28 Sep 2014 00:36:10 -0700	[thread overview]
Message-ID: <20140928073610.GD5015@linux.vnet.ibm.com> (raw)
In-Reply-To: <20140926145335.GI4140@worktop.programming.kicks-ass.net>

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


  reply	other threads:[~2014-09-28  7:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-22 13:26 [PATCH] kernel: events: core: Replace rcu_assign_pointer() with RCU_INIT_POINTER() Andreea-Cristina Bernat
2014-09-09  9:42 ` Peter Zijlstra
2014-09-09 16:16   ` Paul E. McKenney
2014-09-10 13:12     ` Peter Zijlstra
2014-09-12 16:30       ` Paul E. McKenney
2014-09-26 14:53         ` Peter Zijlstra
2014-09-28  7:36           ` Paul E. McKenney [this message]
2014-09-09 14:53 ` [tip:perf/core] perf/core: " tip-bot for Andreea-Cristina Bernat

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=20140928073610.GD5015@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=acme@kernel.org \
    --cc=bernat.ada@gmail.com \
    --cc=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=stephen@networkplumber.org \
    /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).