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

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.

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

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.