[PATCH v2 0/2] sched_ext: Don't run ops.dequeue() with a DSQ lock held

[PATCH v2 0/2] sched_ext: Don't run ops.dequeue() with a DSQ lock held

[Qiurong Fang]

From: fangqiurong <fangqiurong@kylinos.cn>

The consume, move and terminal insert paths invoke ops.dequeue() with a
DSQ lock held, self-deadlocking any BPF scheduler which locks the same
DSQ from ops.dequeue(). Move the invocations after the DSQ unlock.

v1 -> v2:
- Drop the claim that ops.dequeue() may run after the task has
  re-entered custody: SCX_OPSS_DISPATCHING is held across the callback
  on the global/bypass path and @p's rq lock is held across the
  callback on the user-DSQ-to-local paths (Andrea Righi)
- Add a selftest whose ops.dequeue() iterates the source user DSQ
  (Andrea Righi)

Patch 1 carries Andrea's Acked-by from the v1 review.
Link: https://lore.kernel.org/all/aqkQ1MtqtqnS0wUs@gpd4/

fangqiurong (2):
  sched_ext: Don't run ops.dequeue() with a DSQ lock held
  selftests/sched_ext: Test that ops.dequeue() can iterate the consumed
    DSQ

 Documentation/scheduler/sched-ext.rst         |  4 +-
 kernel/sched/ext/ext.c                        | 67 ++++++++++---
 tools/testing/selftests/sched_ext/Makefile    |  1 +
 .../selftests/sched_ext/dequeue_iter.bpf.c    | 65 +++++++++++++
 .../selftests/sched_ext/dequeue_iter.c        | 94 +++++++++++++++++++
 5 files changed, 216 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/sched_ext/dequeue_iter.bpf.c
 create mode 100644 tools/testing/selftests/sched_ext/dequeue_iter.c

-- 
2.43.0

[PATCH v2 1/2] sched_ext: Don't run ops.dequeue() with a DSQ lock held

[Qiurong Fang]

From: fangqiurong <fangqiurong@kylinos.cn>

ops.dequeue() is called with the source user DSQ's lock still held on
the consume and move paths (scx_consume_dispatch_q(),
move_task_between_dsqs()) and with the terminal global/bypass DSQ's
lock still held in scx_dispatch_enqueue(). A BPF scheduler that locks
the same DSQ from ops.dequeue() - e.g. by iterating it with
bpf_iter_scx_dsq, which takes the DSQ lock on every step -
self-deadlocks.

Move the invocation after the DSQ unlock on all three paths.
SCX_TASK_IN_CUSTODY is cleared under the lock so that the callback is
invoked exactly once; it is not ordered against consumption of the
task and may run after the task has been moved to, or consumed from,
a terminal DSQ.

Fixes: ebf1ccff79c4 ("sched_ext: Fix ops.dequeue() semantics")
Acked-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>
---
 Documentation/scheduler/sched-ext.rst |  4 +-
 kernel/sched/ext/ext.c                | 67 +++++++++++++++++++++------
 2 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/Documentation/scheduler/sched-ext.rst b/Documentation/scheduler/sched-ext.rst
index 794ae80b3ba3..6e65d3b38951 100644
--- a/Documentation/scheduler/sched-ext.rst
+++ b/Documentation/scheduler/sched-ext.rst
@@ -361,7 +361,9 @@ The following briefly shows how a waking task is scheduled and executed.
    ``scx_bpf_dsq_reenq()``. The task stays in BPF custody the entire time.
 
    When a task leaves BPF scheduler custody, ``ops.dequeue()`` is invoked.
-   The dequeue can happen for different reasons, distinguished by flags:
+   The callback is not ordered against consumption of the task and may run
+   after the task has been moved to, or consumed from, a terminal DSQ. The
+   dequeue can happen for different reasons, distinguished by flags:
 
    1. **Regular dispatch**: when a task in BPF custody is dispatched to a
       terminal DSQ from ``ops.dispatch()`` (leaving BPF custody for
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 40fa1697bdb7..13efddb729e3 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -1499,20 +1499,28 @@ static inline bool task_scx_migrating(struct task_struct *p)
 	return p->scx.sticky_cpu >= 0;
 }
 
+/* Must be called under the lock serializing @p's custody transfers. */
+static bool task_leave_custody(struct task_struct *p)
+{
+	if (!(p->scx.flags & SCX_TASK_IN_CUSTODY) || task_scx_migrating(p))
+		return false;
+
+	p->scx.flags &= ~SCX_TASK_IN_CUSTODY;
+	return true;
+}
+
 /*
  * Call ops.dequeue() if the task is in BPF custody and not migrating.
- * Clears %SCX_TASK_IN_CUSTODY when the callback is invoked.
+ * Clears %SCX_TASK_IN_CUSTODY before the callback is invoked.
  */
 static void call_task_dequeue(struct scx_sched *sch, struct rq *rq,
 			      struct task_struct *p, u64 deq_flags)
 {
-	if (!(p->scx.flags & SCX_TASK_IN_CUSTODY) || task_scx_migrating(p))
+	if (!task_leave_custody(p))
 		return;
 
 	if (SCX_HAS_OP(sch, dequeue))
 		SCX_CALL_OP_TASK(sch, dequeue, rq, p, deq_flags);
-
-	p->scx.flags &= ~SCX_TASK_IN_CUSTODY;
 }
 
 static void rq_owned_post_enq(struct scx_sched *sch, struct rq *rq,
@@ -1705,20 +1713,28 @@ static void scx_dispatch_enqueue(struct scx_sched *sch, struct rq *rq,
 	if (is_rq_owned) {
 		rq_owned_post_enq(sch, rq, dsq, p, enq_flags);
 	} else {
+		bool call_dequeue = false;
+
 		/*
 		 * Global and bypass DSQs are terminal - the task leaves the
-		 * scheduler's custody, so ops.dequeue() fires here. It can run
+		 * scheduler's custody, so ops.dequeue() fires. It can run
 		 * without @p's rq lock (finish_dispatch() passes the dispatch
 		 * rq); that's safe because dequeue_task_scx() waits on
 		 * SCX_OPSS_DISPATCHING (see the ops_state note above) and so
 		 * can't race it. A non-terminal DSQ keeps the task in custody.
+		 * The custody transfer happens under @dsq->lock so that
+		 * later consumers see the flag clear; the callback runs
+		 * unlocked - it must not run with a DSQ lock held.
 		 */
 		if (dsq->id == SCX_DSQ_GLOBAL || dsq->id == SCX_DSQ_BYPASS)
-			call_task_dequeue(sch, rq, p, 0);
+			call_dequeue = task_leave_custody(p);
 		else
 			p->scx.flags |= SCX_TASK_IN_CUSTODY;
 
 		raw_spin_unlock(&dsq->lock);
+
+		if (call_dequeue && SCX_HAS_OP(sch, dequeue))
+			SCX_CALL_OP_TASK(sch, dequeue, rq, p, 0);
 	}
 
 	/*
@@ -2373,11 +2389,13 @@ static void wakeup_preempt_scx(struct rq *rq, struct task_struct *p, int wake_fl
 		scx_schedule_reenq_local(rq, 0);
 }
 
-void scx_move_local_task_to_local_dsq(struct scx_sched *sch, struct task_struct *p,
-				      u64 enq_flags, struct scx_dispatch_q *src_dsq,
-				      struct rq *dst_rq)
+static struct scx_dispatch_q *
+__scx_move_local_task_to_local_dsq(struct scx_sched *sch,
+				   struct task_struct *p, u64 *enq_flags,
+				   struct scx_dispatch_q *src_dsq,
+				   struct rq *dst_rq)
 {
-	struct scx_dispatch_q *dst_dsq = scx_resolve_local_dsq(sch, dst_rq, p, &enq_flags);
+	struct scx_dispatch_q *dst_dsq = scx_resolve_local_dsq(sch, dst_rq, p, enq_flags);
 
 	/* @p is on @dst_rq, an rq-owned @src_dsq is covered by the rq lock */
 	if (!dsq_is_rq_owned(src_dsq))
@@ -2386,14 +2404,25 @@ void scx_move_local_task_to_local_dsq(struct scx_sched *sch, struct task_struct
 
 	WARN_ON_ONCE(p->scx.holding_cpu >= 0);
 
-	if (enq_flags & (SCX_ENQ_HEAD | SCX_ENQ_PREEMPT))
+	if (*enq_flags & (SCX_ENQ_HEAD | SCX_ENQ_PREEMPT))
 		dsq_insert_head(dst_dsq, p);
 	else
 		list_add_tail(&p->scx.dsq_list.node, &dst_dsq->list);
 
-	dsq_inc_nr(dst_dsq, p, enq_flags);
+	dsq_inc_nr(dst_dsq, p, *enq_flags);
 	p->scx.dsq = dst_dsq;
 
+	return dst_dsq;
+}
+
+void scx_move_local_task_to_local_dsq(struct scx_sched *sch, struct task_struct *p,
+				      u64 enq_flags, struct scx_dispatch_q *src_dsq,
+				      struct rq *dst_rq)
+{
+	struct scx_dispatch_q *dst_dsq;
+
+	dst_dsq = __scx_move_local_task_to_local_dsq(sch, p, &enq_flags,
+						     src_dsq, dst_rq);
 	rq_owned_post_enq(sch, dst_rq, dst_dsq, p, enq_flags);
 }
 
@@ -2628,9 +2657,14 @@ static struct rq *move_task_between_dsqs(struct scx_sched *sch,
 	if (dst_dsq->id == SCX_DSQ_LOCAL) {
 		/* @p is going from a non-local DSQ to a local DSQ */
 		if (src_rq == dst_rq) {
+			struct scx_dispatch_q *ldsq;
+
 			scx_task_unlink_from_dsq(p, src_dsq);
-			scx_move_local_task_to_local_dsq(sch, p, enq_flags, src_dsq, dst_rq);
+			ldsq = __scx_move_local_task_to_local_dsq(sch, p,
+								  &enq_flags,
+								  src_dsq, dst_rq);
 			raw_spin_unlock(&src_dsq->lock);
+			rq_owned_post_enq(sch, dst_rq, ldsq, p, enq_flags);
 		} else {
 			raw_spin_unlock(&src_dsq->lock);
 			move_remote_task_to_local_dsq(sch, p, enq_flags, src_rq, dst_rq);
@@ -2679,9 +2713,14 @@ bool scx_consume_dispatch_q(struct scx_sched *sch, struct rq *rq,
 			break;
 
 		if (rq == task_rq) {
+			struct scx_dispatch_q *ldsq;
+
 			scx_task_unlink_from_dsq(p, dsq);
-			scx_move_local_task_to_local_dsq(sch, p, enq_flags, dsq, rq);
+			ldsq = __scx_move_local_task_to_local_dsq(sch, p,
+								  &enq_flags,
+								  dsq, rq);
 			raw_spin_unlock(&dsq->lock);
+			rq_owned_post_enq(sch, rq, ldsq, p, enq_flags);
 			return true;
 		}
 
-- 
2.43.0

[PATCH v2 2/2] selftests/sched_ext: Test that ops.dequeue() can iterate the consumed DSQ

[Qiurong Fang]

From: fangqiurong <fangqiurong@kylinos.cn>

Add a scheduler whose ops.dequeue() iterates the user DSQ tasks are
dispatched from with bpf_iter_scx_dsq. The iteration takes the DSQ's raw
spinlock; on a kernel that runs ops.dequeue() while the consume path
still holds that lock, the first task consumed self-deadlocks the CPU
until the scheduler watchdog fires and the test fails with a UEI. On a
fixed kernel the scheduler runs clean and the test passes.

Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>
---
 tools/testing/selftests/sched_ext/Makefile    |  1 +
 .../selftests/sched_ext/dequeue_iter.bpf.c    | 65 +++++++++++++
 .../selftests/sched_ext/dequeue_iter.c        | 94 +++++++++++++++++++
 3 files changed, 160 insertions(+)
 create mode 100644 tools/testing/selftests/sched_ext/dequeue_iter.bpf.c
 create mode 100644 tools/testing/selftests/sched_ext/dequeue_iter.c

diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile
index 5f5dd9ab903a..08c2646cdd1a 100644
--- a/tools/testing/selftests/sched_ext/Makefile
+++ b/tools/testing/selftests/sched_ext/Makefile
@@ -164,6 +164,7 @@ all_test_bpfprogs := $(foreach prog,$(wildcard *.bpf.c),$(INCLUDE_DIR)/$(patsubs
 auto-test-targets :=			\
 	create_dsq			\
 	dequeue				\
+	dequeue_iter			\
 	enq_last_no_enq_fails		\
 	ddsp_bogus_dsq_fail		\
 	ddsp_vtimelocal_fail		\
diff --git a/tools/testing/selftests/sched_ext/dequeue_iter.bpf.c b/tools/testing/selftests/sched_ext/dequeue_iter.bpf.c
new file mode 100644
index 000000000000..406a767c31b2
--- /dev/null
+++ b/tools/testing/selftests/sched_ext/dequeue_iter.bpf.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ops.dequeue() of this scheduler iterates the user DSQ tasks are
+ * consumed from with bpf_iter_scx_dsq, which takes the DSQ lock.
+ * On a kernel that still runs ops.dequeue() with that lock held, the
+ * iteration self-deadlocks the CPU - this test wedges the system on
+ * unfixed kernels instead of failing cleanly.
+ *
+ * Copyright (c) 2026 fangqiurong <fangqiurong@kylinos.cn>
+ */
+
+#include <scx/common.bpf.h>
+
+char _license[] SEC("license") = "GPL";
+
+UEI_DEFINE(uei);
+
+#define TEST_DSQ_ID 1000
+
+u64 dq_count;
+
+s32 BPF_STRUCT_OPS_SLEEPABLE(dequeue_iter_init)
+{
+	return scx_bpf_create_dsq(TEST_DSQ_ID, -1);
+}
+
+void BPF_STRUCT_OPS(dequeue_iter_enqueue, struct task_struct *p, u64 enq_flags)
+{
+	scx_bpf_dsq_insert(p, TEST_DSQ_ID, SCX_SLICE_DFL, enq_flags);
+}
+
+void BPF_STRUCT_OPS(dequeue_iter_dispatch, s32 cpu, struct task_struct *task)
+{
+	scx_bpf_dsq_move_to_local(TEST_DSQ_ID, 0);
+}
+
+void BPF_STRUCT_OPS(dequeue_iter_dequeue, struct task_struct *p, u64 deq_flags)
+{
+	struct bpf_iter_scx_dsq it;
+	struct task_struct *t;
+
+	if (!bpf_iter_scx_dsq_new(&it, TEST_DSQ_ID, 0)) {
+		while ((t = bpf_iter_scx_dsq_next(&it)))
+			;
+	}
+	bpf_iter_scx_dsq_destroy(&it);
+
+	__sync_fetch_and_add(&dq_count, 1);
+}
+
+void BPF_STRUCT_OPS(dequeue_iter_exit, struct scx_exit_info *ei)
+{
+	scx_bpf_destroy_dsq(TEST_DSQ_ID);
+}
+
+SEC(".struct_ops.link")
+struct sched_ext_ops dequeue_iter_ops = {
+	.init			= (void *)dequeue_iter_init,
+	.enqueue		= (void *)dequeue_iter_enqueue,
+	.dispatch		= (void *)dequeue_iter_dispatch,
+	.dequeue		= (void *)dequeue_iter_dequeue,
+	.exit			= (void *)dequeue_iter_exit,
+	.timeout_ms		= 5000,
+	.name			= "dequeue_iter",
+};
diff --git a/tools/testing/selftests/sched_ext/dequeue_iter.c b/tools/testing/selftests/sched_ext/dequeue_iter.c
new file mode 100644
index 000000000000..35bc52cfc474
--- /dev/null
+++ b/tools/testing/selftests/sched_ext/dequeue_iter.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 fangqiurong <fangqiurong@kylinos.cn>
+ */
+#include <bpf/bpf.h>
+#include <pthread.h>
+#include <sched.h>
+#include <scx/common.h>
+#include <time.h>
+#include <unistd.h>
+#include "dequeue_iter.bpf.skel.h"
+#include "scx_test.h"
+
+#define DQ_TARGET	10
+#define DQ_DEADLINE_MS	3000
+
+static unsigned long long now_ms(void)
+{
+	struct timespec ts;
+
+	clock_gettime(CLOCK_MONOTONIC, &ts);
+
+	return ts.tv_sec * 1000ULL + ts.tv_nsec / 1000000;
+}
+
+static void *run_workload(void *arg)
+{
+	struct dequeue_iter *skel = arg;
+	unsigned long long end = now_ms() + DQ_DEADLINE_MS;
+
+	while (skel->bss->dq_count < DQ_TARGET && now_ms() < end)
+		usleep(100);
+
+	return NULL;
+}
+
+static enum scx_test_status setup(void **ctx)
+{
+	struct dequeue_iter *skel;
+
+	skel = dequeue_iter__open();
+	SCX_FAIL_IF(!skel, "Failed to open");
+	SCX_ENUM_INIT(skel);
+	SCX_FAIL_IF(dequeue_iter__load(skel), "Failed to load skel");
+
+	*ctx = skel;
+
+	return SCX_TEST_PASS;
+}
+
+static enum scx_test_status run(void *ctx)
+{
+	struct dequeue_iter *skel = ctx;
+	struct bpf_link *link;
+	pthread_t tid;
+
+	link = bpf_map__attach_struct_ops(skel->maps.dequeue_iter_ops);
+	SCX_FAIL_IF(!link, "Failed to attach scheduler");
+
+	SCX_FAIL_IF(pthread_create(&tid, NULL, run_workload, skel),
+		    "Failed to create workload thread");
+	pthread_join(tid, NULL);
+	bpf_link__destroy(link);
+
+	if (UEI_EXITED(skel, uei)) {
+		UEI_REPORT(skel, uei);
+		SCX_ERR("Scheduler exited unexpectedly\n");
+		return SCX_TEST_FAIL;
+	}
+
+	if (skel->bss->dq_count < DQ_TARGET) {
+		SCX_ERR("ops.dequeue() fired only %llu times\n",
+			(unsigned long long)skel->bss->dq_count);
+		return SCX_TEST_FAIL;
+	}
+
+	return SCX_TEST_PASS;
+}
+
+static void cleanup(void *ctx)
+{
+	struct dequeue_iter *skel = ctx;
+
+	dequeue_iter__destroy(skel);
+}
+
+struct scx_test dequeue_iter = {
+	.name = "dequeue_iter",
+	.description = "Verify ops.dequeue() can iterate its source user DSQ",
+	.setup = setup,
+	.run = run,
+	.cleanup = cleanup,
+};
+REGISTER_SCX_TEST(&dequeue_iter)
-- 
2.43.0

Re: [PATCH v2 1/2] sched_ext: Don't run ops.dequeue() with a DSQ lock held

[Tejun Heo]

Hello, Qiurong.

On Tue, Sep 15, 2026 at 09:03:33PM +0800, Qiurong Fang wrote:
> ops.dequeue() is called with the source user DSQ's lock still held on
> the consume and move paths (scx_consume_dispatch_q(),
> move_task_between_dsqs()) and with the terminal global/bypass DSQ's
> lock still held in scx_dispatch_enqueue(). A BPF scheduler that locks
> the same DSQ from ops.dequeue() - e.g. by iterating it with
> bpf_iter_scx_dsq, which takes the DSQ lock on every step -
> self-deadlocks.

The self-deadlock only exists on the two user DSQ paths. ops.dequeue() can
only call the "any" kfuncs and none of them can lock a builtin DSQ, so the
global/bypass path can't deadlock. Moving it out is still right because all
DSQ locks share one lockdep class and iterating any user DSQ from there
trips the recursion check. Please describe it that way.

> Move the invocation after the DSQ unlock on all three paths.
> SCX_TASK_IN_CUSTODY is cleared under the lock so that the callback is
> invoked exactly once; it is not ordered against consumption of the
> task and may run after the task has been moved to, or consumed from,
> a terminal DSQ.

The last sentence and the matching doc addition confuse more than they help.
The only thing that matters is that the task can't start running or be
re-enqueued before ops.dequeue() completes. Please state that invariant
instead or drop it.

That invariant currently has a hole which this patch widens. After the
unlock, the task sits in an unlocked DSQ with ops_state still DISPATCHING
until the final store, now for the whole callback, and the reenq paths don't
wait on DISPATCHING the way ops_dequeue() does. That's from ebf1ccff79c4 and
I'll fix it separately. No need to address it here.

>  		if (dsq->id == SCX_DSQ_GLOBAL || dsq->id == SCX_DSQ_BYPASS)
> -			call_task_dequeue(sch, rq, p, 0);
> +			call_dequeue = task_leave_custody(p);
>  		else
>  			p->scx.flags |= SCX_TASK_IN_CUSTODY;
>
>  		raw_spin_unlock(&dsq->lock);
> +
> +		if (call_dequeue && SCX_HAS_OP(sch, dequeue))
> +			SCX_CALL_OP_TASK(sch, dequeue, rq, p, 0);

This leaves call_task_dequeue() with two callers, both of which are these
two lines without the unlock in between. Please open-code them too and drop
call_task_dequeue(). Each ops.dequeue() invocation being explicit is better
than a helper which may or may not invoke it depending on a flag cleared
elsewhere.

> +static struct scx_dispatch_q *
> +__scx_move_local_task_to_local_dsq(struct scx_sched *sch,
> +				   struct task_struct *p, u64 *enq_flags,
> +				   struct scx_dispatch_q *src_dsq,
> +				   struct rq *dst_rq)

The split isn't necessary. Unlock @src_dsq right after
scx_task_unlink_from_dsq() in the two callers and call
scx_move_local_task_to_local_dsq() as is, dropping its lockdep assert on
@src_dsq. @p is off the DSQ and its rq is locked, so nothing can reach it in
between.

Also, please add Cc: stable # v7.1+.

Thanks.

--
tejun

Re: [PATCH v2 2/2] selftests/sched_ext: Test that ops.dequeue() can iterate the consumed DSQ

[Tejun Heo]

Hello, Qiurong.

On Tue, Sep 15, 2026 at 09:03:34PM +0800, Qiurong Fang wrote:
> Add a scheduler whose ops.dequeue() iterates the user DSQ tasks are
> dispatched from with bpf_iter_scx_dsq. The iteration takes the DSQ's raw
> spinlock; on a kernel that runs ops.dequeue() while the consume path
> still holds that lock, the first task consumed self-deadlocks the CPU
> until the scheduler watchdog fires and the test fails with a UEI. On a
> fixed kernel the scheduler runs clean and the test passes.

The watchdog can't recover from this. Both it and the disable path need the
wedged CPU's rq lock. The header comment's "wedges the system" is the
accurate description. Please make the two agree.

> +void BPF_STRUCT_OPS(dequeue_iter_enqueue, struct task_struct *p, u64 enq_flags)
> +{
> +	scx_bpf_dsq_insert(p, TEST_DSQ_ID, SCX_SLICE_DFL, enq_flags);
> +}

Without ops.select_cpu(), the default direct-dispatches every wakeup that
finds an idle CPU to the local DSQ, so the poller's own wakeups never reach
TEST_DSQ_ID and the pass condition depends on unrelated traffic. Add a
select_cpu() which returns prev_cpu so that everything flows through the DSQ.

> +void BPF_STRUCT_OPS(dequeue_iter_exit, struct scx_exit_info *ei)
> +{
> +	scx_bpf_destroy_dsq(TEST_DSQ_ID);
> +}

Without UEI_RECORD() here, UEI_EXITED() is never true and the failure branch
in run() is dead. Record it, break the poll loop on UEI_EXITED(), and check
for SCX_EXIT_UNREG after destroying the link like the other tests do.

SCX_ERR() already appends the newline.

Thanks.

--
tejun