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

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

[Qiurong Fang]

From: fangqiurong <fangqiurong@kylinos.cn>

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

v2 -> v3:
- Restrict the deadlock description to the two user DSQ paths; keep
  the global/bypass move for the shared DSQ lockdep class (Tejun Heo)
- Drop the ordering sentence and the doc change until the DISPATCHING
  reenq hole is fixed (Tejun Heo)
- Open-code the ops.dequeue() invocations and drop call_task_dequeue()
  (Tejun Heo)
- Drop the __scx_move_local_task_to_local_dsq() split; unlock @src_dsq
  right after unlinking (Tejun Heo)
- selftest: add ops.select_cpu(), record UEI and check SCX_EXIT_UNREG,
  fix the watchdog wording (Tejun Heo)
- selftest: set ops.timeout_ms to the suite-standard 1000U
- Add Cc: stable # v7.1+ (Tejun Heo)

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/20260915130334.714388-1-fangqiurong@kylinos.cn/

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

 kernel/sched/ext/ext.c                        | 41 ++++-----
 tools/testing/selftests/sched_ext/Makefile    |  1 +
 .../selftests/sched_ext/dequeue_iter.bpf.c    | 73 +++++++++++++++
 .../selftests/sched_ext/dequeue_iter.c        | 90 +++++++++++++++++++
 4 files changed, 185 insertions(+), 20 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 v3 1/2] sched_ext: Don't run ops.dequeue() with a DSQ lock held

[Qiurong Fang]

From: fangqiurong <fangqiurong@kylinos.cn>

ops.dequeue() is invoked with the source user DSQ's lock still held on
the consume and move paths (scx_consume_dispatch_q(),
move_task_between_dsqs()). A BPF scheduler which locks the source user
DSQ from ops.dequeue() - e.g. by iterating it with bpf_iter_scx_dsq -
self-deadlocks.

ops.dequeue() can only call the "any" kfuncs and none of them can lock a
builtin DSQ, so the global and bypass paths can't deadlock; however,
all DSQ locks share one lockdep class, so iterating any user DSQ from
ops.dequeue() there trips the recursion check.

Move the invocation after the DSQ unlock on all three paths.
SCX_TASK_IN_CUSTODY is cleared under the lock serializing the transfer
so that the callback is invoked exactly once.

Fixes: ebf1ccff79c4 ("sched_ext: Fix ops.dequeue() semantics")
Cc: stable@vger.kernel.org # v7.1+
Acked-by: Andrea Righi <arighi@nvidia.com>
Signed-off-by: fangqiurong <fangqiurong@kylinos.cn>
---
 kernel/sched/ext/ext.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 40fa1697bdb7..b3aa2fb729a2 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -1499,27 +1499,22 @@ static inline bool task_scx_migrating(struct task_struct *p)
 	return p->scx.sticky_cpu >= 0;
 }
 
-/*
- * Call ops.dequeue() if the task is in BPF custody and not migrating.
- * Clears %SCX_TASK_IN_CUSTODY when the callback is invoked.
- */
-static void call_task_dequeue(struct scx_sched *sch, struct rq *rq,
-			      struct task_struct *p, u64 deq_flags)
+/* 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;
-
-	if (SCX_HAS_OP(sch, dequeue))
-		SCX_CALL_OP_TASK(sch, dequeue, rq, p, deq_flags);
+		return false;
 
 	p->scx.flags &= ~SCX_TASK_IN_CUSTODY;
+	return true;
 }
 
 static void rq_owned_post_enq(struct scx_sched *sch, struct rq *rq,
 			      struct scx_dispatch_q *dsq, struct task_struct *p,
 			      u64 enq_flags)
 {
-	call_task_dequeue(sch, rq, p, 0);
+	if (task_leave_custody(p) && SCX_HAS_OP(sch, dequeue))
+		SCX_CALL_OP_TASK(sch, dequeue, rq, p, 0);
 
 	/*
 	 * Only local inserts get the wakeup treatment below. Rejects kick the
@@ -1705,20 +1700,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);
 	}
 
 	/*
@@ -2210,7 +2213,7 @@ static void ops_dequeue(struct rq *rq, struct task_struct *p, u64 deq_flags)
 		/*
 		 * A queued task must always be in BPF scheduler's custody. If
 		 * SCX_TASK_IN_CUSTODY is clear, finish_dispatch() on another
-		 * CPU has already passed call_task_dequeue() (which clears the
+		 * CPU has already passed task_leave_custody() (which clears the
 		 * flag), but has not yet written SCX_OPSS_NONE. That final
 		 * store does not require this rq's lock, so retrying with
 		 * cpu_relax() is bounded: we will observe NONE (or DISPATCHING,
@@ -2258,7 +2261,8 @@ static void ops_dequeue(struct rq *rq, struct task_struct *p, u64 deq_flags)
 	 * NONE but the task may still have %SCX_TASK_IN_CUSTODY set until
 	 * it is enqueued on the destination.
 	 */
-	call_task_dequeue(sch, rq, p, deq_flags);
+	if (task_leave_custody(p) && SCX_HAS_OP(sch, dequeue))
+		SCX_CALL_OP_TASK(sch, dequeue, rq, p, deq_flags);
 }
 
 static bool dequeue_task_scx(struct rq *rq, struct task_struct *p, int core_deq_flags)
@@ -2379,9 +2383,6 @@ void scx_move_local_task_to_local_dsq(struct scx_sched *sch, struct task_struct
 {
 	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))
-		lockdep_assert_held(&src_dsq->lock);
 	lockdep_assert_rq_held(dst_rq);
 
 	WARN_ON_ONCE(p->scx.holding_cpu >= 0);
@@ -2629,8 +2630,8 @@ static struct rq *move_task_between_dsqs(struct scx_sched *sch,
 		/* @p is going from a non-local DSQ to a local DSQ */
 		if (src_rq == dst_rq) {
 			scx_task_unlink_from_dsq(p, src_dsq);
-			scx_move_local_task_to_local_dsq(sch, p, enq_flags, src_dsq, dst_rq);
 			raw_spin_unlock(&src_dsq->lock);
+			scx_move_local_task_to_local_dsq(sch, p, enq_flags, src_dsq, dst_rq);
 		} else {
 			raw_spin_unlock(&src_dsq->lock);
 			move_remote_task_to_local_dsq(sch, p, enq_flags, src_rq, dst_rq);
@@ -2680,8 +2681,8 @@ bool scx_consume_dispatch_q(struct scx_sched *sch, struct rq *rq,
 
 		if (rq == task_rq) {
 			scx_task_unlink_from_dsq(p, dsq);
-			scx_move_local_task_to_local_dsq(sch, p, enq_flags, dsq, rq);
 			raw_spin_unlock(&dsq->lock);
+			scx_move_local_task_to_local_dsq(sch, p, enq_flags, dsq, rq);
 			return true;
 		}
 
-- 
2.43.0

[PATCH v3 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 source user DSQ 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 with IRQs
disabled. The watchdog cannot recover from that state, so on an
unfixed kernel this test wedges the system instead of failing cleanly.
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    | 73 +++++++++++++++
 .../selftests/sched_ext/dequeue_iter.c        | 90 +++++++++++++++++++
 3 files changed, 164 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..7f23b0a8af7f
--- /dev/null
+++ b/tools/testing/selftests/sched_ext/dequeue_iter.bpf.c
@@ -0,0 +1,73 @@
+// 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);
+}
+
+s32 BPF_STRUCT_OPS(dequeue_iter_select_cpu, struct task_struct *p,
+		   s32 prev_cpu, u64 wake_flags)
+{
+	return prev_cpu;
+}
+
+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)
+{
+	UEI_RECORD(uei, ei);
+	scx_bpf_destroy_dsq(TEST_DSQ_ID);
+}
+
+SEC(".struct_ops.link")
+struct sched_ext_ops dequeue_iter_ops = {
+	.init			= (void *)dequeue_iter_init,
+	.select_cpu		= (void *)dequeue_iter_select_cpu,
+	.enqueue		= (void *)dequeue_iter_enqueue,
+	.dispatch		= (void *)dequeue_iter_dispatch,
+	.dequeue		= (void *)dequeue_iter_dequeue,
+	.exit			= (void *)dequeue_iter_exit,
+	.timeout_ms		= 1000U,
+	.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..bcd12fc6c781
--- /dev/null
+++ b/tools/testing/selftests/sched_ext/dequeue_iter.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 fangqiurong <fangqiurong@kylinos.cn>
+ */
+#include <bpf/bpf.h>
+#include <pthread.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 && !UEI_EXITED(skel, uei) &&
+	       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);
+
+	SCX_EQ(skel->data->uei.kind, EXIT_KIND(SCX_EXIT_UNREG));
+
+	if (skel->bss->dq_count < DQ_TARGET) {
+		SCX_ERR("ops.dequeue() fired only %llu times",
+			(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 v3 1/2] sched_ext: Don't run ops.dequeue() with a DSQ lock held

[Tejun Heo]

Hello, Qiurong.

On Wed, Sep 16, 2026 at 03:07:52PM +0800, Qiurong Fang wrote:
> ops.dequeue() can only call the "any" kfuncs and none of them can lock a
> builtin DSQ, so the global and bypass paths can't deadlock; however,
> all DSQ locks share one lockdep class, so iterating any user DSQ from
> ops.dequeue() there trips the recursion check.

"there" has no referent. "on those paths" or similar.

> +		 * 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.

The callback isn't unlocked. The rq lock is held on every path. Only
@dsq->lock is dropped. Say that it runs after @dsq->lock is dropped because
it may lock a DSQ itself.

> -	/* @p is on @dst_rq, an rq-owned @src_dsq is covered by the rq lock */
> -	if (!dsq_is_rq_owned(src_dsq))
> -		lockdep_assert_held(&src_dsq->lock);

This leaves @src_dsq unused in scx_move_local_task_to_local_dsq(). Please
drop the parameter from the function and its callers, including the three
in sub.c.

Thanks.

--
tejun

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

[Tejun Heo]

Hello, Qiurong.

On Wed, Sep 16, 2026 at 03:07:53PM +0800, Qiurong Fang wrote:
> + * ops.dequeue() of this scheduler iterates the user DSQ tasks are
> + * consumed from with bpf_iter_scx_dsq, which takes the DSQ lock.

"the user DSQ it consumes tasks from" reads easier.

> +	SCX_FAIL_IF(pthread_create(&tid, NULL, run_workload, skel),
> +		    "Failed to create workload thread");
> +	pthread_join(tid, NULL);

The thread doesn't add anything. Poll from run() directly. Its own wakeups
are the workload.

Thanks.

--
tejun