[PATCH 3/4] selftests/sched_ext: dsq_reenq: fix reliability with two-DSQ design

zhidao su posted 4 patches 6 days, 22 hours ago
[PATCH 3/4] selftests/sched_ext: dsq_reenq: fix reliability with two-DSQ design
Posted by zhidao su 6 days, 22 hours ago
The original test used a single USER_DSQ for both holding tasks and
triggering scx_bpf_dsq_reenq().  ops.dispatch() consumed directly from
USER_DSQ, which raced with the 5 ms BPF timer: by the time the timer
fired, dispatch had already drained USER_DSQ, so scx_bpf_dsq_reenq()
found no tasks to reenqueue and nr_reenq_kfunc remained zero.

Fix with a two-DSQ design:
- USER_DSQ: tasks land here on initial enqueue; ops.dispatch() never
  consumes from it, so tasks accumulate and the timer always finds them.
- RUN_DSQ: reenqueued tasks (SCX_ENQ_REENQ path) land here; dispatch
  consumes from RUN_DSQ so tasks can run.

This guarantees scx_bpf_dsq_reenq() always observes tasks in USER_DSQ
and nr_reenq_kfunc is reliably incremented every timer tick.

Fixes: c50dcf533149 ("selftests/sched_ext: Add tests for SCX_ENQ_IMMED and scx_bpf_dsq_reenq()")
Signed-off-by: zhidao su <suzhidao@xiaomi.com>
---
 .../selftests/sched_ext/dsq_reenq.bpf.c       | 45 ++++++++++++++-----
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/sched_ext/dsq_reenq.bpf.c b/tools/testing/selftests/sched_ext/dsq_reenq.bpf.c
index 750bb10508df..ccce75a99d8a 100644
--- a/tools/testing/selftests/sched_ext/dsq_reenq.bpf.c
+++ b/tools/testing/selftests/sched_ext/dsq_reenq.bpf.c
@@ -2,9 +2,15 @@
 /*
  * Validate scx_bpf_dsq_reenq() semantics on user DSQs.
  *
- * A BPF timer periodically calls scx_bpf_dsq_reenq() on a user DSQ,
- * causing tasks to be re-enqueued through ops.enqueue() with SCX_ENQ_REENQ
- * set and SCX_TASK_REENQ_KFUNC recorded in p->scx.flags.
+ * Worker tasks are placed into USER_DSQ.  ops.dispatch() does NOT consume
+ * from USER_DSQ directly, so tasks accumulate there between timer ticks.
+ * A BPF timer fires every 5 ms and calls scx_bpf_dsq_reenq(USER_DSQ, 0),
+ * which re-enqueues every task in USER_DSQ through ops.enqueue() with
+ * SCX_ENQ_REENQ set and SCX_TASK_REENQ_KFUNC in p->scx.flags.
+ *
+ * Reenqueued tasks are placed into RUN_DSQ so ops.dispatch() can pick them
+ * up and run them.  This two-queue design guarantees tasks remain in
+ * USER_DSQ long enough for the timer to observe them.
  *
  * The test verifies:
  *  - scx_bpf_dsq_reenq() triggers ops.enqueue() with SCX_ENQ_REENQ
@@ -18,7 +24,13 @@ char _license[] SEC("license") = "GPL";
 
 UEI_DEFINE(uei);
 
+/*
+ * USER_DSQ: tasks sit here between timer ticks so scx_bpf_dsq_reenq()
+ *           can observe and re-enqueue them.
+ * RUN_DSQ:  tasks land here after reenqueue and are consumed by dispatch.
+ */
 #define USER_DSQ	0
+#define RUN_DSQ		1
 
 /*
  * SCX_TASK_REENQ_REASON_MASK and SCX_TASK_REENQ_KFUNC are exported via
@@ -58,27 +70,36 @@ static int reenq_timerfn(void *map, int *key, struct bpf_timer *timer)
 
 void BPF_STRUCT_OPS(dsq_reenq_enqueue, struct task_struct *p, u64 enq_flags)
 {
-	/*
-	 * If this is a kfunc-triggered reenqueue, verify that
-	 * SCX_TASK_REENQ_KFUNC is recorded in p->scx.flags.
-	 */
 	if (enq_flags & SCX_ENQ_REENQ) {
 		u32 reason = p->scx.flags & SCX_TASK_REENQ_REASON_MASK;
 
 		if (reason == SCX_TASK_REENQ_KFUNC)
 			__sync_fetch_and_add(&nr_reenq_kfunc, 1);
+
+		/*
+		 * Reenqueued tasks go to RUN_DSQ so dispatch() can run them.
+		 * Keeping them in USER_DSQ would cause the timer to reenqueue
+		 * them again endlessly without ever running.
+		 */
+		scx_bpf_dsq_insert(p, RUN_DSQ, SCX_SLICE_DFL, enq_flags);
+		return;
 	}
 
 	/*
-	 * Always dispatch to USER_DSQ so the timer can reenqueue tasks again
-	 * on the next tick.
+	 * Non-reenqueue path: place the task in USER_DSQ.  ops.dispatch()
+	 * does not consume USER_DSQ directly, so tasks sit here until the
+	 * timer fires and calls scx_bpf_dsq_reenq(USER_DSQ, 0).
 	 */
 	scx_bpf_dsq_insert(p, USER_DSQ, SCX_SLICE_DFL, enq_flags);
 }
 
 void BPF_STRUCT_OPS(dsq_reenq_dispatch, s32 cpu, struct task_struct *prev)
 {
-	scx_bpf_dsq_move_to_local(USER_DSQ, 0);
+	/*
+	 * Only consume from RUN_DSQ (tasks that have been through the
+	 * reenqueue path).  USER_DSQ is deliberately left for the timer.
+	 */
+	scx_bpf_dsq_move_to_local(RUN_DSQ, 0);
 }
 
 s32 BPF_STRUCT_OPS_SLEEPABLE(dsq_reenq_init)
@@ -91,6 +112,10 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(dsq_reenq_init)
 	if (ret)
 		return ret;
 
+	ret = scx_bpf_create_dsq(RUN_DSQ, -1);
+	if (ret)
+		return ret;
+
 	if (!__COMPAT_has_generic_reenq()) {
 		scx_bpf_error("scx_bpf_dsq_reenq() not available");
 		return -EOPNOTSUPP;
-- 
2.43.0