Re: [RFC PATCH 01/15] net: qdisc: use rcu prefix and silence sparse warnings
From: John Fastabend <john.fastabend@gmail.com>
Date: 2014-05-14 19:39:26
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 <redacted> ---
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 Here are (what I believe are) the relevant changes to trigger it,
quoted hunk ↗ jump to hunk
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;
[...]
quoted hunk ↗ jump to hunk
+++ b/net/sched/sch_generic.c
[...]
quoted hunk ↗ jump to hunk
@@ -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
quoted hunk ↗ jump to hunk
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