BPF List
 help / color / mirror / Atom feed
* [PATCH 0/2] sched_ext: Fix entry-path contract breaks in core.c
@ 2026-11-12 14:49 Tao Cui
  2026-11-12 14:49 ` [PATCH 1/2] sched/core: Pass ENQUEUE_RQ_SELECTED on initial enqueue Tao Cui
  2026-11-12 14:49 ` [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext Tao Cui
  0 siblings, 2 replies; 10+ messages in thread
From: Tao Cui @ 2026-11-12 14:49 UTC (permalink / raw)
  To: tj
  Cc: void, arighi, changwoo, mingo, peterz, sched-ext, linux-kernel,
	bpf, cui.tao, Tao Cui

From: Tao Cui <cuitao@kylinos.cn>

Hello,

wake_up_new_task() consults select_task_rq() but activates the new task
without ENQUEUE_RQ_SELECTED, so on the fork path ops.enqueue() never
sees SCX_ENQ_CPU_SELECTED and SCX_EV_SELECT_CPU_FALLBACK never counts a
fork-path fallback; pass the flag on the initial enqueue to match the
wakeup path. rt_mutex_setprio() is the only entry path into sched_ext
that does not call sched_proxy_block_task(): an ext task PI-boosted by
an rt waiter and blocked on a second kernel mutex is routed back to ext
on deboost carrying the queued+blocked state that
scx_prepare_setscheduler() promises never happens; block a retained
donor before the deboost routes it back.

0001 passes the flag on the initial enqueue to match the wakeup path,
and 0002 blocks a retained donor before the deboost routes it back to
ext.

Verified in qemu on tj/sched_ext for-next (5b740a72b55c), same kernel
and config with and without each patch, comparing stock and patched
binaries; the touched core.c regions are byte-identical on this base,
so the runs carry over: with a probe forcing select_cpu() fallbacks,
40k forks produce 0 fallback events on the baseline kernel while the
wakeup side counts normally, and fork and wakeup paths both count 1:1
with patch 1; and a driver-model trigger hands the scheduler ~39966
queued+blocked donors across 40000 PI cycles on the baseline kernel
with 0 with patch 2, PI chain traffic unchanged.

Based on sched_ext/for-next (5b740a72b55c).

Tao Cui (2):
  sched/core: Pass ENQUEUE_RQ_SELECTED on initial enqueue
  sched_ext: Block a retained donor before PI deboost routes it back to
    sched_ext

 kernel/sched/core.c    | 17 ++++++++++++++++-
 kernel/sched/ext/ext.h |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.53.0

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

* [PATCH 1/2] sched/core: Pass ENQUEUE_RQ_SELECTED on initial enqueue
  2026-11-12 14:49 [PATCH 0/2] sched_ext: Fix entry-path contract breaks in core.c Tao Cui
@ 2026-11-12 14:49 ` Tao Cui
  2026-11-12 14:49 ` [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext Tao Cui
  1 sibling, 0 replies; 10+ messages in thread
From: Tao Cui @ 2026-11-12 14:49 UTC (permalink / raw)
  To: tj
  Cc: void, arighi, changwoo, mingo, peterz, sched-ext, linux-kernel,
	bpf, cui.tao, Tao Cui

From: Tao Cui <cuitao@kylinos.cn>

select_task_rq() sets WF_RQ_SELECTED and ttwu_do_activate() maps it to
ENQUEUE_RQ_SELECTED so enqueue_task_scx() knows whether ops.select_cpu()
ran. wake_up_new_task() also calls select_task_rq() but activates the
new task without the flag, so on the fork path ops.enqueue() never sees
SCX_ENQ_CPU_SELECTED and SCX_EV_SELECT_CPU_FALLBACK never counts a
fork-path fallback: a misbehaving select_cpu() leaves no trace in the
counters, and schedulers testing SCX_ENQ_CPU_SELECTED run a redundant
select_cpu_dfl() on tasks select_cpu() already placed.

Set ENQUEUE_RQ_SELECTED on the initial enqueue when select_task_rq() was
called, matching the wakeup path. Only sched_ext consumes the flag.

Fixes: 9b671793c7d9 ("sched_ext, scx_qmap: Add and use SCX_ENQ_CPU_SELECTED")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 kernel/sched/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 04b62c16fa8c..f7a452123ef5 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5016,6 +5016,7 @@ void wake_up_new_task(struct task_struct *p)
 	struct rq_flags rf;
 	struct rq *rq;
 	int wake_flags = WF_FORK;
+	int en_flags = ENQUEUE_NOCLOCK | ENQUEUE_INITIAL;
 
 	raw_spin_lock_irqsave(&p->pi_lock, rf.flags);
 	WRITE_ONCE(p->__state, TASK_RUNNING);
@@ -5029,11 +5030,13 @@ void wake_up_new_task(struct task_struct *p)
 	 */
 	p->recent_used_cpu = task_cpu(p);
 	__set_task_cpu(p, select_task_rq(p, task_cpu(p), &wake_flags));
+	if (wake_flags & WF_RQ_SELECTED)
+		en_flags |= ENQUEUE_RQ_SELECTED;
 	rq = __task_rq_lock(p, &rf);
 	update_rq_clock(rq);
 	post_init_entity_util_avg(p);
 
-	activate_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_INITIAL);
+	activate_task(rq, p, en_flags);
 	trace_sched_wakeup_new(p);
 	wakeup_preempt(rq, p, wake_flags);
 	if (p->sched_class->task_woken) {
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

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

* [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 14:49 [PATCH 0/2] sched_ext: Fix entry-path contract breaks in core.c Tao Cui
  2026-11-12 14:49 ` [PATCH 1/2] sched/core: Pass ENQUEUE_RQ_SELECTED on initial enqueue Tao Cui
@ 2026-11-12 14:49 ` Tao Cui
  2026-11-12 15:21   ` Peter Zijlstra
  1 sibling, 1 reply; 10+ messages in thread
From: Tao Cui @ 2026-11-12 14:49 UTC (permalink / raw)
  To: tj
  Cc: void, arighi, changwoo, mingo, peterz, sched-ext, linux-kernel,
	bpf, cui.tao, Tao Cui

From: Tao Cui <cuitao@kylinos.cn>

rt_mutex_setprio() is the only entry path into sched_ext that does not
block a retained donor: an ext task PI-boosted by an rt waiter can
block on a second kernel mutex while boosted, and on deboost is routed
back to sched_ext carrying the queued+blocked state that
scx_prepare_setscheduler() promises never happens; every PI cycle
hands the BPF scheduler a queued+blocked donor it never admitted.

Call scx_prepare_task_sched_change() before the sched_change guard when
the class change routes the task back to sched_ext, which performs the
missing sched_proxy_block_task() (plus an update_rq_clock() that is
harmless here: the clock was already advanced, so the second advance
only applies a small delta). The boost direction (ext -> rt) needs no
handling.

Fixes: be8be8acb268 ("sched_ext: Block proxy donors before taking control")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 kernel/sched/core.c    | 12 ++++++++++++
 kernel/sched/ext/ext.h |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f7a452123ef5..dcb16b006740 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7816,6 +7816,18 @@ void rt_mutex_setprio(struct task_struct *p, struct task_struct *pi_task)
 	if (prev_class != next_class)
 		queue_flag |= DEQUEUE_CLASS;
 
+	/*
+	 * PI boost can route @p out of sched_ext: policy stays SCHED_EXT but
+	 * __setscheduler_class() follows the effective rt prio, and rt/fair
+	 * may have retained @p on the rq as a blocked donor. Block it before
+	 * sched_change_begin() records the queued state, the same way the
+	 * other entry points into sched_ext do.
+	 */
+#ifdef CONFIG_SCHED_CLASS_EXT
+	if ((queue_flag & DEQUEUE_CLASS) && next_class == &ext_sched_class)
+		scx_prepare_task_sched_change(p);
+#endif
+
 	scoped_guard (sched_change, p, queue_flag) {
 		/*
 		 * Boosting condition are:
diff --git a/kernel/sched/ext/ext.h b/kernel/sched/ext/ext.h
index d32bc01071b0..1a699be25331 100644
--- a/kernel/sched/ext/ext.h
+++ b/kernel/sched/ext/ext.h
@@ -19,6 +19,7 @@ void scx_rq_activate(struct rq *rq);
 void scx_rq_deactivate(struct rq *rq);
 int scx_check_setscheduler(struct task_struct *p, int policy);
 void scx_prepare_setscheduler(struct task_struct *p, int policy);
+void scx_prepare_task_sched_change(struct task_struct *p);
 bool task_should_scx(int policy);
 bool scx_allow_ttwu_queue(const struct task_struct *p);
 bool __scx_allow_proxy_exec(const struct task_struct *p);
@@ -72,6 +73,7 @@ static inline void scx_rq_activate(struct rq *rq) {}
 static inline void scx_rq_deactivate(struct rq *rq) {}
 static inline int scx_check_setscheduler(struct task_struct *p, int policy) { return 0; }
 static inline void scx_prepare_setscheduler(struct task_struct *p, int policy) {}
+static inline void scx_prepare_task_sched_change(struct task_struct *p) {}
 static inline bool task_on_scx(const struct task_struct *p) { return false; }
 static inline bool scx_allow_ttwu_queue(const struct task_struct *p) { return true; }
 static inline bool scx_allow_proxy_exec(const struct task_struct *p) { return true; }
-- 
2.53.0

base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 14:49 ` [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext Tao Cui
@ 2026-11-12 15:21   ` Peter Zijlstra
  2026-11-12 16:01     ` Peter Zijlstra
  2026-11-12 16:05     ` Andrea Righi
  0 siblings, 2 replies; 10+ messages in thread
From: Peter Zijlstra @ 2026-11-12 15:21 UTC (permalink / raw)
  To: Tao Cui
  Cc: tj, void, arighi, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

On Thu, Nov 12, 2026 at 10:49:35PM +0800, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> rt_mutex_setprio() is the only entry path into sched_ext that does not
> block a retained donor: an ext task PI-boosted by an rt waiter can
> block on a second kernel mutex while boosted, and on deboost is routed
> back to sched_ext carrying the queued+blocked state that
> scx_prepare_setscheduler() promises never happens; every PI cycle
> hands the BPF scheduler a queued+blocked donor it never admitted.
> 
> Call scx_prepare_task_sched_change() before the sched_change guard when
> the class change routes the task back to sched_ext, which performs the
> missing sched_proxy_block_task() (plus an update_rq_clock() that is
> harmless here: the clock was already advanced, so the second advance
> only applies a small delta). The boost direction (ext -> rt) needs no
> handling.

I can't understand this. And I take a very dim view of sprinking magic
hooks like this. So no.

The little I understand of things makes me think __setscheduler_class()
is buggy for EXT and *that* should be fixed.

> Fixes: be8be8acb268 ("sched_ext: Block proxy donors before taking control")
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>  kernel/sched/core.c    | 12 ++++++++++++
>  kernel/sched/ext/ext.h |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f7a452123ef5..dcb16b006740 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -7816,6 +7816,18 @@ void rt_mutex_setprio(struct task_struct *p, struct task_struct *pi_task)
>  	if (prev_class != next_class)
>  		queue_flag |= DEQUEUE_CLASS;
>  
> +	/*
> +	 * PI boost can route @p out of sched_ext: policy stays SCHED_EXT but
> +	 * __setscheduler_class() follows the effective rt prio, and rt/fair
> +	 * may have retained @p on the rq as a blocked donor. Block it before
> +	 * sched_change_begin() records the queued state, the same way the
> +	 * other entry points into sched_ext do.
> +	 */
> +#ifdef CONFIG_SCHED_CLASS_EXT
> +	if ((queue_flag & DEQUEUE_CLASS) && next_class == &ext_sched_class)
> +		scx_prepare_task_sched_change(p);
> +#endif
> +
>  	scoped_guard (sched_change, p, queue_flag) {
>  		/*
>  		 * Boosting condition are:
> diff --git a/kernel/sched/ext/ext.h b/kernel/sched/ext/ext.h
> index d32bc01071b0..1a699be25331 100644
> --- a/kernel/sched/ext/ext.h
> +++ b/kernel/sched/ext/ext.h
> @@ -19,6 +19,7 @@ void scx_rq_activate(struct rq *rq);
>  void scx_rq_deactivate(struct rq *rq);
>  int scx_check_setscheduler(struct task_struct *p, int policy);
>  void scx_prepare_setscheduler(struct task_struct *p, int policy);
> +void scx_prepare_task_sched_change(struct task_struct *p);
>  bool task_should_scx(int policy);
>  bool scx_allow_ttwu_queue(const struct task_struct *p);
>  bool __scx_allow_proxy_exec(const struct task_struct *p);
> @@ -72,6 +73,7 @@ static inline void scx_rq_activate(struct rq *rq) {}
>  static inline void scx_rq_deactivate(struct rq *rq) {}
>  static inline int scx_check_setscheduler(struct task_struct *p, int policy) { return 0; }
>  static inline void scx_prepare_setscheduler(struct task_struct *p, int policy) {}
> +static inline void scx_prepare_task_sched_change(struct task_struct *p) {}
>  static inline bool task_on_scx(const struct task_struct *p) { return false; }
>  static inline bool scx_allow_ttwu_queue(const struct task_struct *p) { return true; }
>  static inline bool scx_allow_proxy_exec(const struct task_struct *p) { return true; }
> -- 
> 2.53.0
> 
> base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 15:21   ` Peter Zijlstra
@ 2026-11-12 16:01     ` Peter Zijlstra
  2026-11-12 16:32       ` Andrea Righi
  2026-11-12 16:05     ` Andrea Righi
  1 sibling, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2026-11-12 16:01 UTC (permalink / raw)
  To: Tao Cui
  Cc: tj, void, arighi, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

On Thu, Nov 12, 2026 at 04:21:18PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 12, 2026 at 10:49:35PM +0800, Tao Cui wrote:
> > From: Tao Cui <cuitao@kylinos.cn>
> > 
> > rt_mutex_setprio() is the only entry path into sched_ext that does not
> > block a retained donor: an ext task PI-boosted by an rt waiter can
> > block on a second kernel mutex while boosted, and on deboost is routed
> > back to sched_ext carrying the queued+blocked state that
> > scx_prepare_setscheduler() promises never happens; every PI cycle
> > hands the BPF scheduler a queued+blocked donor it never admitted.
> > 
> > Call scx_prepare_task_sched_change() before the sched_change guard when
> > the class change routes the task back to sched_ext, which performs the
> > missing sched_proxy_block_task() (plus an update_rq_clock() that is
> > harmless here: the clock was already advanced, so the second advance
> > only applies a small delta). The boost direction (ext -> rt) needs no
> > handling.
> 
> I can't understand this. And I take a very dim view of sprinking magic
> hooks like this. So no.

Ooh, is this the thing from here:

  https://patch.msgid.link/20260924075925.GE4121620%40noisy.programming.kicks-ass.net

where you need to block a task because ext doesn't want to support
proxy-exec?

Why can't anybody search email these days..

Something like so? Although I note the scx_block_proxy_donor() thing has
gotten more complicated, so perhaps this needs a little more?

Andrea?

---
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 1a088d4576f5..1c1e669473a6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -11252,8 +11252,13 @@ void sched_change_end(struct sched_change_ctx *ctx)
 	 */
 	WARN_ON_ONCE(p->sched_class != ctx->class && !(ctx->flags & ENQUEUE_CLASS));
 
-	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to)
-		p->sched_class->switching_to(rq, p);
+	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to) {
+		bool block = p->sched_class->switching_to(rq, p);
+		if (block && p->is_blocked && ctx->queued) {
+			block_task(rq, p, READ_ONCE(p->__state));
+			ctx->queued = false;
+		}
+	}
 
 	if (ctx->queued)
 		enqueue_task(rq, p, ctx->flags);
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index f74ffb3b1aca..4440e32236b3 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -46,21 +46,6 @@ bool __scx_allow_proxy_exec(const struct task_struct *p)
 	return !sch || (sch->ops.flags & SCX_OPS_ENQ_BLOCKED);
 }
 
-/*
- * End retained proxy execution before changing @p's BPF scheduler ownership.
- * Called with @p's pi and rq locks held immediately before
- * sched_change_begin(). The caller must pass DEQUEUE_NOCLOCK so the rq clock
- * is updated only once.
- */
-void scx_prepare_task_sched_change(struct task_struct *p)
-{
-	lockdep_assert_held(&p->pi_lock);
-	lockdep_assert_rq_held(task_rq(p));
-
-	update_rq_clock(task_rq(p));
-	scx_block_proxy_donor(task_rq(p), p);
-}
-
 /*
  * NOTE: sched_ext is in the process of growing multiple scheduler support and
  * scx_root usage is in a transitional state. Naked dereferences are safe if the
@@ -5004,12 +4989,12 @@ static void prio_changed_scx(struct rq *rq, struct task_struct *p, u64 oldprio)
 {
 }
 
-static void switching_to_scx(struct rq *rq, struct task_struct *p)
+static bool switching_to_scx(struct rq *rq, struct task_struct *p)
 {
 	struct scx_sched *sch = scx_task_sched(p);
 
 	if (task_dead_and_done(p))
-		return;
+		return false;
 
 	scx_enable_task(sch, p);
 
@@ -5020,6 +5005,8 @@ static void switching_to_scx(struct rq *rq, struct task_struct *p)
 	 */
 	if (!scx_is_cid_type() && SCX_HAS_OP(sch, set_cpumask))
 		scx_call_op_set_cpumask(sch, rq, p, (struct cpumask *)p->cpus_ptr);
+
+	return !scx_allow_proxy_exec(p);
 }
 
 static void switched_from_scx(struct rq *rq, struct task_struct *p)
@@ -8947,10 +8934,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
 
 		if (old_class != new_class)
 			queue_flags |= DEQUEUE_CLASS;
-		if (new_class == &ext_sched_class) {
-			scx_prepare_task_sched_change(p);
+		if (new_class == &ext_sched_class)
 			queue_flags |= DEQUEUE_NOCLOCK;
-		}
 
 		scoped_guard (sched_change, p, queue_flags) {
 			scx_set_task_slice(p, READ_ONCE(sch->slice_dfl));
diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
index ff0a3d508533..961694e4c27c 100644
--- a/kernel/sched/ext/internal.h
+++ b/kernel/sched/ext/internal.h
@@ -2300,7 +2300,6 @@ struct task_struct *scx_task_iter_next_locked(struct scx_task_iter *iter);
 bool scx_set_task_slice(struct task_struct *p, u64 slice);
 void scx_task_slice_ended(struct rq *rq, struct task_struct *p);
 void scx_task_unlink_from_dsq(struct task_struct *p, struct scx_dispatch_q *dsq);
-void scx_prepare_task_sched_change(struct task_struct *p);
 void scx_dispatch_dequeue(struct rq *rq, struct task_struct *p);
 void scx_do_enqueue_task(struct rq *rq, struct task_struct *p, u64 enq_flags,
 			 int sticky_cpu);
diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
index b5d92d89629b..e73f3474d685 100644
--- a/kernel/sched/ext/sub.c
+++ b/kernel/sched/ext/sub.c
@@ -1647,7 +1647,6 @@ static void scx_rehome_task(struct scx_sched *to, struct task_struct *p)
 	lockdep_assert_held(&p->pi_lock);
 	lockdep_assert_rq_held(task_rq(p));
 
-	scx_prepare_task_sched_change(p);
 	scoped_guard (sched_change, p,
 		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
 		scx_disable_and_exit_task(scx_task_sched(p), p);
@@ -1680,7 +1679,6 @@ static void scx_punt_task(struct scx_sched *to, struct task_struct *p)
 	lockdep_assert_rq_held(task_rq(p));
 	WARN_ON_ONCE(!READ_ONCE(to->bypass_depth));
 
-	scx_prepare_task_sched_change(p);
 	scoped_guard (sched_change, p,
 		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
 		scx_disable_and_exit_task(scx_task_sched(p), p);
@@ -2459,7 +2457,6 @@ void scx_sub_enable_workfn(struct kthread_work *work)
 		if (!(p->scx.flags & SCX_TASK_SUB_INIT))
 			continue;
 
-		scx_prepare_task_sched_change(p);
 		scoped_guard (sched_change, p,
 			      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
 			/*
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index e0c10a5dbda2..cda367bd92af 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -547,7 +547,7 @@ static void task_tick_idle(struct rq *rq, struct task_struct *curr, int queued)
 	update_curr_idle(rq);
 }
 
-static void switching_to_idle(struct rq *rq, struct task_struct *p)
+static bool switching_to_idle(struct rq *rq, struct task_struct *p)
 {
 	BUG();
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 1d76950dbb0a..979c520034b1 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2782,7 +2782,7 @@ struct sched_class {
 	 */
 	void (*switching_from)(struct rq *this_rq, struct task_struct *task);
 	void (*switched_from) (struct rq *this_rq, struct task_struct *task);
-	void (*switching_to)  (struct rq *this_rq, struct task_struct *task);
+	bool (*switching_to)  (struct rq *this_rq, struct task_struct *task);
 	void (*switched_to)   (struct rq *this_rq, struct task_struct *task);
 	u64  (*get_prio)     (struct rq *this_rq, struct task_struct *task);
 	void (*prio_changed) (struct rq *this_rq, struct task_struct *task,
diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
index 1e0109ec36b3..b7541bb7b593 100644
--- a/kernel/sched/stop_task.c
+++ b/kernel/sched/stop_task.c
@@ -78,7 +78,7 @@ static void task_tick_stop(struct rq *rq, struct task_struct *curr, int queued)
 {
 }
 
-static void switching_to_stop(struct rq *rq, struct task_struct *p)
+static bool switching_to_stop(struct rq *rq, struct task_struct *p)
 {
 	BUG(); /* its impossible to change to this class */
 }

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 15:21   ` Peter Zijlstra
  2026-11-12 16:01     ` Peter Zijlstra
@ 2026-11-12 16:05     ` Andrea Righi
  2026-11-12 16:31       ` Peter Zijlstra
  1 sibling, 1 reply; 10+ messages in thread
From: Andrea Righi @ 2026-11-12 16:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Tao Cui, tj, void, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

On Thu, Nov 12, 2026 at 04:21:18PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 12, 2026 at 10:49:35PM +0800, Tao Cui wrote:
> > From: Tao Cui <cuitao@kylinos.cn>
> > 
> > rt_mutex_setprio() is the only entry path into sched_ext that does not
> > block a retained donor: an ext task PI-boosted by an rt waiter can
> > block on a second kernel mutex while boosted, and on deboost is routed
> > back to sched_ext carrying the queued+blocked state that
> > scx_prepare_setscheduler() promises never happens; every PI cycle
> > hands the BPF scheduler a queued+blocked donor it never admitted.
> > 
> > Call scx_prepare_task_sched_change() before the sched_change guard when
> > the class change routes the task back to sched_ext, which performs the
> > missing sched_proxy_block_task() (plus an update_rq_clock() that is
> > harmless here: the clock was already advanced, so the second advance
> > only applies a small delta). The boost direction (ext -> rt) needs no
> > handling.
> 
> I can't understand this. And I take a very dim view of sprinking magic
> hooks like this. So no.
> 
> The little I understand of things makes me think __setscheduler_class()
> is buggy for EXT and *that* should be fixed.

Agreed on avoiding another special-case hook.

To recap, we ran into this while developing the proxy-exec support for
scx. Earlier versions were handling the PI deboost case too and we also
explored passing next_class into sched_change_begin(). But that didn't
make it into the final version and it was left as a future improvement,
to keep things simpler.

Regarding __setscheduler_class(), the task keeps its SCHED_EXT policy
while PI moves it into RT/DL, so selecting EXT again after deboost seems
expected. The problem is carrying the retained proxy state across that
transition.

Would it make sense to revisit the old idea passing next_class into
sched_change_begin(), so we can handle entry into EXT consistently
before recording the queued/running state and replace the separate
class-entry preparation hooks?

And unlike the earlier version, we could restrict this to transitions
into EXT, leaving retained proxy state across FAIR/RT/DL transitions
untouched. An incoming-class preparation callback would let EXT enforce
that requirement without an EXT-specific check in each caller.

That would address the transition handling generally, including PI
deboost. Does it make sense?

Thanks,
-Andrea

> 
> > Fixes: be8be8acb268 ("sched_ext: Block proxy donors before taking control")
> > Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> > ---
> >  kernel/sched/core.c    | 12 ++++++++++++
> >  kernel/sched/ext/ext.h |  2 ++
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index f7a452123ef5..dcb16b006740 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -7816,6 +7816,18 @@ void rt_mutex_setprio(struct task_struct *p, struct task_struct *pi_task)
> >  	if (prev_class != next_class)
> >  		queue_flag |= DEQUEUE_CLASS;
> >  
> > +	/*
> > +	 * PI boost can route @p out of sched_ext: policy stays SCHED_EXT but
> > +	 * __setscheduler_class() follows the effective rt prio, and rt/fair
> > +	 * may have retained @p on the rq as a blocked donor. Block it before
> > +	 * sched_change_begin() records the queued state, the same way the
> > +	 * other entry points into sched_ext do.
> > +	 */
> > +#ifdef CONFIG_SCHED_CLASS_EXT
> > +	if ((queue_flag & DEQUEUE_CLASS) && next_class == &ext_sched_class)
> > +		scx_prepare_task_sched_change(p);
> > +#endif
> > +
> >  	scoped_guard (sched_change, p, queue_flag) {
> >  		/*
> >  		 * Boosting condition are:
> > diff --git a/kernel/sched/ext/ext.h b/kernel/sched/ext/ext.h
> > index d32bc01071b0..1a699be25331 100644
> > --- a/kernel/sched/ext/ext.h
> > +++ b/kernel/sched/ext/ext.h
> > @@ -19,6 +19,7 @@ void scx_rq_activate(struct rq *rq);
> >  void scx_rq_deactivate(struct rq *rq);
> >  int scx_check_setscheduler(struct task_struct *p, int policy);
> >  void scx_prepare_setscheduler(struct task_struct *p, int policy);
> > +void scx_prepare_task_sched_change(struct task_struct *p);
> >  bool task_should_scx(int policy);
> >  bool scx_allow_ttwu_queue(const struct task_struct *p);
> >  bool __scx_allow_proxy_exec(const struct task_struct *p);
> > @@ -72,6 +73,7 @@ static inline void scx_rq_activate(struct rq *rq) {}
> >  static inline void scx_rq_deactivate(struct rq *rq) {}
> >  static inline int scx_check_setscheduler(struct task_struct *p, int policy) { return 0; }
> >  static inline void scx_prepare_setscheduler(struct task_struct *p, int policy) {}
> > +static inline void scx_prepare_task_sched_change(struct task_struct *p) {}
> >  static inline bool task_on_scx(const struct task_struct *p) { return false; }
> >  static inline bool scx_allow_ttwu_queue(const struct task_struct *p) { return true; }
> >  static inline bool scx_allow_proxy_exec(const struct task_struct *p) { return true; }
> > -- 
> > 2.53.0
> > 
> > base-commit: 5b740a72b55c4267ba4d8010f17dde26b26f52d3
> 

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 16:05     ` Andrea Righi
@ 2026-11-12 16:31       ` Peter Zijlstra
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Zijlstra @ 2026-11-12 16:31 UTC (permalink / raw)
  To: Andrea Righi
  Cc: Tao Cui, tj, void, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

On Thu, Nov 12, 2026 at 05:05:04PM +0100, Andrea Righi wrote:

> To recap, we ran into this while developing the proxy-exec support for
> scx. Earlier versions were handling the PI deboost case too and we also
> explored passing next_class into sched_change_begin(). But that didn't
> make it into the final version and it was left as a future improvement,
> to keep things simpler.

> Would it make sense to revisit the old idea passing next_class into
> sched_change_begin(), so we can handle entry into EXT consistently
> before recording the queued/running state and replace the separate
> class-entry preparation hooks?

As per my follow up email, I think all we need is for the existing
switching_to callback to return a boolean to indicate if the class is
willing to accept blocked tasks.

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 16:01     ` Peter Zijlstra
@ 2026-11-12 16:32       ` Andrea Righi
  2026-11-12 16:41         ` Peter Zijlstra
  0 siblings, 1 reply; 10+ messages in thread
From: Andrea Righi @ 2026-11-12 16:32 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Tao Cui, tj, void, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

Hi Peter,

On Thu, Nov 12, 2026 at 05:01:36PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 12, 2026 at 04:21:18PM +0100, Peter Zijlstra wrote:
> > On Thu, Nov 12, 2026 at 10:49:35PM +0800, Tao Cui wrote:
> > > From: Tao Cui <cuitao@kylinos.cn>
> > > 
> > > rt_mutex_setprio() is the only entry path into sched_ext that does not
> > > block a retained donor: an ext task PI-boosted by an rt waiter can
> > > block on a second kernel mutex while boosted, and on deboost is routed
> > > back to sched_ext carrying the queued+blocked state that
> > > scx_prepare_setscheduler() promises never happens; every PI cycle
> > > hands the BPF scheduler a queued+blocked donor it never admitted.
> > > 
> > > Call scx_prepare_task_sched_change() before the sched_change guard when
> > > the class change routes the task back to sched_ext, which performs the
> > > missing sched_proxy_block_task() (plus an update_rq_clock() that is
> > > harmless here: the clock was already advanced, so the second advance
> > > only applies a small delta). The boost direction (ext -> rt) needs no
> > > handling.
> > 
> > I can't understand this. And I take a very dim view of sprinking magic
> > hooks like this. So no.
> 
> Ooh, is this the thing from here:
> 
>   https://patch.msgid.link/20260924075925.GE4121620%40noisy.programming.kicks-ass.net
> 
> where you need to block a task because ext doesn't want to support
> proxy-exec?
> 
> Why can't anybody search email these days..
> 
> Something like so? Although I note the scx_block_proxy_donor() thing has
> gotten more complicated, so perhaps this needs a little more?
> 
> Andrea?

Ah yes, our emails crossed. I was proposing to reconsider passing
next_class into sched_change_begin(), but using the existing
switching_to() callback looks like a better direction, without needing
the extra argument.

A few details below.

> 
> ---
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1a088d4576f5..1c1e669473a6 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -11252,8 +11252,13 @@ void sched_change_end(struct sched_change_ctx *ctx)
>  	 */
>  	WARN_ON_ONCE(p->sched_class != ctx->class && !(ctx->flags & ENQUEUE_CLASS));
>  
> -	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to)
> -		p->sched_class->switching_to(rq, p);
> +	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to) {
> +		bool block = p->sched_class->switching_to(rq, p);
> +		if (block && p->is_blocked && ctx->queued) {
> +			block_task(rq, p, READ_ONCE(p->__state));
> +			ctx->queued = false;
> +		}

So at this point sched_change_begin() has already dequeued p from the
old class, but p hasn't been enqueued in the new class yet, block_task()
would dequeue it again, through the new class, and repeat the generic
dequeue accounting. I think we should complete the block without another
dequeue here.

Also, if p is rq->donor, clearing ctx->queued isn't enough: we need to
reset the donor and clear ctx->running, otherwise sched_change_end()
would call set_next_task(rq, p) for the task we just blocked.

> +	}
>  
>  	if (ctx->queued)
>  		enqueue_task(rq, p, ctx->flags);
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index f74ffb3b1aca..4440e32236b3 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -46,21 +46,6 @@ bool __scx_allow_proxy_exec(const struct task_struct *p)
>  	return !sch || (sch->ops.flags & SCX_OPS_ENQ_BLOCKED);
>  }
>  
> -/*
> - * End retained proxy execution before changing @p's BPF scheduler ownership.
> - * Called with @p's pi and rq locks held immediately before
> - * sched_change_begin(). The caller must pass DEQUEUE_NOCLOCK so the rq clock
> - * is updated only once.
> - */
> -void scx_prepare_task_sched_change(struct task_struct *p)
> -{
> -	lockdep_assert_held(&p->pi_lock);
> -	lockdep_assert_rq_held(task_rq(p));
> -
> -	update_rq_clock(task_rq(p));
> -	scx_block_proxy_donor(task_rq(p), p);
> -}
> -
>  /*
>   * NOTE: sched_ext is in the process of growing multiple scheduler support and
>   * scx_root usage is in a transitional state. Naked dereferences are safe if the
> @@ -5004,12 +4989,12 @@ static void prio_changed_scx(struct rq *rq, struct task_struct *p, u64 oldprio)
>  {
>  }
>  
> -static void switching_to_scx(struct rq *rq, struct task_struct *p)
> +static bool switching_to_scx(struct rq *rq, struct task_struct *p)
>  {
>  	struct scx_sched *sch = scx_task_sched(p);
>  
>  	if (task_dead_and_done(p))
> -		return;
> +		return false;
>  
>  	scx_enable_task(sch, p);
>  
> @@ -5020,6 +5005,8 @@ static void switching_to_scx(struct rq *rq, struct task_struct *p)
>  	 */
>  	if (!scx_is_cid_type() && SCX_HAS_OP(sch, set_cpumask))
>  		scx_call_op_set_cpumask(sch, rq, p, (struct cpumask *)p->cpus_ptr);
> +
> +	return !scx_allow_proxy_exec(p);
>  }
>  
>  static void switched_from_scx(struct rq *rq, struct task_struct *p)
> @@ -8947,10 +8934,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  
>  		if (old_class != new_class)
>  			queue_flags |= DEQUEUE_CLASS;
> -		if (new_class == &ext_sched_class) {
> -			scx_prepare_task_sched_change(p);
> +		if (new_class == &ext_sched_class)
>  			queue_flags |= DEQUEUE_NOCLOCK;
> -		}

scx_prepare_task_sched_change() also calls update_rq_clock(). With
that call removed, keeping DEQUEUE_NOCLOCK makes sched_change_begin()
skip the clock update even though we haven't done it here.

Maybe we should drop this flag too and let sched_change_begin() update
the clock.

>  
>  		scoped_guard (sched_change, p, queue_flags) {
>  			scx_set_task_slice(p, READ_ONCE(sch->slice_dfl));
> diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
> index ff0a3d508533..961694e4c27c 100644
> --- a/kernel/sched/ext/internal.h
> +++ b/kernel/sched/ext/internal.h
> @@ -2300,7 +2300,6 @@ struct task_struct *scx_task_iter_next_locked(struct scx_task_iter *iter);
>  bool scx_set_task_slice(struct task_struct *p, u64 slice);
>  void scx_task_slice_ended(struct rq *rq, struct task_struct *p);
>  void scx_task_unlink_from_dsq(struct task_struct *p, struct scx_dispatch_q *dsq);
> -void scx_prepare_task_sched_change(struct task_struct *p);
>  void scx_dispatch_dequeue(struct rq *rq, struct task_struct *p);
>  void scx_do_enqueue_task(struct rq *rq, struct task_struct *p, u64 enq_flags,
>  			 int sticky_cpu);
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index b5d92d89629b..e73f3474d685 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
> @@ -1647,7 +1647,6 @@ static void scx_rehome_task(struct scx_sched *to, struct task_struct *p)
>  	lockdep_assert_held(&p->pi_lock);
>  	lockdep_assert_rq_held(task_rq(p));
>  
> -	scx_prepare_task_sched_change(p);
>  	scoped_guard (sched_change, p,
>  		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {

With sub-schedulers these paths can move a task between BPF schedulers
without changing p->sched_class, so switching_to() won't run. We still
need to end the old proxy session before changing ownership, even if the
new BPF scheduler also supports proxy-exec.

Can we keep scx_prepare_task_sched_change() for these ownership changes
and use switching_to() for the actual class transitions?

>  		scx_disable_and_exit_task(scx_task_sched(p), p);
> @@ -1680,7 +1679,6 @@ static void scx_punt_task(struct scx_sched *to, struct task_struct *p)
>  	lockdep_assert_rq_held(task_rq(p));
>  	WARN_ON_ONCE(!READ_ONCE(to->bypass_depth));
>  
> -	scx_prepare_task_sched_change(p);
>  	scoped_guard (sched_change, p,
>  		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {

And ditto here.

>  		scx_disable_and_exit_task(scx_task_sched(p), p);
> @@ -2459,7 +2457,6 @@ void scx_sub_enable_workfn(struct kthread_work *work)
>  		if (!(p->scx.flags & SCX_TASK_SUB_INIT))
>  			continue;
>  
> -		scx_prepare_task_sched_change(p);
>  		scoped_guard (sched_change, p,
>  			      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
>  			/*
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index e0c10a5dbda2..cda367bd92af 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -547,7 +547,7 @@ static void task_tick_idle(struct rq *rq, struct task_struct *curr, int queued)
>  	update_curr_idle(rq);
>  }
>  
> -static void switching_to_idle(struct rq *rq, struct task_struct *p)
> +static bool switching_to_idle(struct rq *rq, struct task_struct *p)
>  {
>  	BUG();
>  }
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 1d76950dbb0a..979c520034b1 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2782,7 +2782,7 @@ struct sched_class {
>  	 */
>  	void (*switching_from)(struct rq *this_rq, struct task_struct *task);
>  	void (*switched_from) (struct rq *this_rq, struct task_struct *task);
> -	void (*switching_to)  (struct rq *this_rq, struct task_struct *task);
> +	bool (*switching_to)  (struct rq *this_rq, struct task_struct *task);
>  	void (*switched_to)   (struct rq *this_rq, struct task_struct *task);
>  	u64  (*get_prio)     (struct rq *this_rq, struct task_struct *task);
>  	void (*prio_changed) (struct rq *this_rq, struct task_struct *task,
> diff --git a/kernel/sched/stop_task.c b/kernel/sched/stop_task.c
> index 1e0109ec36b3..b7541bb7b593 100644
> --- a/kernel/sched/stop_task.c
> +++ b/kernel/sched/stop_task.c
> @@ -78,7 +78,7 @@ static void task_tick_stop(struct rq *rq, struct task_struct *curr, int queued)
>  {
>  }
>  
> -static void switching_to_stop(struct rq *rq, struct task_struct *p)
> +static bool switching_to_stop(struct rq *rq, struct task_struct *p)
>  {
>  	BUG(); /* its impossible to change to this class */
>  }

Thanks,
-Andrea

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 16:32       ` Andrea Righi
@ 2026-11-12 16:41         ` Peter Zijlstra
  2026-11-12 16:50           ` Andrea Righi
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2026-11-12 16:41 UTC (permalink / raw)
  To: Andrea Righi
  Cc: Tao Cui, tj, void, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

On Thu, Nov 12, 2026 at 05:32:40PM +0100, Andrea Righi wrote:

> > 
> > ---
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 1a088d4576f5..1c1e669473a6 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -11252,8 +11252,13 @@ void sched_change_end(struct sched_change_ctx *ctx)
> >  	 */
> >  	WARN_ON_ONCE(p->sched_class != ctx->class && !(ctx->flags & ENQUEUE_CLASS));
> >  
> > -	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to)
> > -		p->sched_class->switching_to(rq, p);
> > +	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to) {
> > +		bool block = p->sched_class->switching_to(rq, p);
> > +		if (block && p->is_blocked && ctx->queued) {
> > +			block_task(rq, p, READ_ONCE(p->__state));
> > +			ctx->queued = false;
> > +		}
> 
> So at this point sched_change_begin() has already dequeued p from the
> old class, but p hasn't been enqueued in the new class yet, block_task()
> would dequeue it again, through the new class, and repeat the generic
> dequeue accounting. I think we should complete the block without another
> dequeue here.
> 
> Also, if p is rq->donor, clearing ctx->queued isn't enough: we need to
> reset the donor and clear ctx->running, otherwise sched_change_end()
> would call set_next_task(rq, p) for the task we just blocked.

Right, I figured there was something missing. Can you take this patch
and make it work?; you are more proficient with running this ext stuff
than me.

> > +	}
> >  
> >  	if (ctx->queued)
> >  		enqueue_task(rq, p, ctx->flags);
> > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > index f74ffb3b1aca..4440e32236b3 100644
> > --- a/kernel/sched/ext/ext.c
> > +++ b/kernel/sched/ext/ext.c

> > @@ -8947,10 +8934,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
> >  
> >  		if (old_class != new_class)
> >  			queue_flags |= DEQUEUE_CLASS;
> > -		if (new_class == &ext_sched_class) {
> > -			scx_prepare_task_sched_change(p);
> > +		if (new_class == &ext_sched_class)
> >  			queue_flags |= DEQUEUE_NOCLOCK;
> > -		}
> 
> scx_prepare_task_sched_change() also calls update_rq_clock(). With
> that call removed, keeping DEQUEUE_NOCLOCK makes sched_change_begin()
> skip the clock update even though we haven't done it here.
> 
> Maybe we should drop this flag too and let sched_change_begin() update
> the clock.

Sure.

> > diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> > index b5d92d89629b..e73f3474d685 100644
> > --- a/kernel/sched/ext/sub.c
> > +++ b/kernel/sched/ext/sub.c
> > @@ -1647,7 +1647,6 @@ static void scx_rehome_task(struct scx_sched *to, struct task_struct *p)
> >  	lockdep_assert_held(&p->pi_lock);
> >  	lockdep_assert_rq_held(task_rq(p));
> >  
> > -	scx_prepare_task_sched_change(p);
> >  	scoped_guard (sched_change, p,
> >  		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
> 
> With sub-schedulers these paths can move a task between BPF schedulers
> without changing p->sched_class, so switching_to() won't run. We still
> need to end the old proxy session before changing ownership, even if the
> new BPF scheduler also supports proxy-exec.
> 
> Can we keep scx_prepare_task_sched_change() for these ownership changes
> and use switching_to() for the actual class transitions?

ARGH, are you *sure* you want to support crazy like that? Just have the
thing mandate consistent proxy support.

Enforce the primary scheduler's take when loading sub-schedulers. If it
does support proxy, dis-allow loading sub scheds that don't and
vice-versa.

Alternatively, make the support the logical and of all sub-scheduler
support. Ergo if one sub sched doesn't want to support proxy, nobody
gets it.

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

* Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
  2026-11-12 16:41         ` Peter Zijlstra
@ 2026-11-12 16:50           ` Andrea Righi
  0 siblings, 0 replies; 10+ messages in thread
From: Andrea Righi @ 2026-11-12 16:50 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Tao Cui, tj, void, changwoo, mingo, sched-ext, linux-kernel, bpf,
	Tao Cui

On Thu, Nov 12, 2026 at 05:41:49PM +0100, Peter Zijlstra wrote:
> On Thu, Nov 12, 2026 at 05:32:40PM +0100, Andrea Righi wrote:
> 
> > > 
> > > ---
> > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > > index 1a088d4576f5..1c1e669473a6 100644
> > > --- a/kernel/sched/core.c
> > > +++ b/kernel/sched/core.c
> > > @@ -11252,8 +11252,13 @@ void sched_change_end(struct sched_change_ctx *ctx)
> > >  	 */
> > >  	WARN_ON_ONCE(p->sched_class != ctx->class && !(ctx->flags & ENQUEUE_CLASS));
> > >  
> > > -	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to)
> > > -		p->sched_class->switching_to(rq, p);
> > > +	if ((ctx->flags & ENQUEUE_CLASS) && p->sched_class->switching_to) {
> > > +		bool block = p->sched_class->switching_to(rq, p);
> > > +		if (block && p->is_blocked && ctx->queued) {
> > > +			block_task(rq, p, READ_ONCE(p->__state));
> > > +			ctx->queued = false;
> > > +		}
> > 
> > So at this point sched_change_begin() has already dequeued p from the
> > old class, but p hasn't been enqueued in the new class yet, block_task()
> > would dequeue it again, through the new class, and repeat the generic
> > dequeue accounting. I think we should complete the block without another
> > dequeue here.
> > 
> > Also, if p is rq->donor, clearing ctx->queued isn't enough: we need to
> > reset the donor and clear ctx->running, otherwise sched_change_end()
> > would call set_next_task(rq, p) for the task we just blocked.
> 
> Right, I figured there was something missing. Can you take this patch
> and make it work?; you are more proficient with running this ext stuff
> than me.

Ack, I can take it from here and test all the nasty ext edge cases. :)

> 
> > > +	}
> > >  
> > >  	if (ctx->queued)
> > >  		enqueue_task(rq, p, ctx->flags);
> > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> > > index f74ffb3b1aca..4440e32236b3 100644
> > > --- a/kernel/sched/ext/ext.c
> > > +++ b/kernel/sched/ext/ext.c
> 
> > > @@ -8947,10 +8934,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
> > >  
> > >  		if (old_class != new_class)
> > >  			queue_flags |= DEQUEUE_CLASS;
> > > -		if (new_class == &ext_sched_class) {
> > > -			scx_prepare_task_sched_change(p);
> > > +		if (new_class == &ext_sched_class)
> > >  			queue_flags |= DEQUEUE_NOCLOCK;
> > > -		}
> > 
> > scx_prepare_task_sched_change() also calls update_rq_clock(). With
> > that call removed, keeping DEQUEUE_NOCLOCK makes sched_change_begin()
> > skip the clock update even though we haven't done it here.
> > 
> > Maybe we should drop this flag too and let sched_change_begin() update
> > the clock.
> 
> Sure.
> 
> > > diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> > > index b5d92d89629b..e73f3474d685 100644
> > > --- a/kernel/sched/ext/sub.c
> > > +++ b/kernel/sched/ext/sub.c
> > > @@ -1647,7 +1647,6 @@ static void scx_rehome_task(struct scx_sched *to, struct task_struct *p)
> > >  	lockdep_assert_held(&p->pi_lock);
> > >  	lockdep_assert_rq_held(task_rq(p));
> > >  
> > > -	scx_prepare_task_sched_change(p);
> > >  	scoped_guard (sched_change, p,
> > >  		      DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK) {
> > 
> > With sub-schedulers these paths can move a task between BPF schedulers
> > without changing p->sched_class, so switching_to() won't run. We still
> > need to end the old proxy session before changing ownership, even if the
> > new BPF scheduler also supports proxy-exec.
> > 
> > Can we keep scx_prepare_task_sched_change() for these ownership changes
> > and use switching_to() for the actual class transitions?
> 
> ARGH, are you *sure* you want to support crazy like that? Just have the
> thing mandate consistent proxy support.
> 
> Enforce the primary scheduler's take when loading sub-schedulers. If it
> does support proxy, dis-allow loading sub scheds that don't and
> vice-versa.
> 
> Alternatively, make the support the logical and of all sub-scheduler
> support. Ergo if one sub sched doesn't want to support proxy, nobody
> gets it.

Yeah, fair point, mixed proxy support across sub-schedulers makes this
unnecessarily complicated for zero benefit. It seems more reasonable to
make proxy exec opt-in hierarchical.

I'll look into this along with the switching_to() changes.

Thanks,
-Andrea

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

end of thread, other threads:[~2026-11-12 16:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-12 14:49 [PATCH 0/2] sched_ext: Fix entry-path contract breaks in core.c Tao Cui
2026-11-12 14:49 ` [PATCH 1/2] sched/core: Pass ENQUEUE_RQ_SELECTED on initial enqueue Tao Cui
2026-11-12 14:49 ` [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext Tao Cui
2026-11-12 15:21   ` Peter Zijlstra
2026-11-12 16:01     ` Peter Zijlstra
2026-11-12 16:32       ` Andrea Righi
2026-11-12 16:41         ` Peter Zijlstra
2026-11-12 16:50           ` Andrea Righi
2026-11-12 16:05     ` Andrea Righi
2026-11-12 16:31       ` Peter Zijlstra

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