All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Tao Cui <cui.tao@linux.dev>
Cc: tj@kernel.org, void@manifault.com, arighi@nvidia.com,
	changwoo@igalia.com, mingo@redhat.com, sched-ext@lists.linux.dev,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	Tao Cui <cuitao@kylinos.cn>
Subject: Re: [PATCH 2/2] sched_ext: Block a retained donor before PI deboost routes it back to sched_ext
Date: Thu, 12 Nov 2026 17:01:36 +0100	[thread overview]
Message-ID: <20261112160136.GF1922751@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20261112152118.GD1813469@noisy.programming.kicks-ass.net>

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 */
 }

  reply	other threads:[~2026-11-12 16:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261112160136.GF1922751@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=arighi@nvidia.com \
    --cc=bpf@vger.kernel.org \
    --cc=changwoo@igalia.com \
    --cc=cui.tao@linux.dev \
    --cc=cuitao@kylinos.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.