Re: [PATCH v4] sched/proxy_exec: Detect cycles in proxy walks

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Zhidao,

On 7/17/2026 6:25 PM, zhidao su (Xiaomi) wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 2e7cde033a319..1a9d3decd66e0 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2221,6 +2221,9 @@ void activate_task(struct rq *rq, struct task_struct *p, int flags)
>  	if (task_on_rq_migrating(p))
>  		flags |= ENQUEUE_MIGRATED;
>  
> +#ifdef CONFIG_SCHED_PROXY_EXEC
> +	p->proxy_pick_seq = 0;
> +#endif

nit.

I think it would be better if we have a sched_proxy_enqueue_task()
wrapper around this to avoid sprinkling #ifdef in otherwise what is a
clean function.

Also you can bail out early for !sched_proxy_exec() to avoid this write
for users who disable proxy execution at runtime.

>  	enqueue_task(rq, p, flags);
>  
>  	WRITE_ONCE(p->on_rq, TASK_ON_RQ_QUEUED);
> @@ -6724,6 +6727,7 @@ static bool try_to_block_task(struct rq *rq, struct task_struct *p,
>  }
>  
>  #ifdef CONFIG_SCHED_PROXY_EXEC
> +
>  static inline void proxy_set_task_cpu(struct task_struct *p, int cpu)
>  {
>  	unsigned int wake_cpu;
> @@ -6839,14 +6843,14 @@ static void proxy_migrate_task(struct rq *rq, struct rq_flags *rf,
>  }
>  
>  /*
> - * Find runnable lock owner to proxy for mutex blocked donor
> + * Find runnable lock owner to proxy for a blocked donor
>   *
>   * Follow the blocked-on relation:
>   *
>   *                ,-> task
>   *                |     | blocked-on
>   *                |     v
> - *  blocked_donor |   mutex
> + *  blocked_donor | blocking primitive
>   *                |     | owner
>   *                |     v
>   *                `-- task
> @@ -6874,6 +6878,8 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  	struct task_struct *p;
>  	int owner_cpu;
>  
> +	rq->proxy_pick_seq++;
> +
>  	/* Follow blocked_on chain. */
>  	for (p = donor; p->is_blocked; p = owner) {
>  		/* if its PROXY_WAKING, do return migration or run if current */
> @@ -6905,6 +6911,14 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  			return NULL;
>  		}
>  
> +		if (p->proxy_pick_seq == rq->proxy_pick_seq) {
> +			WARN_ONCE(1, "sched/pe: deadlock cycle detected, pid %d\n",

You can use pr_warn_once().

> +				  p->pid);
> +			__clear_task_blocked_on(p, NULL);
> +			goto deactivate;
> +		}
> +		p->proxy_pick_seq = rq->proxy_pick_seq;
> +

Do we need to do this here too? ...

>  		if (task_current(rq, p))
>  			curr_in_chain = true;
>  
> @@ -6990,6 +7004,13 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>  			 */
>  			return proxy_resched_idle(rq);
>  		}
> +
> +		if (owner->proxy_pick_seq == rq->proxy_pick_seq) {
> +			WARN_ONCE(1, "sched/pe: deadlock cycle detected, pid %d\n",
> +				  p->pid);
> +			__clear_task_blocked_on(p, NULL);
> +			goto deactivate;
> +		}

... This should be sufficient right? You can just set

   owner->proxy_pick_seq = rq->proxy_pick_seq;

below once owner is finalized instead of setting "p->proxy_pick_seq"
very early above.

>  		/*
>  		 * OK, now we're absolutely sure @owner is on this
>  		 * rq, therefore holding @rq->lock is sufficient to

Apart from those, the patch looks good. With the comments addressed,
feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>

-- 
Thanks and Regards,
Prateek