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
next prev parent 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).