net/sched: sch_sfq: annotate data-races from sfq_dump_class_stats()

sfq_dump_class_stats() runs locklessly, add needed READ_ONCE()
and WRITE_ONCE() annotations.

Fixes: edb09eb17ed8 ("net: sched: do not acquire qdisc spinlock in qdisc/class stats dump")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260505091133.2452510-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index c3f3181dba5424eb9d26362a1628653bb9392e89..f39822babf88bee9d52cac9f39637d38ec36994f 100644 (file)
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -225,7 +225,8 @@ static inline void sfq_dec(struct sfq_sched_data *q, sfq_index x)
 
        sfq_unlink(q, x, n, p);
 
-       d = q->slots[x].qlen--;
+       d = q->slots[x].qlen;
+       WRITE_ONCE(q->slots[x].qlen, d - 1);
        if (n == p && q->cur_depth == d)
                q->cur_depth--;
        sfq_link(q, x);
@@ -238,7 +239,8 @@ static inline void sfq_inc(struct sfq_sched_data *q, sfq_index x)
 
        sfq_unlink(q, x, n, p);
 
-       d = ++q->slots[x].qlen;
+       d = q->slots[x].qlen + 1;
+       WRITE_ONCE(q->slots[x].qlen, d);
        if (q->cur_depth < d)
                q->cur_depth = d;
        sfq_link(q, x);
@@ -298,7 +300,7 @@ static unsigned int sfq_drop(struct Qdisc *sch, struct sk_buff **to_free)
 drop:
                skb = q->headdrop ? slot_dequeue_head(slot) : slot_dequeue_tail(slot);
                len = qdisc_pkt_len(skb);
-               slot->backlog -= len;
+               WRITE_ONCE(slot->backlog, slot->backlog - len);
                sfq_dec(q, x);
                sch->q.qlen--;
                qdisc_qstats_backlog_dec(sch, skb);
@@ -314,7 +316,7 @@ drop:
                        q->tail = NULL; /* no more active slots */
                else
                        q->tail->next = slot->next;
-               q->ht[slot->hash] = SFQ_EMPTY_SLOT;
+               WRITE_ONCE(q->ht[slot->hash], SFQ_EMPTY_SLOT);
                goto drop;
        }
 
@@ -364,10 +366,10 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
                x = q->dep[0].next; /* get a free slot */
                if (x >= SFQ_MAX_FLOWS)
                        return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_MAXFLOWS);
-               q->ht[hash] = x;
+               WRITE_ONCE(q->ht[hash], x);
                slot = &q->slots[x];
                slot->hash = hash;
-               slot->backlog = 0; /* should already be 0 anyway... */
+               WRITE_ONCE(slot->backlog, 0); /* should already be 0 anyway... */
                red_set_vars(&slot->vars);
                goto enqueue;
        }
@@ -426,7 +428,7 @@ congestion_drop:
                head = slot_dequeue_head(slot);
                delta = qdisc_pkt_len(head) - qdisc_pkt_len(skb);
                sch->qstats.backlog -= delta;
-               slot->backlog -= delta;
+               WRITE_ONCE(slot->backlog, slot->backlog - delta);
                qdisc_drop_reason(head, sch, to_free, QDISC_DROP_FLOW_LIMIT);
 
                slot_queue_add(slot, skb);
@@ -436,7 +438,7 @@ congestion_drop:
 
 enqueue:
        qdisc_qstats_backlog_inc(sch, skb);
-       slot->backlog += qdisc_pkt_len(skb);
+       WRITE_ONCE(slot->backlog, slot->backlog + qdisc_pkt_len(skb));
        slot_queue_add(slot, skb);
        sfq_inc(q, x);
        if (slot->qlen == 1) {          /* The flow is new */
@@ -452,7 +454,7 @@ enqueue:
                 */
                q->tail = slot;
                /* We could use a bigger initial quantum for new flows */
-               slot->allot = q->quantum;
+               WRITE_ONCE(slot->allot, q->quantum);
        }
        if (++sch->q.qlen <= q->limit)
                return NET_XMIT_SUCCESS;
@@ -489,7 +491,7 @@ next_slot:
        slot = &q->slots[a];
        if (slot->allot <= 0) {
                q->tail = slot;
-               slot->allot += q->quantum;
+               WRITE_ONCE(slot->allot, slot->allot + q->quantum);
                goto next_slot;
        }
        skb = slot_dequeue_head(slot);
@@ -497,10 +499,10 @@ next_slot:
        qdisc_bstats_update(sch, skb);
        sch->q.qlen--;
        qdisc_qstats_backlog_dec(sch, skb);
-       slot->backlog -= qdisc_pkt_len(skb);
+       WRITE_ONCE(slot->backlog, slot->backlog - qdisc_pkt_len(skb));
        /* Is the slot empty? */
        if (slot->qlen == 0) {
-               q->ht[slot->hash] = SFQ_EMPTY_SLOT;
+               WRITE_ONCE(q->ht[slot->hash], SFQ_EMPTY_SLOT);
                next_a = slot->next;
                if (a == next_a) {
                        q->tail = NULL; /* no more active slots */
@@ -508,7 +510,7 @@ next_slot:
                }
                q->tail->next = next_a;
        } else {
-               slot->allot -= qdisc_pkt_len(skb);
+               WRITE_ONCE(slot->allot, slot->allot - qdisc_pkt_len(skb));
        }
        return skb;
 }
@@ -549,9 +551,9 @@ static void sfq_rehash(struct Qdisc *sch)
                        sfq_dec(q, i);
                        __skb_queue_tail(&list, skb);
                }
-               slot->backlog = 0;
+               WRITE_ONCE(slot->backlog, 0);
                red_set_vars(&slot->vars);
-               q->ht[slot->hash] = SFQ_EMPTY_SLOT;
+               WRITE_ONCE(q->ht[slot->hash], SFQ_EMPTY_SLOT);
        }
        q->tail = NULL;
 
@@ -570,7 +572,7 @@ drop:
                                dropped++;
                                continue;
                        }
-                       q->ht[hash] = x;
+                       WRITE_ONCE(q->ht[hash], x);
                        slot = &q->slots[x];
                        slot->hash = hash;
                }
@@ -581,7 +583,7 @@ drop:
                        slot->vars.qavg = red_calc_qavg(q->red_parms,
                                                        &slot->vars,
                                                        slot->backlog);
-               slot->backlog += qdisc_pkt_len(skb);
+               WRITE_ONCE(slot->backlog, slot->backlog + qdisc_pkt_len(skb));
                sfq_inc(q, x);
                if (slot->qlen == 1) {          /* The flow is new */
                        if (q->tail == NULL) {  /* It is the first flow */
@@ -591,7 +593,7 @@ drop:
                                q->tail->next = x;
                        }
                        q->tail = slot;
-                       slot->allot = q->quantum;
+                       WRITE_ONCE(slot->allot, q->quantum);
                }
        }
        sch->q.qlen -= dropped;
@@ -905,16 +907,16 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
                                struct gnet_dump *d)
 {
        struct sfq_sched_data *q = qdisc_priv(sch);
-       sfq_index idx = q->ht[cl - 1];
+       sfq_index idx = READ_ONCE(q->ht[cl - 1]);
        struct gnet_stats_queue qs = { 0 };
        struct tc_sfq_xstats xstats = { 0 };
 
        if (idx != SFQ_EMPTY_SLOT) {
                const struct sfq_slot *slot = &q->slots[idx];
 
-               xstats.allot = slot->allot;
-               qs.qlen = slot->qlen;
-               qs.backlog = slot->backlog;
+               xstats.allot = READ_ONCE(slot->allot);
+               qs.qlen = READ_ONCE(slot->qlen);
+               qs.backlog = READ_ONCE(slot->backlog);
        }
        if (gnet_stats_copy_queue(d, NULL, &qs, qs.qlen) < 0)
                return -1;
@@ -930,7 +932,7 @@ static void sfq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
                return;
 
        for (i = 0; i < q->divisor; i++) {
-               if (q->ht[i] == SFQ_EMPTY_SLOT) {
+               if (READ_ONCE(q->ht[i]) == SFQ_EMPTY_SLOT) {
                        arg->count++;
                        continue;
                }