Skip to content

Commit 770b136

Browse files
edumazetkuba-moo
edumazet
authored and
kuba-moo
committed
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: edb09eb ("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>
1 parent 67ef490 commit 770b136

1 file changed

Lines changed: 25 additions & 23 deletions

File tree

‎net/sched/sch_sfq.c‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,8 @@ static inline void sfq_dec(struct sfq_sched_data *q, sfq_index x)
225225

226226
sfq_unlink(q, x, n, p);
227227

228-
d = q->slots[x].qlen--;
228+
d = q->slots[x].qlen;
229+
WRITE_ONCE(q->slots[x].qlen, d - 1);
229230
if (n == p && q->cur_depth == d)
230231
q->cur_depth--;
231232
sfq_link(q, x);
@@ -238,7 +239,8 @@ static inline void sfq_inc(struct sfq_sched_data *q, sfq_index x)
238239

239240
sfq_unlink(q, x, n, p);
240241

241-
d = ++q->slots[x].qlen;
242+
d = q->slots[x].qlen + 1;
243+
WRITE_ONCE(q->slots[x].qlen, d);
242244
if (q->cur_depth < d)
243245
q->cur_depth = d;
244246
sfq_link(q, x);
@@ -298,7 +300,7 @@ static unsigned int sfq_drop(struct Qdisc *sch, struct sk_buff **to_free)
298300
drop:
299301
skb = q->headdrop ? slot_dequeue_head(slot) : slot_dequeue_tail(slot);
300302
len = qdisc_pkt_len(skb);
301-
slot->backlog -= len;
303+
WRITE_ONCE(slot->backlog, slot->backlog - len);
302304
sfq_dec(q, x);
303305
sch->q.qlen--;
304306
qdisc_qstats_backlog_dec(sch, skb);
@@ -314,7 +316,7 @@ static unsigned int sfq_drop(struct Qdisc *sch, struct sk_buff **to_free)
314316
q->tail = NULL; /* no more active slots */
315317
else
316318
q->tail->next = slot->next;
317-
q->ht[slot->hash] = SFQ_EMPTY_SLOT;
319+
WRITE_ONCE(q->ht[slot->hash], SFQ_EMPTY_SLOT);
318320
goto drop;
319321
}
320322

@@ -364,10 +366,10 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
364366
x = q->dep[0].next; /* get a free slot */
365367
if (x >= SFQ_MAX_FLOWS)
366368
return qdisc_drop_reason(skb, sch, to_free, QDISC_DROP_MAXFLOWS);
367-
q->ht[hash] = x;
369+
WRITE_ONCE(q->ht[hash], x);
368370
slot = &q->slots[x];
369371
slot->hash = hash;
370-
slot->backlog = 0; /* should already be 0 anyway... */
372+
WRITE_ONCE(slot->backlog, 0); /* should already be 0 anyway... */
371373
red_set_vars(&slot->vars);
372374
goto enqueue;
373375
}
@@ -426,7 +428,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
426428
head = slot_dequeue_head(slot);
427429
delta = qdisc_pkt_len(head) - qdisc_pkt_len(skb);
428430
sch->qstats.backlog -= delta;
429-
slot->backlog -= delta;
431+
WRITE_ONCE(slot->backlog, slot->backlog - delta);
430432
qdisc_drop_reason(head, sch, to_free, QDISC_DROP_FLOW_LIMIT);
431433

432434
slot_queue_add(slot, skb);
@@ -436,7 +438,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
436438

437439
enqueue:
438440
qdisc_qstats_backlog_inc(sch, skb);
439-
slot->backlog += qdisc_pkt_len(skb);
441+
WRITE_ONCE(slot->backlog, slot->backlog + qdisc_pkt_len(skb));
440442
slot_queue_add(slot, skb);
441443
sfq_inc(q, x);
442444
if (slot->qlen == 1) { /* The flow is new */
@@ -452,7 +454,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
452454
*/
453455
q->tail = slot;
454456
/* We could use a bigger initial quantum for new flows */
455-
slot->allot = q->quantum;
457+
WRITE_ONCE(slot->allot, q->quantum);
456458
}
457459
if (++sch->q.qlen <= q->limit)
458460
return NET_XMIT_SUCCESS;
@@ -489,26 +491,26 @@ sfq_dequeue(struct Qdisc *sch)
489491
slot = &q->slots[a];
490492
if (slot->allot <= 0) {
491493
q->tail = slot;
492-
slot->allot += q->quantum;
494+
WRITE_ONCE(slot->allot, slot->allot + q->quantum);
493495
goto next_slot;
494496
}
495497
skb = slot_dequeue_head(slot);
496498
sfq_dec(q, a);
497499
qdisc_bstats_update(sch, skb);
498500
sch->q.qlen--;
499501
qdisc_qstats_backlog_dec(sch, skb);
500-
slot->backlog -= qdisc_pkt_len(skb);
502+
WRITE_ONCE(slot->backlog, slot->backlog - qdisc_pkt_len(skb));
501503
/* Is the slot empty? */
502504
if (slot->qlen == 0) {
503-
q->ht[slot->hash] = SFQ_EMPTY_SLOT;
505+
WRITE_ONCE(q->ht[slot->hash], SFQ_EMPTY_SLOT);
504506
next_a = slot->next;
505507
if (a == next_a) {
506508
q->tail = NULL; /* no more active slots */
507509
return skb;
508510
}
509511
q->tail->next = next_a;
510512
} else {
511-
slot->allot -= qdisc_pkt_len(skb);
513+
WRITE_ONCE(slot->allot, slot->allot - qdisc_pkt_len(skb));
512514
}
513515
return skb;
514516
}
@@ -549,9 +551,9 @@ static void sfq_rehash(struct Qdisc *sch)
549551
sfq_dec(q, i);
550552
__skb_queue_tail(&list, skb);
551553
}
552-
slot->backlog = 0;
554+
WRITE_ONCE(slot->backlog, 0);
553555
red_set_vars(&slot->vars);
554-
q->ht[slot->hash] = SFQ_EMPTY_SLOT;
556+
WRITE_ONCE(q->ht[slot->hash], SFQ_EMPTY_SLOT);
555557
}
556558
q->tail = NULL;
557559

@@ -570,7 +572,7 @@ static void sfq_rehash(struct Qdisc *sch)
570572
dropped++;
571573
continue;
572574
}
573-
q->ht[hash] = x;
575+
WRITE_ONCE(q->ht[hash], x);
574576
slot = &q->slots[x];
575577
slot->hash = hash;
576578
}
@@ -581,7 +583,7 @@ static void sfq_rehash(struct Qdisc *sch)
581583
slot->vars.qavg = red_calc_qavg(q->red_parms,
582584
&slot->vars,
583585
slot->backlog);
584-
slot->backlog += qdisc_pkt_len(skb);
586+
WRITE_ONCE(slot->backlog, slot->backlog + qdisc_pkt_len(skb));
585587
sfq_inc(q, x);
586588
if (slot->qlen == 1) { /* The flow is new */
587589
if (q->tail == NULL) { /* It is the first flow */
@@ -591,7 +593,7 @@ static void sfq_rehash(struct Qdisc *sch)
591593
q->tail->next = x;
592594
}
593595
q->tail = slot;
594-
slot->allot = q->quantum;
596+
WRITE_ONCE(slot->allot, q->quantum);
595597
}
596598
}
597599
sch->q.qlen -= dropped;
@@ -905,16 +907,16 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
905907
struct gnet_dump *d)
906908
{
907909
struct sfq_sched_data *q = qdisc_priv(sch);
908-
sfq_index idx = q->ht[cl - 1];
910+
sfq_index idx = READ_ONCE(q->ht[cl - 1]);
909911
struct gnet_stats_queue qs = { 0 };
910912
struct tc_sfq_xstats xstats = { 0 };
911913

912914
if (idx != SFQ_EMPTY_SLOT) {
913915
const struct sfq_slot *slot = &q->slots[idx];
914916

915-
xstats.allot = slot->allot;
916-
qs.qlen = slot->qlen;
917-
qs.backlog = slot->backlog;
917+
xstats.allot = READ_ONCE(slot->allot);
918+
qs.qlen = READ_ONCE(slot->qlen);
919+
qs.backlog = READ_ONCE(slot->backlog);
918920
}
919921
if (gnet_stats_copy_queue(d, NULL, &qs, qs.qlen) < 0)
920922
return -1;
@@ -930,7 +932,7 @@ static void sfq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
930932
return;
931933

932934
for (i = 0; i < q->divisor; i++) {
933-
if (q->ht[i] == SFQ_EMPTY_SLOT) {
935+
if (READ_ONCE(q->ht[i]) == SFQ_EMPTY_SLOT) {
934936
arg->count++;
935937
continue;
936938
}

0 commit comments

Comments
 (0)