[PATCH] sched_ext: Use READ_ONCE() for the read side of dsq->seq update

[PATCH] sched_ext: Use READ_ONCE() for the read side of dsq->seq update

[David Carlier]

Commit 7a8464555d2e ("sched_ext: Use WRITE_ONCE() for the write side
of dsq->seq update") annotated the plain write of dsq->seq in
dispatch_enqueue(), but left the read side unannotated:

  WRITE_ONCE(dsq->seq, dsq->seq + 1);
                        ^^^^^^^^
                        plain read

  p->scx.dsq_seq = dsq->seq;
                    ^^^^^^^^
                    plain read

bpf_iter_scx_dsq_new() reads dsq->seq via READ_ONCE() without holding
any lock, making dsq->seq a lock-free concurrently accessed variable.
The KCSAN documentation requires that if one accessor uses READ_ONCE()
or WRITE_ONCE() on a variable to annotate lock-free access, all other
accesses must also use the appropriate accessor. The plain reads on
the right-hand side of WRITE_ONCE() and the subsequent assignment
leave the annotation incomplete.

Compute the new sequence number into a local variable using
READ_ONCE(), store it with WRITE_ONCE(), and assign the local to
p->scx.dsq_seq. This completes the annotation started by commit
7a8464555d2e and is consistent with the dsq->nr fix in commit
9adfcef334bf ("sched_ext: Use READ_ONCE() for the read side of
dsq->nr update").

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/sched/ext.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 174e3650d7fe..3fb8365ff563 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -1017,6 +1017,7 @@ static void local_dsq_post_enq(struct scx_dispatch_q *dsq, struct task_struct *p
 static void dispatch_enqueue(struct scx_sched *sch, struct scx_dispatch_q *dsq,
 			     struct task_struct *p, u64 enq_flags)
 {
+	u32 nseq;
 	bool is_local = dsq->id == SCX_DSQ_LOCAL;
 
 	WARN_ON_ONCE(p->scx.dsq || !list_empty(&p->scx.dsq_list.node));
@@ -1103,8 +1104,9 @@ static void dispatch_enqueue(struct scx_sched *sch, struct scx_dispatch_q *dsq,
 	}
 
 	/* seq records the order tasks are queued, used by BPF DSQ iterator */
-	WRITE_ONCE(dsq->seq, dsq->seq + 1);
-	p->scx.dsq_seq = dsq->seq;
+	nseq = READ_ONCE(dsq->seq) + 1;
+	WRITE_ONCE(dsq->seq, nseq);
+	p->scx.dsq_seq = nseq;
 
 	dsq_mod_nr(dsq, 1);
 	p->scx.dsq = dsq;
-- 
2.51.0

Re: [PATCH] sched_ext: Use READ_ONCE() for the read side of dsq->seq update

[Tejun Heo]

On Sat, Mar 07, 2026 at 11:32:04PM +0000, David Carlier wrote:
> -	WRITE_ONCE(dsq->seq, dsq->seq + 1);
> -	p->scx.dsq_seq = dsq->seq;
> +	nseq = READ_ONCE(dsq->seq) + 1;
> +	WRITE_ONCE(dsq->seq, nseq);
> +	p->scx.dsq_seq = nseq;

Does this actually trigger KCSAN or wsa this just generated by AI? dsq->seq
is protected by dsq lock. I don't see how dsq->seq load can become a racy
read.

-- 
tejun

Re: [PATCH] sched_ext: Use READ_ONCE() for the read side of dsq->seq update

[David CARLIER]

I missed that both reads are under dsq->lock ...  it was a wrong
analysis on my part. Sorry for the noise.

On Sun, 8 Mar 2026 at 00:17, Tejun Heo <tj@kernel.org> wrote:
>
> On Sat, Mar 07, 2026 at 11:32:04PM +0000, David Carlier wrote:
> > -     WRITE_ONCE(dsq->seq, dsq->seq + 1);
> > -     p->scx.dsq_seq = dsq->seq;
> > +     nseq = READ_ONCE(dsq->seq) + 1;
> > +     WRITE_ONCE(dsq->seq, nseq);
> > +     p->scx.dsq_seq = nseq;
>
> Does this actually trigger KCSAN or wsa this just generated by AI? dsq->seq
> is protected by dsq lock. I don't see how dsq->seq load can become a racy
> read.
>
> --
> tejun

Re: [PATCH] sched_ext: Use READ_ONCE() for the read side of dsq->seq update

[Tejun Heo]

On Sun, Mar 08, 2026 at 04:22:09AM +0000, David CARLIER wrote:
> I missed that both reads are under dsq->lock ...  it was a wrong
> analysis on my part. Sorry for the noise.

Please stop sending patches that you don't fully understand or prove. It
wouldn't have taken you a lot of effort to see whether this patch was
correct or not. Instead, you shifted unnecessary work to me. This doesn't
help.

Thanks.

-- 
tejun