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

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

On 10/28/2026 3:31 PM, Juri Lelli wrote:
> On 28/10/26 09:06, K Prateek Nayak wrote:
>> Hello Juri,
>>
>> On 10/27/2026 9:52 PM, Juri Lelli wrote:
>>> Hello!
>>>
>>> On 26/10/26 09:56, K Prateek Nayak wrote:
>>>> On 10/24/2026 2:37 PM, Andrea Righi wrote:
>>>>>> +static void do_activate_blocked_waiter(struct rq *target_rq, struct task_struct *p, int en_flags)
>>>>>> +{
>>>
>>> ...
>>>
>>>>>> +		if (task_cpu(p) != target_cpu) {
>>>>>> +			en_flags |= ENQUEUE_MIGRATED;
>>>>>> +			psi_ttwu_dequeue(p);
>>>>>> +			proxy_set_task_cpu(p, target_cpu);
>>>>>> +		}
>>>>>
>>>>> Can this break SCHED_DEADLINE bw accounting?
>>>>>
>>>>> p is sleeping here (not TASK_WAKING, not TASK_ON_RQ_MIGRATING), so
>>>>> __set_task_cpu() skips migrate_task_rq_dl(): the old rq never does
>>>>> sub_rq_bw(), while ENQUEUE_MIGRATED makes the new rq do add_rq_bw(), so
>>>>> this_bw is leaked on the old rq. And if p is still dl_non_contending,
>>>>> task_contending() on the new rq just cancels the inactive timer, leaving
>>>>> running_bw on the old rq forever and making it underflow on the new rq
>>>>> the next time p blocks.
>>>>>
>>>>> set_task_cpu() doesn't help, because migrate_task_rq_dl() bails out
>>>>> unless TASK_WAKING, so I'm not sure how to fix this case...
>>>>
>>>> I wonder why "p->dl.dl_non_contending" is not sufficient to know if
>>>> there exists residual bandwidth on prev CPU. Seems to have been like
>>>> that since commit 209a0cbda7a0 ("sched/deadline: Improve the tracking
>>>> of active utilization").
>>>>
>>>> Only time task is non-contending is if it goes away with DEQUEUE_SLEEP
>>>> or switched_from_dl() with dl_runtime.
>>>>
>>>> The latter doesn't need migrating the timer as the timer will either
>>>> be cancelled if the task switches back to deadline, or will expire
>>>> where it was queued - set_task_cpu() has no effect in this case.
>>>>
>>>> Only other case that marks "dl_non_contending" is sleep + wakeup and
>>>> for that "p->dl.non_contending" should be sufficient. I don't think
>>>> we ever do a set_task_cpu() for a blocked task other than wakeup do
>>>> we?
>>>>
>>>> Juri, Peter, are we missing something subtle here?
>>>
>>> I think Andrea's concern is valid, but I'm not sure I follow what you
>>> propose Prateek (apologies!) or are you saying you don't see an issue
>>> here?
>>
>> Sorry for the confusion! I do see the problem and my question was why
>> can't the bailout in migrate_task_rq_dl() be something like:
>>
>>   (Lightly tested)
>>
>> 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?

-- 
Thanks and Regards,
Prateek