mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: K Prateek Nayak <kprateek.nayak@amd.com>
To: <soolaugust@gmail.com>, John Stultz <jstultz@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, "Mel Gorman" <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] sched/proxy_exec: Break cyclic proxy chains by deactivating blocked tasks
Date: Wed, 15 Jul 2026 02:16:30 +0530	[thread overview]
Message-ID: <31e87e29-0e24-4448-a30e-c0d05dec2e2c@amd.com> (raw)
In-Reply-To: <20260714152220.4046736-1-soolaugust@gmail.com>

Hello Zhidao,

On 7/14/2026 8:51 PM, soolaugust@gmail.com wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2e7cde033a319..cff1a3b58660b 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6724,6 +6724,8 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
>  }
>  
>  #ifdef CONFIG_SCHED_PROXY_EXEC
> +#define MAX_PROXY_CHAIN_DEPTH	1024

I'm not really a fan of the arbitrary MAX_PROXY_CHAIN_DEPTH.

> +
>  static inline void proxy_set_task_cpu(struct task_struct *p, int cpu)
>  {
>  	unsigned int wake_cpu;
> @@ -6873,6 +6875,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  	int this_cpu = cpu_of(rq);
>  	struct task_struct *p;
>  	int owner_cpu;
> +	int chain_depth = 0;
>  
>  	/* Follow blocked_on chain. */
>  	for (p = donor; p->is_blocked; p = owner) {
> @@ -6905,6 +6908,13 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  			return NULL;
>  		}
>  
> +		if (++chain_depth > MAX_PROXY_CHAIN_DEPTH) {
> +			WARN_ONCE(1, "sched/pe: proxy chain depth exceeded %d, pid %d\n",
> +				  MAX_PROXY_CHAIN_DEPTH, p->pid);
> +			__clear_task_blocked_on(p, NULL);
> +			goto deactivate;
> +		}
> +
>  		if (task_current(rq, p))
>  			curr_in_chain = true;
>  
> @@ -6923,6 +6933,13 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  			goto deactivate;
>  		}
>  
> +		if (owner == donor) {

So let us take a step back. Say you have a genuine case of:

  A -> B -> C -> D
       ^         |
       |         |
       +---------+

  donor = A
  Cycle is somewhere in-between the chain

Now, we have a "p->blocked_donor" back link that we can probably
leverage to detect this cycle except we don't clear a
p->blocked_donor link when find_proxy_task() returns idle or NULL
so we can have stale p->blocked_donor links for queued tasks but
they are so far harmless.

Me goes and thinks ...

Since p->blocked_donor isn't used for chain migration yet, does
something like below help your deadlock case?

  (Prepared on top of tip:sched/core; Lightly tested)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2e7cde033a31..fa1d9443f744 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6874,6 +6874,9 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
 	struct task_struct *p;
 	int owner_cpu;
 
+	/* Increment proxy_pick_seq such that last 4 bits are always set. */
+	rq->proxy_pick_seq += 0x10;
+
 	/* Follow blocked_on chain. */
 	for (p = donor; p->is_blocked; p = owner) {
 		/* if its PROXY_WAKING, do return migration or run if current */
@@ -6990,12 +6993,28 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
 			 */
 			return proxy_resched_idle(rq);
 		}
+
+		/* Cyclic deadlock detection. */
+		if (((u64)owner->blocked_donor) & 0xF) {
+			u64 pick_seq = (u64)owner->blocked_donor;
+
+			/*
+			 * Owner was already seen during this find_proxy_task() loop.
+			 * Deactivate the task since there is a loop in proxy-chain.
+			 */
+			if (rq->proxy_pick_seq == pick_seq) {
+				__clear_task_blocked_on(p, NULL);
+				goto deactivate;
+			}
+		}
+
 		/*
 		 * OK, now we're absolutely sure @owner is on this
 		 * rq, therefore holding @rq->lock is sufficient to
 		 * guarantee its existence, as per ttwu_remote().
 		 */
 		owner->blocked_donor = p;
+		p->blocked_donor = (void *)rq->proxy_pick_seq;
 	}
 	WARN_ON_ONCE(owner && !owner->on_rq);
 	return owner;
@@ -9057,6 +9076,9 @@ void __init sched_init(void)
 		raw_spin_lock_init(&rq->cpu_epoch_lock);
 		rq->cpu_epoch_next = jiffies;
 #endif
+#ifdef CONFIG_SCHED_PROXY_EXEC
+		rq->proxy_pick_seq = 0xf;
+#endif
 
 		zalloc_cpumask_var_node(&rq->scratch_mask, GFP_KERNEL, cpu_to_node(i));
 	}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 26ae13c86b69..452cbcd3a8c5 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1154,6 +1154,7 @@ struct rq {
 #ifdef CONFIG_SCHED_PROXY_EXEC
 	struct task_struct __rcu	*donor;  /* Scheduling context */
 	struct task_struct __rcu	*curr;   /* Execution context */
+	u64			proxy_pick_seq;  /* find_proxy_task() seq */
 #else
 	union {
 		struct task_struct __rcu *donor; /* Scheduler context */
---
base-commit: 04998aa54848f15332202d0bea008d2ca1ed1713

This assumes task_struct pointers are 4-bytes aligned.

Only rq->curr->blocked_donor matters currently so it should be okay
to clobber the rest of the chain. If this is okay, perhaps we can
just stash the pick_seq copy in task_struct.

Thoughts?

> +			WARN_ONCE(1, "sched/pe: deadlock cycle detected, pid %d\n",
> +				  p->pid);
> +			__clear_task_blocked_on(p, NULL);
> +			goto deactivate;
> +		}
> +
>  		if (!READ_ONCE(owner->on_rq) || owner->se.sched_delayed) {
>  			/* XXX Don't handle blocked owners/delayed dequeue yet */
>  			if (curr_in_chain)

-- 
Thanks and Regards,
Prateek


  reply	other threads:[~2026-07-14 20:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-14  5:36 [PATCH] sched/proxy_exec: Limit find_proxy_task() chain depth to prevent CPU hang soolaugust
2026-04-21  2:27 ` John Stultz
2026-04-21  3:13   ` K Prateek Nayak
2026-04-21 12:02     ` zhidao su
2026-04-21 12:08     ` zhidao su
2026-07-14 15:21     ` [PATCH v2] sched/proxy_exec: Break cyclic proxy chains by deactivating blocked tasks soolaugust
2026-07-14 20:46       ` K Prateek Nayak [this message]
2026-07-15  2:37       ` soolaugust
2026-07-15  3:01         ` K Prateek Nayak
2026-07-15  8:50           ` zhidao su
2026-07-17 10:52       ` [PATCH v3] sched/proxy_exec: Detect cycles in proxy walks zhidao su (Xiaomi)
2026-07-17 11:50         ` K Prateek Nayak
2026-07-17 12:55           ` [PATCH v4] " zhidao su (Xiaomi)

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=31e87e29-0e24-4448-a30e-c0d05dec2e2c@amd.com \
    --to=kprateek.nayak@amd.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=soolaugust@gmail.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.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

Powered by JetHome