Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754522AbbDNUez (ORCPT ); Tue, 14 Apr 2015 16:34:55 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:35868 "EHLO e37.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753409AbbDNUeq (ORCPT ); Tue, 14 Apr 2015 16:34:46 -0400 Date: Tue, 14 Apr 2015 13:34:39 -0700 From: "Paul E. McKenney" To: Patrick Marlier Cc: linux-kernel@vger.kernel.org, Josh Triplett Subject: Re: [PATCH 1/3] rculist: Fix list_entry_rcu to read ptr with rcu_dereference_raw Message-ID: <20150414203439.GE23685@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <55113D0A.2030302@gmail.com> <20150325143018.GZ5718@linux.vnet.ibm.com> <5512CDC4.10203@gmail.com> <20150328104210.GB28980@linux.vnet.ibm.com> <20150328104712.GA4016@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15041420-0025-0000-0000-000009E3BF56 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6416 Lines: 166 On Mon, Apr 13, 2015 at 01:39:04PM +0200, Patrick Marlier wrote: > Strange I don't get any conflict. > Maybe due to my email client so I attached the patches to this email. That did the trick, thank you! I of course pass on the ritual advice to check your email client settings. I have queued these for 4.2, and will let you know if any problems show up during testing. Thanx, Paul > Thanks, > -- > Patrick > > On Sat, Mar 28, 2015 at 11:47 AM, Paul E. McKenney > wrote: > > 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 > >> > >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 > > > From 8ac818d418068105623e43bbd289d9553c182e6c Mon Sep 17 00:00:00 2001 > From: Patrick Marlier > Date: Tue, 24 Mar 2015 11:16:55 +0100 > Subject: [PATCH 1/3] rculist: Fix list_entry_rcu to read ptr with > rcu_dereference_raw > > Change to read effectively ptr with rcu_dereference_raw and not the > __ptr variable on the stack. > > Signed-off-by: Patrick Marlier > --- > include/linux/rculist.h | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/include/linux/rculist.h b/include/linux/rculist.h > index a18b16f..9d9baea 100644 > --- a/include/linux/rculist.h > +++ b/include/linux/rculist.h > @@ -247,10 +247,7 @@ static inline void list_splice_init_rcu(struct list_head *list, > * primitives such as list_add_rcu() as long as it's guarded by rcu_read_lock(). > */ > #define list_entry_rcu(ptr, type, member) \ > -({ \ > - typeof(*ptr) __rcu *__ptr = (typeof(*ptr) __rcu __force *)ptr; \ > - container_of((typeof(ptr))rcu_dereference_raw(__ptr), type, member); \ > -}) > + container_of((typeof(ptr))rcu_dereference_raw(ptr), type, member) > > /** > * Where are list_empty_rcu() and list_first_entry_rcu()? > -- > 2.1.0 > > From 3ab5f342939f768b693708bd32ef3f350af3b5a6 Mon Sep 17 00:00:00 2001 > From: Patrick Marlier > Date: Tue, 24 Mar 2015 11:21:05 +0100 > Subject: [PATCH 2/3] netfilter: fix list_entry_rcu usage. > > Signed-off-by: Patrick Marlier > --- > net/netfilter/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/netfilter/core.c b/net/netfilter/core.c > index fea9ef5..05bd311 100644 > --- a/net/netfilter/core.c > +++ b/net/netfilter/core.c > @@ -174,7 +174,7 @@ int nf_hook_slow(u_int8_t pf, unsigned int hook, struct sk_buff *skb, > /* We may already have this, but read-locks nest anyway */ > rcu_read_lock(); > > - elem = list_entry_rcu(&nf_hooks[pf][hook], struct nf_hook_ops, list); > + elem = list_entry_rcu(nf_hooks[pf][hook].next, struct nf_hook_ops, list); > next_hook: > verdict = nf_iterate(&nf_hooks[pf][hook], skb, hook, indev, > outdev, &elem, okfn, hook_thresh); > -- > 2.1.0 > > From 84f0428e2c9172692aba727636a643efb6994752 Mon Sep 17 00:00:00 2001 > From: Patrick Marlier > Date: Tue, 24 Mar 2015 11:22:10 +0100 > Subject: [PATCH 3/3] md/bitmap: fix list_entry_rcu usage. > > Signed-off-by: Patrick Marlier > --- > drivers/md/bitmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c > index 3a57679..ed00e46 100644 > --- a/drivers/md/bitmap.c > +++ b/drivers/md/bitmap.c > @@ -181,7 +181,7 @@ static struct md_rdev *next_active_rdev(struct md_rdev *rdev, struct mddev *mdde > rcu_read_lock(); > if (rdev == NULL) > /* start at the beginning */ > - rdev = list_entry_rcu(&mddev->disks, struct md_rdev, same_set); > + rdev = list_entry_rcu(mddev->disks.next, struct md_rdev, same_set); > else { > /* release the previous rdev and start from there. */ > rdev_dec_pending(rdev, mddev); > -- > 2.1.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/