All of lore.kernel.org
 help / color / mirror / Atom feed
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


  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.