Re: [BUG] net/sched : qlen can not really be per cpu ?

[Eric Dumazet <eric.dumazet@gmail.com>]

On 02/25/2019 10:42 PM, Eric Dumazet wrote:
> HTB + pfifo_fast as a leaf qdisc hits badly the following warning in htb_activate() :
> 
> WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen);
> 
> This is because pfifo_fast does not update sch->q.qlen, but per cpu counters.
> So cl->leaf.q->q.qlen is zero.
> 
> HFSC, CBQ, DRR, QFQ  have the same problem.
> 
> Any ideas how we can fix this ?

What about something simple for stable ?
( I yet have to boot/test this )

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 9481f2c142e26ee1174653d673e6134edd9851da..3a9e442fcaaf2ea48ae65bc87ee95f59cd7100c8 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -51,7 +51,10 @@ struct qdisc_size_table {
 struct qdisc_skb_head {
        struct sk_buff  *head;
        struct sk_buff  *tail;
-       __u32           qlen;
+       union {
+               __u32           qlen;
+               atomic_t        atomic_qlen;
+       };
        spinlock_t      lock;
 };
 
@@ -408,27 +411,19 @@ static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
        BUILD_BUG_ON(sizeof(qcb->data) < sz);
 }
 
-static inline int qdisc_qlen_cpu(const struct Qdisc *q)
-{
-       return this_cpu_ptr(q->cpu_qstats)->qlen;
-}
-
 static inline int qdisc_qlen(const struct Qdisc *q)
 {
        return q->q.qlen;
 }
 
-static inline int qdisc_qlen_sum(const struct Qdisc *q)
+static inline u32 qdisc_qlen_sum(const struct Qdisc *q)
 {
-       __u32 qlen = q->qstats.qlen;
-       int i;
+       u32 qlen = q->qstats.qlen;
 
-       if (q->flags & TCQ_F_NOLOCK) {
-               for_each_possible_cpu(i)
-                       qlen += per_cpu_ptr(q->cpu_qstats, i)->qlen;
-       } else {
+       if (q->flags & TCQ_F_NOLOCK)
+               qlen += atomic_read(&q->q.atomic_qlen);
+       else
                qlen += q->q.qlen;
-       }
 
        return qlen;
 }
@@ -827,12 +822,12 @@ static inline void qdisc_qstats_cpu_backlog_inc(struct Qdisc *sch,
 
 static inline void qdisc_qstats_cpu_qlen_inc(struct Qdisc *sch)
 {
-       this_cpu_inc(sch->cpu_qstats->qlen);
+       atomic_inc(&sch->q.atomic_qlen);
 }
 
 static inline void qdisc_qstats_cpu_qlen_dec(struct Qdisc *sch)
 {
-       this_cpu_dec(sch->cpu_qstats->qlen);
+       atomic_dec(&sch->q.atomic_qlen);
 }
 
 static inline void qdisc_qstats_cpu_requeues_inc(struct Qdisc *sch)