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

[John Fastabend <john.fastabend@gmail.com>]

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

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