kernel/sched/ext.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
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
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
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
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
© 2016 - 2026 Red Hat, Inc.