Re: [RESEND][PATCH v21 2/6] sched/locking: Add blocked_on_state to provide necessary tri-state for proxy return-migration

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

Hello John,

On 9/19/2025 4:27 AM, John Stultz wrote:
>> Of the three {set,clear}_task_blcoked_on() usage:
>>
>>     $ grep -r "\(set\|clear\)_task_blocked_on" include/
>>     kernel/locking/mutex.c: __set_task_blocked_on(current, lock);
>>     kernel/locking/mutex.c: __clear_task_blocked_on(current, lock);
>>     kernel/locking/mutex.c: clear_task_blocked_on(current, lock);
>>
>> two can be converted directly and perhaps clear_task_blocked_on() can be
>> renamed as clear_task_blocked_on_set_runnable()?
>>
>> This is just me rambling on so feel free to ignore. I probably have to
>> train my mind enough to see __clear_task_blocked_on() not only clears
>> "blocked_on" but also sets task to runnable :)
> 
> Yeah, the case where we don't already hold the lock and want to do
> both gets more complex in that case.
> 
> Hrm. Maybe just the way the functions are organized in the header make
> it seem like we're managing two separate bits of state, where really
> they are ordered.
> I'll try to re-arrange that introducing the
> set_task_blocked_on/clear_task_blocked_on first, then the transition
> set_blocked_on_<state>() helpers after?
> Maybe that and  some comments will make that clearer?

Again that was me rambling. Even a small comment above
clear_task_blocked_on() would also be sufficient if the whole rework
turns out to be more extensive. 

> 
>>> @@ -6749,6 +6776,15 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
>>>
>>>       WARN_ON_ONCE(owner && !owner->on_rq);
>>>       return owner;
>>> +
>>> +     /*
>>> +      * NOTE: This logic is down here, because we need to call
>>> +      * the functions with the mutex wait_lock and task
>>> +      * blocked_lock released, so we have to get out of the
>>> +      * guard() scope.
>>> +      */
>>
>> I didn't know that was possible! Neat. Since cleanup.h has a note
>> reading:
>>
>>     ... the expectation is that usage of "goto" and cleanup helpers is
>>     never mixed in the same function.
>>
>> are there any concerns w.r.t. compiler versions etc. or am I just being
>> paranoid?
> 
> Hrrrrmmmm.  I hadn't seen that detail. :/   I guess I was just lucky
> it worked with my toolchain.

I have been too. Maybe it is okay to use a goto if folks know what
they are doing ¯\_(ツ)_/¯

Another idea is to have:

    bool deactivate_donor = false;

    for (p = donor; task_is_blocked(p); p = owner) {
        guard(raw_spinlock)(...);
        ...
        if (<condition> {
            deactivate_donor = true;
            break;
        }
        ...
    }
    if (deactivate_donor)
        return proxy_deactivate(rq, donor);

Can that work?

> 
> Oof. That may require reworking all this logic back to explicit
> lock/unlock calls, away from the guard() usage.
> 
> Let me think on if there's a better way.
> 
> Thanks so much again for pointing this out!
> -john

-- 
Thanks and Regards,
Prateek