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

zhidao su posted 1 patch 1 month, 1 week ago
There is a newer version of this series
kernel/sched/ext.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] sched_ext: Use WRITE_ONCE() for the write side of dsq->seq update
Posted by zhidao su 1 month, 1 week ago
bpf_iter_scx_dsq_new() reads dsq->seq via READ_ONCE() without holding
any lock, making dsq->seq a lock-free concurrently accessed variable.
However, dispatch_enqueue(), the sole writer of dsq->seq, uses a plain
increment without the matching WRITE_ONCE() on the write side:

    dsq->seq++;
    ^^^^^^^^^^^
    plain write -- KCSAN data race

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. A plain write leaves
the pair incomplete and will trigger KCSAN warnings.

Fix by using WRITE_ONCE() for the write side of the update:

    WRITE_ONCE(dsq->seq, dsq->seq + 1);

This is consistent with bpf_iter_scx_dsq_new() and makes the
concurrent access annotation complete and KCSAN-clean.

Signed-off-by: zhidao su <suzhidao@xiaomi.com>
---
 kernel/sched/ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 576a9b13eabf..49af85a697a5 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -1137,7 +1137,7 @@ static void dispatch_enqueue(struct scx_sched *sch, struct rq *rq,
 	}
 
 	/* seq records the order tasks are queued, used by BPF DSQ iterator */
-	dsq->seq++;
+	WRITE_ONCE(dsq->seq, dsq->seq + 1);
 	p->scx.dsq_seq = dsq->seq;
 
 	dsq_mod_nr(dsq, 1);
-- 
2.43.0
Re: [PATCH] sched_ext: Use WRITE_ONCE() for the write side of dsq->seq update
Posted by Tejun Heo 1 month, 1 week ago
Hello,

Applied to sched_ext/for-7.0-fixes.

Thanks.

--
tejun