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/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)
>> +{
>> +	struct rq_flags rf;
>> +	int target_cpu = cpu_of(target_rq);
>> +
>> +	scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
>> +		/* If task is no longer blocked, don't activate */
>> +		if (!p->is_blocked)
>> +			return;
>> +
>> +		/* Avoid racing with ttwu */
>> +		if (READ_ONCE(p->__state) == TASK_WAKING)
>> +			return;
>> +
>> +		if (READ_ONCE(p->on_rq)) {
>> +			/*
>> +			 * We raced with a non mutex handoff activation of p.
>> +			 * That activation will also take care of activating
>> +			 * all of the tasks after p in the blocked_head list,
>> +			 * so we're done here.
>> +			 */
>> +			return;
>> +		}
> 
> Is a racing activation the only possible case here? When
> proxy_enqueue_on_owner() calls block_task() on a fair task that isn't
> eligible, DELAY_DEQUEUE keeps it on_rq with se.sched_delayed set. In
> that case p simply won't donate until it's woken by the mutex, so
> nothing nasty shoudl happen, but maybe it's worth updating the comment
> to describe this case as well?
> 
>> +		if (task_on_cpu(task_rq(p), p)) {
>> +			/*
>> +			 * Its possible this activation is very late, and
>> +			 * we already were woken up and are running on a
>> +			 * different cpu. If that task blocked, it could be
>> +			 * dequeued (so on_rq == 0), but still on_cpu.
>> +			 * Bail in this case, as we definitely don't want to
>> +			 * activate a task when its on_cpu elsewhere.
>> +			 */
>> +			return;
>> +		}
>> +
>> +		scoped_guard (raw_spinlock, &p->blocked_lock) {
>> +			/*
>> +			 * Again, if activation is delayed, it's possible the
>> +			 * task has been woken and is no longer blocked_on,
>> +			 * but went back to sleep, so double check blocked_on
>> +			 * before activating onto the owner's rq
>> +			 */
>> +			if (!task_is_blocked(p))
>> +				return;
>> +		}
>> +
>> +		/*
>> +		 * Have to make sure we handle nr_iowait adjustment before
>> +		 * we call proxy_set_task_cpu() to ensure we are adjusting
>> +		 * the same runqueue we left (where block_task()
>> +		 * incremented nr_iowait).
>> +		 */
>> +		if (p->in_iowait) {
>> +			delayacct_blkio_end(p);
>> +			atomic_dec(&task_rq(p)->nr_iowait);
>> +		}
>> +
>> +		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?

> 
>> +		rq_lock_irqsave(target_rq, &rf);
>> +		/*
>> +		 * proxy_enqueue_on_owner() called block_task() which
>> +		 * increments nr_uninterruptible, so we need to reverse
>> +		 * that when we activate the blocked waiter
>> +		 */
>> +		if (p->sched_contributes_to_load)
>> +			target_rq->nr_uninterruptible--;
>> +		update_rq_clock(target_rq);
>> +		activate_task(target_rq, p, en_flags);
>> +		resched_curr(target_rq);
>> +		rq_unlock_irqrestore(target_rq, &rf);
>> +	}
>> +}

-- 
Thanks and Regards,
Prateek