* [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path
@ 2026-05-08 7:29 Vineet Agarwal
2026-05-08 9:26 ` Breno Leitao
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Vineet Agarwal @ 2026-05-08 7:29 UTC (permalink / raw)
To: netdev
Cc: jhs, jiri, davem, edumazet, kuba, pabeni, horms, linux-kernel,
Vineet Agarwal
dualpi2_dump_stats() runs without holding the qdisc lock and provides
best-effort statistics to userspace.
These fields are updated concurrently from enqueue and dequeue paths
and may be observed locklessly in the dump path.
Use READ_ONCE() to ensure safe single-copy loads of these counters and
prevent compiler optimizations that could otherwise result in torn or
inconsistent observations on weakly ordered architectures.
No WRITE_ONCE() annotations are added because these statistics are
maintained as best-effort counters, and the update paths already use
simple non-synchronized increments consistent with existing qdisc
statistics patterns. The intent of this change is only to make the
lockless read semantics explicit.
Signed-off-by: Vineet Agarwal <agarwal.vineet2006@gmail•com>
---
net/sched/sch_dualpi2.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index 241e6a46bd00..40035f70db80 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -1046,14 +1046,14 @@ static int dualpi2_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
struct dualpi2_sched_data *q = qdisc_priv(sch);
struct tc_dualpi2_xstats st = {
.prob = READ_ONCE(q->pi2_prob),
- .packets_in_c = q->packets_in_c,
- .packets_in_l = q->packets_in_l,
- .maxq = q->maxq,
- .ecn_mark = q->ecn_mark,
- .credit = q->c_protection_credit,
- .step_marks = q->step_marks,
- .memory_used = q->memory_used,
- .max_memory_used = q->max_memory_used,
+ .packets_in_c = READ_ONCE(q->packets_in_c),
+ .packets_in_l = READ_ONCE(q->packets_in_l),
+ .maxq = READ_ONCE(q->maxq),
+ .ecn_mark = READ_ONCE(q->ecn_mark),
+ .credit = q->c_protection_credit,
+ .step_marks = READ_ONCE(q->step_marks),
+ .memory_used = READ_ONCE(q->memory_used),
+ .max_memory_used = READ_ONCE(q->max_memory_used),
.memory_limit = q->memory_limit,
};
u64 qc, ql;
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path
2026-05-08 7:29 [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path Vineet Agarwal
@ 2026-05-08 9:26 ` Breno Leitao
2026-05-08 23:08 ` Jakub Kicinski
2026-05-14 11:41 ` Eric Dumazet
2 siblings, 0 replies; 4+ messages in thread
From: Breno Leitao @ 2026-05-08 9:26 UTC (permalink / raw)
To: Vineet Agarwal
Cc: netdev, jhs, jiri, davem, edumazet, kuba, pabeni, horms,
linux-kernel
On Fri, May 08, 2026 at 12:59:18PM +0530, Vineet Agarwal wrote:
> No WRITE_ONCE() annotations are added because these statistics are
> maintained as best-effort counters
Adding READ_ONCE() on the dumper side without matching WRITE_ONCE() on
the writer side does prevent torn stores, and also not actually silence
KCSAN, last time I checked.
> , and the update paths already use simple non-synchronized increments
> consistent with existing qdisc statistics patterns. The intent of this
> change is only to make the lockless read semantics explicit.
I am not sure READ_ONCE() does that. Usually data_race is best used for
this.
> diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c index
> 241e6a46bd00..40035f70db80 100644 --- a/net/sched/sch_dualpi2.c +++
> b/net/sched/sch_dualpi2.c @@ -1046,14 +1046,14 @@ static int
> dualpi2_dump_stats(struct Qdisc *sch, struct gnet_dump *d) struct
> dualpi2_sched_data *q = qdisc_priv(sch); struct tc_dualpi2_xstats st
> = { .prob = READ_ONCE(q->pi2_prob),
...
> + .credit = q->c_protection_credit,
Why this is unnanotated. isn't it uupdated locklessly in
dualpi2_qdisc_dequeue()?
Also, you have spaces other than tab for this instance.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path
2026-05-08 7:29 [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path Vineet Agarwal
2026-05-08 9:26 ` Breno Leitao
@ 2026-05-08 23:08 ` Jakub Kicinski
2026-05-14 11:41 ` Eric Dumazet
2 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-05-08 23:08 UTC (permalink / raw)
To: Vineet Agarwal
Cc: netdev, jhs, jiri, davem, edumazet, pabeni, horms, linux-kernel
On Fri, 8 May 2026 12:59:18 +0530 Vineet Agarwal wrote:
> dualpi2_dump_stats() runs without holding the qdisc lock and provides
> best-effort statistics to userspace.
Didn't Eric already tell you that he's actively working on this?
Please don't get in the way.
--
pw-bot: reject
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path
2026-05-08 7:29 [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path Vineet Agarwal
2026-05-08 9:26 ` Breno Leitao
2026-05-08 23:08 ` Jakub Kicinski
@ 2026-05-14 11:41 ` Eric Dumazet
2 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-05-14 11:41 UTC (permalink / raw)
To: Vineet Agarwal
Cc: netdev, jhs, jiri, davem, kuba, pabeni, horms, linux-kernel
On Fri, May 8, 2026 at 12:29 AM Vineet Agarwal
<agarwal.vineet2006@gmail•com> wrote:
>
> dualpi2_dump_stats() runs without holding the qdisc lock and provides
> best-effort statistics to userspace.
>
> These fields are updated concurrently from enqueue and dequeue paths
> and may be observed locklessly in the dump path.
>
> Use READ_ONCE() to ensure safe single-copy loads of these counters and
> prevent compiler optimizations that could otherwise result in torn or
> inconsistent observations on weakly ordered architectures.
>
> No WRITE_ONCE() annotations are added because these statistics are
> maintained as best-effort counters, and the update paths already use
> simple non-synchronized increments consistent with existing qdisc
> statistics patterns. The intent of this change is only to make the
> lockless read semantics explicit.
I missed this V2, sorry for the delay.
I think we can cook a more correct patch, with READ_ONCE() and WRITE_ONCE().
I will send it now.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-14 11:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 7:29 [PATCH net-next v2] net/sched: sch_dualpi2: annotate lockless stats reads in dump path Vineet Agarwal
2026-05-08 9:26 ` Breno Leitao
2026-05-08 23:08 ` Jakub Kicinski
2026-05-14 11:41 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox