Re: [PATCH v33 7/7] sched/proxy: Add deactivated (sleeping) owner handling to find_proxy_task()

[Juri Lelli <juri.lelli@redhat.com>]

On 28/10/26 15:41, K Prateek Nayak wrote:
> 
> 
> On 10/28/2026 3:31 PM, Juri Lelli wrote:
> > On 28/10/26 09:06, K Prateek Nayak wrote:

...

> >> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> >> index c0ebdcde5fe5..6a081c2b7191 100644
> >> --- a/kernel/sched/deadline.c
> >> +++ b/kernel/sched/deadline.c
> >> @@ -2654,19 +2654,23 @@ select_task_rq_dl(struct task_struct *p, int cpu, int flags)
> >>  
> >>  static void migrate_task_rq_dl(struct task_struct *p, int new_cpu __maybe_unused)
> >>  {
> >> -	struct rq_flags rf;
> >>  	struct rq *rq;
> >>  
> >> -	if (READ_ONCE(p->__state) != TASK_WAKING)
> >> +	/* Inactive timer is not armed. Nothing to migrate! */
> >> +	if (task_on_rq_migrating(p) || !p->dl.dl_non_contending)
> >>  		return;
> > 
> > But a task can be blocked/sleeping and non_contending if
> > task_non_contending() was called after 0-lag; in that case running_bw
> > was subtracted already (see zerolag_time < 0 -> dl_task case) but ..
> 
> I see what you mean and I didn't see that the sub_rq_bw() is outside
> the p->dl.dl_non_contending block but simply checking
> task_on_rq_migrating() for bail out should be enough to cover off
> !ENQUEUE_WAKEUP cases right?
> 
> > 
> >> -	rq = task_rq(p);
> >> +	/* Task must be fully blocked if we reach here. */
> >> +	WARN_ON_ONCE(!p->is_blocked);
> >> +
> >>  	/*
> >> -	 * Since p->state == TASK_WAKING, set_task_cpu() has been called
> >> -	 * from try_to_wake_up(). Hence, p->pi_lock is locked, but
> >> -	 * rq->lock is not... So, lock it
> >> +	 * set_task_cpu() has been called from try_to_wake_up().
> >> +	 * Hence, p->pi_lock is locked, but rq->lock is not...
> >> +	 * So, lock it
> >>  	 */
> >> -	rq_lock(rq, &rf);
> >> +	rq = task_rq(p);
> >> +	guard(rq_lock)(rq);
> >> +
> >>  	if (p->dl.dl_non_contending) {
> >>  		update_rq_clock(rq);
> >>  		sub_running_bw(&p->dl, &rq->dl);
> >> @@ -2681,7 +2685,6 @@ static void migrate_task_rq_dl(struct task_struct *p, int new_cpu __maybe_unused
> >>  		cancel_inactive_timer(&p->dl);
> >>  	}
> >>  	sub_rq_bw(&p->dl, &rq->dl);
> > 
> > .. we still need to sub_rq_bw for a migrated task, and I believe what
> > you are proposing would skip that?
> 
> Ack! But this is only needed for wakeup so just:
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index c0ebdcde5fe5..20c388b55c50 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2657,7 +2657,7 @@ static void migrate_task_rq_dl(struct task_struct *p, int new_cpu __maybe_unused
>  	struct rq_flags rf;
>  	struct rq *rq;
>  
> -	if (READ_ONCE(p->__state) != TASK_WAKING)
> +	if (task_on_rq_migrating(rq))
>  		return;
>  
>  	rq = task_rq(p);
> ---
> 
> should be enough then?

It is enough, but I now fear too 'broad'? :)

Looks like dl_task_offline_migration() ends up here via set_task_cpu();
in that case the task was throttled and found the cpu was running on has
been hotplugged so it needs a new cpu at replenishment. All bandwidth
accouting is done by dl_task_offline_migration() itself, the task is not
WAKING so migrate_task_rq_dl() has nothing to do and bails early
correctly. If we use task_on_rq_migrating() it won't bail early anymore
and double subtracting? Adding !p->is_blocked seems to cover this case
as well?