mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs
@ 2026-09-24 14:36 Wanwu Li
  2026-09-24 14:36 ` [PATCH 1/3] tools/sched_ext: Increment nr_queued when a task is queued to user space Wanwu Li
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Wanwu Li @ 2026-09-24 14:36 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
  Cc: sched-ext, linux-kernel, Wanwu Li

Patch 1 fixes scx_userland, where the BPF component never increments
nr_queued although both its declaration comment and the NOTE in
userland_update_idle() say it does.  As a result the "wake the
user-space scheduler when a CPU goes idle" decision silently degrades
to nr_scheduled alone and tasks sitting in the enqueued map are
invisible to it.

Patches 2 and 3 fix two related queue-lifetime bugs in scx_pair, which
has no .dequeue and no .cgroup_move callback and therefore leaves stale
pids in the per-cgroup FIFOs:

 - pair_cgroup_exit() releases a queue slot without draining it.  The
   next cgroup inheriting the slot starts with a stale non-zero
   cgrp_q_len, so its first enqueue never sees the 0 -> 1 transition
   and its cgid is never queued on top_q: the cgroup starves for the
   lifetime of the scheduler.

 - try_dispatch() trusts a pid popped from a cgroup's FIFO.  If the
   task migrated to another cgroup between enqueue and dispatch, it is
   dispatched under the old cgroup's context, violating the core
   "paired CPUs only run tasks from the same cgroup" invariant.

Wanwu Li (3):
  tools/sched_ext: Increment nr_queued when a task is queued to user
    space
  tools/sched_ext: scx_pair: Drain task queue on cgroup exit
  tools/sched_ext: scx_pair: Verify task cgroup before dispatch

 tools/sched_ext/scx_pair.bpf.c     | 47 ++++++++++++++++++++++++++++++++++++++
 tools/sched_ext/scx_userland.bpf.c |  1 +
 2 files changed, 48 insertions(+)

-- 
2.25.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] tools/sched_ext: Increment nr_queued when a task is queued to user space
  2026-09-24 14:36 [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Wanwu Li
@ 2026-09-24 14:36 ` Wanwu Li
  2026-09-24 14:36 ` [PATCH 2/3] tools/sched_ext: scx_pair: Drain task queue on cgroup exit Wanwu Li
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Wanwu Li @ 2026-09-24 14:36 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
  Cc: sched-ext, linux-kernel, Wanwu Li

userland_update_idle() decides whether a CPU going idle must wake the
user-space scheduler with "if (nr_queued || nr_scheduled)", where
nr_queued is documented -- both in its declaration and in the NOTE in
userland_update_idle() -- to be incremented by the BPF component when
a task is queued to user space.  But enqueue_task_in_user_space()
never increments it: a successful push only bumps the nr_user_enqueues
stats counter, so nr_queued stays 0 forever and the check silently
degrades to nr_scheduled alone.  Tasks sitting in the enqueued map,
not yet drained by the user-space scheduler, are invisible to a CPU
going idle.

Increment nr_queued when the push succeeds; the user-space side
already clears it once the map is drained, as documented.

Fixes: cc4448d0856d ("tools/sched_ext: add scx_userland scheduler")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
 tools/sched_ext/scx_userland.bpf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/sched_ext/scx_userland.bpf.c b/tools/sched_ext/scx_userland.bpf.c
index f29862b89386..648c6bd230eb 100644
--- a/tools/sched_ext/scx_userland.bpf.c
+++ b/tools/sched_ext/scx_userland.bpf.c
@@ -197,6 +197,7 @@ static void enqueue_task_in_user_space(struct task_struct *p, u64 enq_flags)
 		scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, enq_flags);
 	} else {
 		__sync_fetch_and_add(&nr_user_enqueues, 1);
+		__sync_fetch_and_add(&nr_queued, 1);
 		set_usersched_needed();
 	}
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] tools/sched_ext: scx_pair: Drain task queue on cgroup exit
  2026-09-24 14:36 [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Wanwu Li
  2026-09-24 14:36 ` [PATCH 1/3] tools/sched_ext: Increment nr_queued when a task is queued to user space Wanwu Li
@ 2026-09-24 14:36 ` Wanwu Li
  2026-09-24 14:36 ` [PATCH 3/3] tools/sched_ext: scx_pair: Verify task cgroup before dispatch Wanwu Li
  2026-09-24 16:17 ` [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Wanwu Li @ 2026-09-24 14:36 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
  Cc: sched-ext, linux-kernel, Wanwu Li

pair_cgroup_exit() releases a cgroup's queue slot by clearing the
busy marker and deleting the cgid hash entry, but leaves the task
queue (cgrp_q_arr) and its length counter (cgrp_q_len) alone.  The
slot is handed to the next cgroup through pair_cgroup_init() without
any reset, so if the outgoing cgroup left entries behind -- tasks
which exited or migrated away after being enqueued, which scx_pair
never dequeues -- the inheriting cgroup starts with a non-zero
cgrp_q_len.  pair_enqueue() only queues a cgroup on top_q on the
0 -> 1 transition of that counter, so the new cgroup never reaches
top_q and none of its tasks is ever dispatched: the cgroup starves
for the lifetime of the scheduler.

Fix it by draining the queue and taking the length counter to zero
with it, claiming each entry just as try_dispatch() does before
popping so that the drain cannot race an in-flight dispatcher into
a fatal pop failure.  All pids left behind are stale so dropping
them is safe.

Fixes: f0262b102c7c ("tools/sched_ext: add scx_pair scheduler")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
 tools/sched_ext/scx_pair.bpf.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tools/sched_ext/scx_pair.bpf.c b/tools/sched_ext/scx_pair.bpf.c
index 0d61b7b812db..a1fe67fdaf5c 100644
--- a/tools/sched_ext/scx_pair.bpf.c
+++ b/tools/sched_ext/scx_pair.bpf.c
@@ -633,6 +633,34 @@ void BPF_STRUCT_OPS(pair_cgroup_exit, struct cgroup *cgrp)
 
 	q_idx = bpf_map_lookup_elem(&cgrp_q_idx_hash, &cgid);
 	if (q_idx) {
+		struct cgrp_q *cgq;
+		s32 pid;
+		u64 *cgq_len;
+
+		/*
+		 * All tasks have left the cgroup by the time it exits, so
+		 * the pids left behind are stale; drain the queue and take
+		 * the length counter down with them -- the next cgroup
+		 * inheriting this slot must see its first enqueue go
+		 * 0 -> 1 or it never reaches top_q.  Each pop claims a
+		 * counter slot as try_dispatch() does, so that the drain
+		 * can't steal an entry from an in-flight dispatcher and
+		 * trip its scx_bpf_error().
+		 */
+		cgq = bpf_map_lookup_elem(&cgrp_q_arr, q_idx);
+		cgq_len = MEMBER_VPTR(cgrp_q_len, [*q_idx]);
+		if (cgq && cgq_len)
+			bpf_repeat(BPF_MAX_LOOPS) {
+				u64 len = *(volatile u64 *)cgq_len;
+
+				if (!len)
+					break;
+				if (__sync_val_compare_and_swap(cgq_len, len, len - 1) != len)
+					continue;
+				if (bpf_map_pop_elem(cgq, &pid))
+					break;
+			}
+
 		u64 *busy = MEMBER_VPTR(cgrp_q_idx_busy, [*q_idx]);
 		if (busy)
 			*busy = 0;
-- 
2.25.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] tools/sched_ext: scx_pair: Verify task cgroup before dispatch
  2026-09-24 14:36 [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Wanwu Li
  2026-09-24 14:36 ` [PATCH 1/3] tools/sched_ext: Increment nr_queued when a task is queued to user space Wanwu Li
  2026-09-24 14:36 ` [PATCH 2/3] tools/sched_ext: scx_pair: Drain task queue on cgroup exit Wanwu Li
@ 2026-09-24 14:36 ` Wanwu Li
  2026-09-24 16:17 ` [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Wanwu Li @ 2026-09-24 14:36 UTC (permalink / raw)
  To: Tejun Heo, David Vernet, Andrea Righi, Changwoo Min
  Cc: sched-ext, linux-kernel, Wanwu Li

pair_enqueue() pushes a task's pid into the FIFO for the cgroup it
was in at enqueue time.  scx_pair has no .dequeue callback and no
.cgroup_move callback, so when a task migrates to another cgroup
between enqueue and dispatch, its stale pid remains in the old
cgroup's FIFO.

try_dispatch() pops such a stale pid, bpf_task_from_pid() returns
the (alive) task, and scx_bpf_dsq_insert() dispatches it under the
old cgroup's context.  This violates the pair scheduler's core
invariant: paired CPUs must only run tasks from the same cgroup.

Fix it by reading the task's current cgroup after
bpf_task_from_pid() and comparing it against the cgid the pair is
dispatching for.  A mismatch means the task migrated away; drop it
and retry, mirroring the existing lost-task path.

Fixes: f0262b102c7c ("tools/sched_ext: add scx_pair scheduler")
Signed-off-by: Wanwu Li <liwanwu@kylinos.cn>
---
 tools/sched_ext/scx_pair.bpf.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tools/sched_ext/scx_pair.bpf.c b/tools/sched_ext/scx_pair.bpf.c
index 0d61b7b812db..05658d5dc796 100644
--- a/tools/sched_ext/scx_pair.bpf.c
+++ b/tools/sched_ext/scx_pair.bpf.c
@@ -519,6 +519,25 @@ static int try_dispatch(s32 cpu)
 
 	p = bpf_task_from_pid(pid);
 	if (p) {
+		struct cgroup *task_cgrp;
+		u64 task_cgid;
+
+		/*
+		 * Without a .dequeue callback, a task that migrated to
+		 * another cgroup after being enqueued leaves a stale pid
+		 * behind. Dispatching it here would run it under the wrong
+		 * cgroup. Drop it and retry.
+		 */
+		task_cgrp = scx_bpf_task_cgroup(p);
+		task_cgid = task_cgrp->kn->id;
+		bpf_cgroup_release(task_cgrp);
+
+		if (task_cgid != cgid) {
+			bpf_task_release(p);
+			__sync_fetch_and_add(&nr_missing, 1);
+			return -EAGAIN;
+		}
+
 		__sync_fetch_and_add(&nr_dispatched, 1);
 		scx_bpf_dsq_insert(p, SCX_DSQ_GLOBAL, SCX_SLICE_DFL, 0);
 		bpf_task_release(p);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs
  2026-09-24 14:36 [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Wanwu Li
                   ` (2 preceding siblings ...)
  2026-09-24 14:36 ` [PATCH 3/3] tools/sched_ext: scx_pair: Verify task cgroup before dispatch Wanwu Li
@ 2026-09-24 16:17 ` Tejun Heo
  3 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2026-09-24 16:17 UTC (permalink / raw)
  To: Wanwu Li
  Cc: David Vernet, Andrea Righi, Changwoo Min, sched-ext, linux-kernel

Hello, Wanwu.

On Thu, Sep 24, 2026 at 10:36:56PM +0800, Wanwu Li wrote:
>  - try_dispatch() trusts a pid popped from a cgroup's FIFO.  If the
>    task migrated to another cgroup between enqueue and dispatch, it is
>    dispatched under the old cgroup's context, violating the core
>    "paired CPUs only run tasks from the same cgroup" invariant.

3/3 calls scx_bpf_task_cgroup() on a task from bpf_task_from_pid(). The
kfunc only accepts the tasks passed to the current op, so the scheduler
is disabled on its first dispatch:

  sched_ext: BPF scheduler "pair" enabled
  sched_ext: BPF scheduler "pair" disabled (runtime error)
  sched_ext: pair: called on a task not being operated on
     scx_bpf_task_cgroup+0x15b/0x160
     bpf__sched_ext_ops_dispatch+0x47/0xa3

This means the patches were posted without being run at all. That's not
acceptable. Please reproduce each bug and verify that the fix resolves it
before posting.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-24 16:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24 14:36 [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Wanwu Li
2026-09-24 14:36 ` [PATCH 1/3] tools/sched_ext: Increment nr_queued when a task is queued to user space Wanwu Li
2026-09-24 14:36 ` [PATCH 2/3] tools/sched_ext: scx_pair: Drain task queue on cgroup exit Wanwu Li
2026-09-24 14:36 ` [PATCH 3/3] tools/sched_ext: scx_pair: Verify task cgroup before dispatch Wanwu Li
2026-09-24 16:17 ` [PATCH 0/3] tools/sched_ext: Fix three example scheduler bugs Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®