From: K Prateek Nayak <kprateek.nayak@amd.com>
To: John Stultz <jstultz@google.com>, LKML <linux-kernel@vger.kernel.org>
Cc: Joel Fernandes <joelagnelf@nvidia.com>,
Qais Yousef <qyousef@layalina.io>, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
"Juri Lelli" <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Valentin Schneider <vschneid@redhat.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>,
Zimuzo Ezeozue <zezeozue@google.com>,
Mel Gorman <mgorman@suse.de>, Will Deacon <will@kernel.org>,
Waiman Long <longman@redhat.com>,
Boqun Feng <boqun.feng@gmail.com>,
"Paul E. McKenney" <paulmck@kernel.org>,
Metin Kaya <Metin.Kaya@arm.com>,
Xuewen Yan <xuewen.yan94@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
"Daniel Lezcano" <daniel.lezcano@linaro.org>,
Suleiman Souhlal <suleiman@google.com>,
kuyo chang <kuyo.chang@mediatek.com>, hupu <hupu.gm@gmail.com>,
<kernel-team@android.com>
Subject: Re: [PATCH v25 9/9] sched: Handle blocked-waiter migration (and return migration)
Date: Sun, 15 Mar 2026 23:08:20 +0530 [thread overview]
Message-ID: <f092c21f-2e48-4e4e-90ac-af6a05dc7c28@amd.com> (raw)
In-Reply-To: <20260313023022.2902479-10-jstultz@google.com>
Hello John,
On 3/13/2026 8:00 AM, John Stultz wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index af497b8c72dce..fe20204cf51cc 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3643,6 +3643,23 @@ void update_rq_avg_idle(struct rq *rq)
> rq->idle_stamp = 0;
> }
>
> +#ifdef CONFIG_SCHED_PROXY_EXEC
> +static inline void proxy_set_task_cpu(struct task_struct *p, int cpu)
> +{
> + unsigned int wake_cpu;
> +
> + /*
> + * Since we are enqueuing a blocked task on a cpu it may
> + * not be able to run on, preserve wake_cpu when we
> + * __set_task_cpu so we can return the task to where it
> + * was previously runnable.
> + */
> + wake_cpu = p->wake_cpu;
> + __set_task_cpu(p, cpu);
> + p->wake_cpu = wake_cpu;
> +}
> +#endif /* CONFIG_SCHED_PROXY_EXEC */
> +
> static void
> ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
> struct rq_flags *rf)
> @@ -4242,13 +4259,6 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
> ttwu_queue(p, cpu, wake_flags);
> }
> out:
> - /*
> - * For now, if we've been woken up, clear the task->blocked_on
> - * regardless if it was set to a mutex or PROXY_WAKING so the
> - * task can run. We will need to be more careful later when
> - * properly handling proxy migration
> - */
> - clear_task_blocked_on(p, NULL);
So, for this bit, there are mutex variants that are interruptible and
killable which probably benefits from clearing the blocked_on
relation.
For potential proxy task that are still queued, we'll hit the
ttwu_runnable() path and resched out of there so it makes sense to
mark them as PROXY_WAKING so schedule() can return migrate them, they
run and hit the signal_pending_state() check in __mutex_lock_common()
loop, and return -EINTR.
Otherwise, if they need a full wakeup, they may be blocked on a
sleeping owner, in which case it is beneficial to clear blocked_on, do
a full wakeup. and let them run to evaluate the pending signal.
ttwu_state_match() should filter out any spurious signals. Thoughts?
> if (success)
> ttwu_stat(p, task_cpu(p), wake_flags);
>
> @@ -6575,7 +6585,7 @@ static inline struct task_struct *proxy_resched_idle(struct rq *rq)
> return rq->idle;
> }
>
> -static bool __proxy_deactivate(struct rq *rq, struct task_struct *donor)
> +static bool proxy_deactivate(struct rq *rq, struct task_struct *donor)
> {
> unsigned long state = READ_ONCE(donor->__state);
>
> @@ -6595,17 +6605,135 @@ static bool __proxy_deactivate(struct rq *rq, struct task_struct *donor)
> return try_to_block_task(rq, donor, &state, true);
> }
>
> -static struct task_struct *proxy_deactivate(struct rq *rq, struct task_struct *donor)
> +/*
> + * If the blocked-on relationship crosses CPUs, migrate @p to the
> + * owner's CPU.
> + *
> + * This is because we must respect the CPU affinity of execution
> + * contexts (owner) but we can ignore affinity for scheduling
> + * contexts (@p). So we have to move scheduling contexts towards
> + * potential execution contexts.
> + *
> + * Note: The owner can disappear, but simply migrate to @target_cpu
> + * and leave that CPU to sort things out.
> + */
> +static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,
> + struct task_struct *p, int target_cpu)
> {
> - if (!__proxy_deactivate(rq, donor)) {
> - /*
> - * XXX: For now, if deactivation failed, set donor
> - * as unblocked, as we aren't doing proxy-migrations
> - * yet (more logic will be needed then).
> - */
> - clear_task_blocked_on(donor, NULL);
> + struct rq *target_rq = cpu_rq(target_cpu);
> +
> + lockdep_assert_rq_held(rq);
> +
> + /*
> + * Since we're going to drop @rq, we have to put(@rq->donor) first,
> + * otherwise we have a reference that no longer belongs to us.
> + *
> + * Additionally, as we put_prev_task(prev) earlier, its possible that
> + * prev will migrate away as soon as we drop the rq lock, however we
> + * still have it marked as rq->curr, as we've not yet switched tasks.
> + *
> + * So call proxy_resched_idle() to let go of the references before
> + * we release the lock.
> + */
> + proxy_resched_idle(rq);
> +
> + WARN_ON(p == rq->curr);
> +
> + deactivate_task(rq, p, DEQUEUE_NOCLOCK);
> + proxy_set_task_cpu(p, target_cpu);
> +
> + /*
> + * We have to zap callbacks before unlocking the rq
> + * as another CPU may jump in and call sched_balance_rq
> + * which can trip the warning in rq_pin_lock() if we
> + * leave callbacks set.
> + */
> + zap_balance_callbacks(rq);
> + rq_unpin_lock(rq, rf);
> + raw_spin_rq_unlock(rq);
> +
> + attach_one_task(target_rq, p);
> +
> + raw_spin_rq_lock(rq);
> + rq_repin_lock(rq, rf);
> + update_rq_clock(rq);
> +}
> +
> +static void proxy_force_return(struct rq *rq, struct rq_flags *rf,
> + struct task_struct *p)
> +{
> + struct rq *this_rq, *target_rq;
> + struct rq_flags this_rf;
> + int cpu, wake_flag = WF_TTWU;
> +
> + lockdep_assert_rq_held(rq);
> + WARN_ON(p == rq->curr);
> +
> + /*
> + * We have to zap callbacks before unlocking the rq
> + * as another CPU may jump in and call sched_balance_rq
> + * which can trip the warning in rq_pin_lock() if we
> + * leave callbacks set.
> + */
> + zap_balance_callbacks(rq);
> + rq_unpin_lock(rq, rf);
> + raw_spin_rq_unlock(rq);
> +
> + /*
> + * We drop the rq lock, and re-grab task_rq_lock to get
> + * the pi_lock (needed for select_task_rq) as well.
> + */
> + this_rq = task_rq_lock(p, &this_rf);
> +
> + /*
> + * Since we let go of the rq lock, the task may have been
> + * woken or migrated to another rq before we got the
> + * task_rq_lock. So re-check we're on the same RQ. If
> + * not, the task has already been migrated and that CPU
> + * will handle any futher migrations.
> + */
> + if (this_rq != rq)
> + goto err_out;
> +
> + /* Similarly, if we've been dequeued, someone else will wake us */
> + if (!task_on_rq_queued(p))
> + goto err_out;
> +
> + /*
> + * Since we should only be calling here from __schedule()
> + * -> find_proxy_task(), no one else should have
> + * assigned current out from under us. But check and warn
> + * if we see this, then bail.
> + */
> + if (task_current(this_rq, p) || task_on_cpu(this_rq, p)) {
> + WARN_ONCE(1, "%s rq: %i current/on_cpu task %s %d on_cpu: %i\n",
> + __func__, cpu_of(this_rq),
> + p->comm, p->pid, p->on_cpu);
> + goto err_out;
> }
> - return NULL;
> +
> + update_rq_clock(this_rq);
> + proxy_resched_idle(this_rq);
I still think this is too late, and only required if we are moving the
donor. Can we do this before we drop the rq_lock so that a remote
wakeup doesn't need to clear the this? (although I think we don't have
that bit in the ttwu path anymore and we rely on the schedule() bits
completely for return migration on this version - any particular
reason?).
> + deactivate_task(this_rq, p, DEQUEUE_NOCLOCK);
> + cpu = select_task_rq(p, p->wake_cpu, &wake_flag);
> + set_task_cpu(p, cpu);
> + target_rq = cpu_rq(cpu);
> + clear_task_blocked_on(p, NULL);
> + task_rq_unlock(this_rq, p, &this_rf);
> +
> + attach_one_task(target_rq, p);
I'm still having a hard time believing we cannot use wake_up_process()
but let me look more into that tomorrow when the sun rises.
> +
> + /* Finally, re-grab the origianl rq lock and return to pick-again */
> + raw_spin_rq_lock(rq);
> + rq_repin_lock(rq, rf);
> + update_rq_clock(rq);
> + return;
> +
> +err_out:
> + task_rq_unlock(this_rq, p, &this_rf);
> + raw_spin_rq_lock(rq);
> + rq_repin_lock(rq, rf);
> + update_rq_clock(rq);
> }
>
> /*
--
Thanks and Regards,
Prateek
next prev parent reply other threads:[~2026-03-15 17:38 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-13 2:30 [PATCH v25 0/9] Simple Donor Migration for Proxy Execution John Stultz
2026-03-13 2:30 ` [PATCH v25 1/9] sched: Make class_schedulers avoid pushing current, and get rid of proxy_tag_curr() John Stultz
2026-03-13 13:48 ` Juri Lelli
2026-03-13 17:53 ` John Stultz
2026-03-15 16:26 ` K Prateek Nayak
2026-03-17 4:49 ` John Stultz
2026-03-17 5:41 ` K Prateek Nayak
2026-03-17 6:04 ` John Stultz
2026-03-17 7:52 ` K Prateek Nayak
2026-03-17 18:35 ` John Stultz
2026-03-18 13:36 ` Peter Zijlstra
2026-03-18 13:52 ` Peter Zijlstra
2026-03-18 17:55 ` K Prateek Nayak
2026-03-18 20:30 ` John Stultz
2026-03-18 20:34 ` Peter Zijlstra
2026-03-18 20:35 ` John Stultz
2026-03-18 12:55 ` Peter Zijlstra
2026-03-18 18:01 ` K Prateek Nayak
2026-03-13 2:30 ` [PATCH v25 2/9] sched: Minimise repeated sched_proxy_exec() checking John Stultz
2026-03-15 17:01 ` K Prateek Nayak
2026-03-13 2:30 ` [PATCH v25 3/9] locking: Add task::blocked_lock to serialize blocked_on state John Stultz
2026-03-13 2:30 ` [PATCH v25 4/9] sched: Fix modifying donor->blocked on without proper locking John Stultz
2026-03-13 2:30 ` [PATCH v25 5/9] sched/locking: Add special p->blocked_on==PROXY_WAKING value for proxy return-migration John Stultz
2026-03-13 2:30 ` [PATCH v25 6/9] sched: Add assert_balance_callbacks_empty helper John Stultz
2026-03-13 2:30 ` [PATCH v25 7/9] sched: Add logic to zap balance callbacks if we pick again John Stultz
2026-03-13 2:30 ` [PATCH v25 8/9] sched: Move attach_one_task and attach_task helpers to sched.h John Stultz
2026-03-15 16:34 ` K Prateek Nayak
2026-03-16 23:34 ` John Stultz
2026-03-17 2:29 ` K Prateek Nayak
2026-03-13 2:30 ` [PATCH v25 9/9] sched: Handle blocked-waiter migration (and return migration) John Stultz
2026-03-15 17:38 ` K Prateek Nayak [this message]
2026-03-18 19:07 ` John Stultz
2026-03-18 6:35 ` Juri Lelli
2026-03-18 6:56 ` K Prateek Nayak
2026-03-18 10:16 ` Juri Lelli
2026-03-18 12:59 ` Peter Zijlstra
2026-03-19 12:49 ` Peter Zijlstra
2026-03-19 21:26 ` John Stultz
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=f092c21f-2e48-4e4e-90ac-af6a05dc7c28@amd.com \
--to=kprateek.nayak@amd.com \
--cc=Metin.Kaya@arm.com \
--cc=boqun.feng@gmail.com \
--cc=bsegall@google.com \
--cc=daniel.lezcano@linaro.org \
--cc=dietmar.eggemann@arm.com \
--cc=hupu.gm@gmail.com \
--cc=joelagnelf@nvidia.com \
--cc=jstultz@google.com \
--cc=juri.lelli@redhat.com \
--cc=kernel-team@android.com \
--cc=kuyo.chang@mediatek.com \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=qyousef@layalina.io \
--cc=rostedt@goodmis.org \
--cc=suleiman@google.com \
--cc=tglx@linutronix.de \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
--cc=will@kernel.org \
--cc=xuewen.yan94@gmail.com \
--cc=zezeozue@google.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox