From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Paul E. McKenney" Subject: Re: [RFC PATCH 01/15] net: qdisc: use rcu prefix and silence sparse warnings Date: Thu, 15 May 2014 13:41:48 -0700 Message-ID: <20140515204148.GP4570@linux.vnet.ibm.com> References: <20140430163250.9021.21199.stgit@nitbit.x32> <20140430163504.9021.12828.stgit@nitbit.x32> <5373C660.7030301@gmail.com> Reply-To: paulmck@linux.vnet.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: eric.dumazet@gmail.com, Cong Wang , jhs@mojatatu.com, netdev@vger.kernel.org, davem@davemloft.net To: John Fastabend Return-path: Received: from e31.co.us.ibm.com ([32.97.110.149]:41230 "EHLO e31.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753039AbaEOUlv (ORCPT ); Thu, 15 May 2014 16:41:51 -0400 Received: from /spool/local by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 15 May 2014 14:41:51 -0600 Received: from b03cxnp07028.gho.boulder.ibm.com (b03cxnp07028.gho.boulder.ibm.com [9.17.130.15]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 7168219D8026 for ; Thu, 15 May 2014 14:41:43 -0600 (MDT) Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by b03cxnp07028.gho.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s4FKehGJ10355182 for ; Thu, 15 May 2014 22:40:43 +0200 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id s4FKjgAc014339 for ; Thu, 15 May 2014 14:45:43 -0600 Content-Disposition: inline In-Reply-To: <5373C660.7030301@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, May 14, 2014 at 12:39:12PM -0700, John Fastabend wrote: > On 04/30/2014 09:35 AM, John Fastabend wrote: > >Add __rcu notation to qdisc handling by doing this we can make > >smatch output more legible. And anyways some of the cases should > >be using rcu_dereference() see qdisc_all_tx_empty(), > >qdisc_tx_chainging(), and so on. > > > >Signed-off-by: John Fastabend > >--- > > Now I'm trying to resolve the lingering sparse errors/warnings and I > have one that I'm not sure about. Maybe someone has some insight, > > net/sched/sch_generic.c:694:9: error: bad constant expression > net/sched/sch_generic.c:694:9: error: cannot size expression > net/sched/sch_generic.c:751:9: error: bad constant expression > net/sched/sch_generic.c:751:9: error: cannot size expression > net/sched/sch_generic.c:800:17: error: bad constant expression > net/sched/sch_generic.c:800:17: error: cannot size expression > net/sched/sch_generic.c:886:9: error: bad constant expression > net/sched/sch_generic.c:886:9: error: cannot size expression > net/sched/sch_generic.c:908:17: error: bad constant expression > net/sched/sch_generic.c:908:17: error: cannot size expression There is some compiletime_assert_atomic_type() bustage that causes errors like this. There should be a fix on its way in. Try making compiletime_assert_atomic_type() be an empty macro, and if that works, help is on the way. Thanx, Paul > Here are (what I believe are) the relevant changes to trigger it, > > > include/linux/netdevice.h | 29 ++++------------------------- > > include/net/sch_generic.h | 29 +++++++++++++++++++++++------ > > net/core/dev.c | 45 +++++++++++++++++++++++++++++++++++++++++++-- > > net/sched/sch_generic.c | 4 ++-- > > net/sched/sch_mqprio.c | 6 ++++-- > > net/sched/sch_teql.c | 9 +++++---- > > 6 files changed, 81 insertions(+), 41 deletions(-) > > > >diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > >index a803d79..4826988 100644 > >--- a/include/linux/netdevice.h > >+++ b/include/linux/netdevice.h > >@@ -546,7 +546,7 @@ struct netdev_queue { > > * read mostly part > > */ > > struct net_device *dev; > >- struct Qdisc *qdisc; > >+ struct Qdisc __rcu *qdisc; > > Add __rcu here, > > > struct Qdisc *qdisc_sleeping; > > #ifdef CONFIG_SYSFS > > struct kobject kobj; > > [...] > > >+++ b/net/sched/sch_generic.c > > [...] > > >@@ -871,7 +871,7 @@ static void dev_init_scheduler_queue(struct net_device *dev, > > { > > struct Qdisc *qdisc = _qdisc; > > > >- dev_queue->qdisc = qdisc; > >+ rcu_assign_pointer(dev_queue->qdisc, qdisc); > > dev_queue->qdisc_sleeping = qdisc; > > } > > Then rcu_assign_pointer() operations throw the "bad constant expression" > and "cannot size expression" errors. The line numbers are a bit off from > the errors above due to some other improvements/fixes but this is line > 886 from above sparse output. > > I've done similar types of transformations before without sparse errors > so I thought I might see what changed. After doing a revert of > > "rcu: Define rcu_assign_pointer() in terms of smp_store_release()" > commit 88c1863066ccfa456797e12c5d8b4631aa1ad0d0 > > Sparse no longer throws an error. Here is the diff of that commit for > reference > > >diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > >index 00a7fd6..4550f22 100644 > >--- a/include/linux/rcupdate.h > >+++ b/include/linux/rcupdate.h > >@@ -44,7 +44,6 @@ > > #include > > #include > > #include > >-#include > > > > extern int rcu_expedited; /* for sysctl */ > > #ifdef CONFIG_RCU_TORTURE_TEST > >@@ -582,7 +581,12 @@ static inline void rcu_preempt_sleep_check(void) > > * please be careful when making changes to rcu_assign_pointer() and the > > * other macros that it invokes. > > */ > >-#define rcu_assign_pointer(p, v) smp_store_release(&p, RCU_INITIALIZER(v)) > >+#define rcu_assign_pointer(p, v) \ > >+ do { \ > >+ smp_wmb(); \ > >+ ACCESS_ONCE(p) = RCU_INITIALIZER(v); \ > >+ } while (0) > >+ > > > > /** > > * rcu_access_pointer() - fetch RCU pointer with no dereferencing > > At the moment I'm not seeing what changed that would cause the error. > Any ideas what I got wrong here? > > Thanks! > John > > -- > John Fastabend Intel Corporation >