Re: [RFC PATCH 01/15] net: qdisc: use rcu prefix and silence sparse warnings

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

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 <john.r.fastabend@intel.com>
> >---
> 
> 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 <linux/debugobjects.h>
> > #include <linux/bug.h>
> > #include <linux/compiler.h>
> >-#include <asm/barrier.h>
> >
> > 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
>