From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Patrick Marlier <patrick.marlier@gmail.com>
Cc: linux-kernel@vger.kernel.org, Josh Triplett <josh@joshtriplett.org>
Subject: Re: [PATCH 1/3] rculist: Fix list_entry_rcu to read ptr with rcu_dereference_raw
Date: Sat, 28 Mar 2015 03:47:12 -0700 [thread overview]
Message-ID: <20150328104712.GA4016@linux.vnet.ibm.com> (raw)
In-Reply-To: <20150328104210.GB28980@linux.vnet.ibm.com>
On Sat, Mar 28, 2015 at 03:42:10AM -0700, Paul E. McKenney wrote:
> On Wed, Mar 25, 2015 at 04:01:24PM +0100, Patrick Marlier wrote:
> > On 03/25/2015 03:30 PM, Paul E. McKenney wrote:
> > >On Tue, Mar 24, 2015 at 11:31:38AM +0100, Patrick Marlier wrote:
> > >>Change to read effectively ptr with rcu_dereference_raw and not the
> > >>__ptr variable on the stack.
> > >>
> > >>Signed-off-by: Patrick Marlier <patrick.marlier@gmail.com>
> > >Avoiding an extra load could be worthwhile in a number of situations,
> > >agreed.
> > Not only a load. It adds a store and a load on the stack and I think
> > this creates a dependency in the processor pipeline.
> >
> > >However, won't this change cause sparse to complain if invoked on a
> > >non-RCU-protected pointer? The ability to use list-RCU API
> > >members on both RCU and non-RCU pointers was one of the points
> > >of the previous commit, right?
> > Probably we can put back the cast but I am not familiar enough with
> > the RCU API.
> >
> > Also, the problem here is that you probably want ACCESS_ONCE to
> > happen on the content of 'ptr' and not on the stack variable
> > '__ptr'.
> >
> > (you have to follow this chain: rcu_dereference_raw ->
> > rcu_dereference_check -> __rcu_dereference_check ->
> > lockless_dereference -> ACCESS_ONCE)
> >
> > #define lockless_dereference(p) \
> > ({ \
> > typeof(p) _________p1 = ACCESS_ONCE(p); \
> > smp_read_barrier_depends(); /* Dependency order vs. p above. */ \
> > (_________p1); \
> > })
> >
> > #define __ACCESS_ONCE(x) ({ \
> > __maybe_unused typeof(x) __var = (__force typeof(x)) 0; \
> > (volatile typeof(x) *)&(x); })
> > #define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
> >
> > Note that ACCESS_ONCE is doing "&" on x.
> >
> > IMHO, I would prefer saving some useless instructions for better
> > performance rather than giving too much flexibility on the API (also
> > pretty sure the cast can be still done).
>
> OK, what I am going to do is to apply your patches for testing purposes.
> If there are no complaints, they will likely go into v4.3 or thereabouts.
Except that I hit conflicts. Could you please rebase to rcu/dev at
git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git?
Thanx, Paul
next prev parent reply other threads:[~2015-03-28 10:47 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-24 10:31 [PATCH 1/3] rculist: Fix list_entry_rcu to read ptr with rcu_dereference_raw Patrick Marlier
2015-03-25 14:30 ` Paul E. McKenney
2015-03-25 15:01 ` Patrick Marlier
2015-03-28 10:42 ` Paul E. McKenney
2015-03-28 10:47 ` Paul E. McKenney [this message]
2015-04-13 11:39 ` Patrick Marlier
2015-04-14 20:34 ` Paul E. McKenney
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=20150328104712.GA4016@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=patrick.marlier@gmail.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).