path: root/include/net/sch_generic.h
diff options
authorEric Dumazet <edumazet@google.com>2019-02-28 12:55:43 -0800
committerDavid S. Miller <davem@davemloft.net>2019-03-02 14:10:18 -0800
commit46b1c18f9deb326a7e18348e668e4c7ab7c7458b (patch)
tree9ab8b4996f340482b8bdcf0a0f9ac11f79117520 /include/net/sch_generic.h
parentMerge branch 'x86-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip (diff)
net: sched: put back q.qlen into a single location
In the series fc8b81a5981f ("Merge branch 'lockless-qdisc-series'") John made the assumption that the data path had no need to read the qdisc qlen (number of packets in the qdisc). It is true when pfifo_fast is used as the root qdisc, or as direct MQ/MQPRIO children. But pfifo_fast can be used as leaf in class full qdiscs, and existing logic needs to access the child qlen in an efficient way. HTB breaks badly, since it uses cl->leaf.q->q.qlen in : htb_activate() -> WARN_ON() htb_dequeue_tree() to decide if a class can be htb_deactivated when it has no more packets. HFSC, DRR, CBQ, QFQ have similar issues, and some calls to qdisc_tree_reduce_backlog() also read q.qlen directly. Using qdisc_qlen_sum() (which iterates over all possible cpus) in the data path is a non starter. It seems we have to put back qlen in a central location, at least for stable kernels. For all qdisc but pfifo_fast, qlen is guarded by the qdisc lock, so the existing q.qlen{++|--} are correct. For 'lockless' qdisc (pfifo_fast so far), we need to use atomic_{inc|dec}() because the spinlock might be not held (for example from pfifo_fast_enqueue() and pfifo_fast_dequeue()) This patch adds atomic_qlen (in the same location than qlen) and renames the following helpers, since we want to express they can be used without qdisc lock, and that qlen is no longer percpu. - qdisc_qstats_cpu_qlen_dec -> qdisc_qstats_atomic_qlen_dec() - qdisc_qstats_cpu_qlen_inc -> qdisc_qstats_atomic_qlen_inc() Later (net-next) we might revert this patch by tracking all these qlen uses and replace them by a more efficient method (not having to access a precise qlen, but an empty/non_empty status that might be less expensive to maintain/track). Another possibility is to have a legacy pfifo_fast version that would be used when used a a child qdisc, since the parent qdisc needs a spinlock anyway. But then, future lockless qdiscs would also have the same problem. Fixes: 7e66016f2c65 ("net: sched: helpers to sum qlen and qlen for per cpu logic") Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: John Fastabend <john.fastabend@gmail.com> Cc: Jamal Hadi Salim <jhs@mojatatu.com> Cc: Cong Wang <xiyou.wangcong@gmail.com> Cc: Jiri Pirko <jiri@resnulli.us> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'include/net/sch_generic.h')
1 files changed, 13 insertions, 18 deletions
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 9481f2c142e2..e7eb4aa6ccc9 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;
@@ -825,14 +820,14 @@ static inline void qdisc_qstats_cpu_backlog_inc(struct Qdisc *sch,
this_cpu_add(sch->cpu_qstats->backlog, qdisc_pkt_len(skb));
-static inline void qdisc_qstats_cpu_qlen_inc(struct Qdisc *sch)
+static inline void qdisc_qstats_atomic_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)
+static inline void qdisc_qstats_atomic_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)