Re: [RFC PATCH 04/16] sched/core: Activate blocked donor when no owner is found

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

Hello Andrea,

On 8/26/2026 9:26 PM, Andrea Righi wrote:
> On Wed, Aug 26, 2026 at 06:28:48AM +0000, K Prateek Nayak wrote:
>> mutex_unlock_slowpath() follows:
>>
>>     if (owner & MUTEX_FLAG_HANDOFF)
>>         break /* ... and do __mutex_handoff() */
>>
>>     if (atomic_long_try_cmpxchg_release(&lock->owner, &owner, __owner_flags(owner))) {
>>         if (owner & MUTEX_FLAG_WAITERS)
>>             break; /* ... and wake up the forst waiter. */
> 
> nit: s/forst/first/

Ack

> 
>>
>> MUTEX_FLAG_HANDOFF is only set by first-waiter after it has been woken
>> up and in absence of MUTEX_FLAG_HANDOFF, the owner clears itself from
>> the lock_word and wakes up the first waiter to try a
>> __mutex_trylock_or_handoff().
>>
>> MUTEX_FLAG_HANDOFF exists to prevent new optimistic spinners from
>> trying to hijack the lock from waiter all the time and potentially
>> starving them but it is not necessary for MUTEX_FLAG_HANDOFF to be
>> always set in presence of a waiter.
>>
>> If a blocked donor is deactivated when no owner is observed, it may not
>> be woken up until it becomes the first waiter and is naturally woken up
>> which breaks proxy in the interim.
>>
>> Wake up the blocked donor and allow it to grab the lock when no owner is
>> observed. If the task manages to grab the lock, the block chain will
>> follow at the next proxy migration. If the task fails to grab the lock,
>> same situation is restored and everyone migrated to the CPU of new
>> owner.
>>
>> Fixes: f13beb010e4a ("sched: Have try_to_wake_up() handle return-migration for PROXY_WAKING case")
>> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
>> ---
>> XXX: Is there a better way to handle this? If we can confirm a owner in
>> find_proxy_task(), we don't need to do a spurious wakeup of every task
>> observing !owner.
>>
>> proxy_resched_idle() until owner appears in an option but it will spin
>> until next owner appears.
> 
> IIUC, the owner can remain NULL until the waiter selected by mutex_unlock() gets
> CPU time and acquires the mutex, so proxy_resched_idle() could spin for longer
> than just the unlock critical section.
> 
> Maybe we could instead force a handoff from mutex_unlock_slowpath() when proxy
> execution is enabled and the mutex has waiters? This would keep the owner
> identifiable and avoid waking every task that happens to observe !owner.

I think that negates some of the benefits of the optimistic spinning +
mutex_try_lock(). I'll see if it makes any difference to the benchmark
results if we always force a handoff for MUTEX_FLAG_WAITERS.

wait_lock should give enough guarantee that the waiter cannot simple
disappear before the handoff after MUTEX_FLAG_PICKUP is set since
waiter has to try at least one mutex_trylock() under wait_lock before
checking for pending signals.

Thank you for taking a look at the series and ack on the comment on
the previous patch.

> 
>> ---
>>  kernel/sched/core.c | 23 ++++++++++++++++++++---
>>  1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 4cd69b08b415..9d9db7ccf01f 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -6809,6 +6809,19 @@ static inline void proxy_reacquire_rq_lock(struct rq *rq, struct rq_flags *rf)
>>  	update_rq_clock(rq);
>>  }
>>  
>> +static void
>> +proxy_activate(struct rq *rq, struct rq_flags *rf, struct task_struct *p)
>> +	__must_hold(__rq_lockp(rq))
>> +{
>> +	lockdep_assert_rq_held(rq);
>> +	proxy_resched_idle(rq);
>> +	proxy_release_rq_lock(rq, rf);
>> +
>> +	wake_up_process(p);
>> +
>> +	proxy_reacquire_rq_lock(rq, rf);
>> +}
>> +
>>  /*
>>   * If the blocked-on relationship crosses CPUs, migrate @p to the
>>   * owner's CPU.
>> @@ -6934,14 +6947,15 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>>  			/*
>>  			 * If there is no owner, either clear blocked_on
>>  			 * and return p (if it is current and safe to
>> -			 * just run on this rq), or return-migrate the task.
>> +			 * just run on this rq), or wake the task to try
>> +			 * and grab the lock it is blocked on.
>>  			 */
>>  			__clear_task_blocked_on(p, NULL);
>> -			if (task_current(rq, p)) {
>> +			if (task_current(rq, p) || p->wake_cpu == task_cpu(p)) {
>>  				p->is_blocked = 0;
>>  				return p;
>>  			}
>> -			goto deactivate;
>> +			goto activate;
>>  		}
>>  
>>  		if (!READ_ONCE(owner->on_rq) || owner->se.sched_delayed) {
>> @@ -7029,6 +7043,9 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>>  	}
>>  	return owner;
>>  
>> +activate:
>> +	proxy_activate(rq, rf, p);
>> +	return NULL;
>>  deactivate:
>>  	proxy_deactivate(rq, p);
>>  	return NULL;
-- 
Thanks and Regards,
Prateek