Re: [PATCH 1/6] sched/proxy: Remove superfluous clear_task_blocked_in()
From: K Prateek Nayak
Date: Fri May 29 2026 - 04:05:33 EST
Hello John,
On 5/29/2026 12:18 PM, John Stultz wrote:
> Bascially we can get in a situation where (sorry this gets a bit convoluted):
>
> 1) On CPU1, __mutex_lock_common, we set task A
> blocked_on/TASK_UNINTERRUPTABLE, and call into __schedule().
>
> 2) On CPU2, task B who holds the mutex calls __mutex_unlock_slowpath()
> and sets task A as PROXY_WAKING and starts to call into
> wake_up_task().
>
> 3) On CPU1, in __schedule() we pick_next_task(), which returns task A,
> which is_blocked. We call find_proxy_task() and note task A is
> PROXY_WAKING. Since its also current, we take the short-cut and clear
> is_blocked and blocked_on and return task A to run.
>
> 4) On CPU2, try_to_wake_up() hits ttwu_runnable(), and
> proxy_needs_return() returns false as A->blocked_on is zero.
>
> 5) On CPU 1, task A is running, it grabs the lock it was waiting for
> and exits __mutex_lock_common. It then enters __mutex_lock_common to
> grab a different mutex that is already locked. It sets itself
> blocked_on/TASK_INTERRUPTABLE and calls into __schedule()
>
> 6) On CPU2, ttwu_runnable() continues, and calls ttwu_do_wakeup(),
> which clears A->is_blocked and sets the A->__state TASK_RUNNING
>
> 7) On CPU3, task C that holds the mutex A is waiting on, calls
> __mutex_unlock_slowpath, setting A as PROXY_WAKING and calls into
> wake_up_task()
>
> 8) On CPU1, in __schedule() pick_next_task() again returns task A. But
> is_blocked is now zero, so we just return task A, even though
> blocked_on is PROXY_WAKING.
>
> 9) On CPU1, task A gets back to the __mutex_lock_common() loop, calls
> set_task_blocked_on() and trips warnings as A->blocked_on is still
> PROXY_WAKING.
Oh geez! Me tries to visualize:
CPU1 CPU2 CPU3
==== ==== ====
__mutex_lock_common(MutexA)
set_task_blocked_on(TaskA, MutexA)
set_current_state(TASK_UNINTERRUPTABLE) __mutex_unlock_slowpath(MutexA)
... set_task_blocked_on_waking(TaskA)
schedule_preempt_disabled() wake_up_process(TaskA)
__schedule() /* (1) */ ... /* (2) */
if (prev_state &..)
TaskA->is_blocked = 0;
next = TaskA
find_proxy_task(TaskA)
/* TaskA-> blocked_on == TASK_WAKING */
clear_task_blocked_on(TaskA, NULL);
TaskA->is_blocked = 0;
...
next = TaskA /* (3) */
rq_unlock(CPU1) try_to_wake_up(TaskA)
rq_lock(CPU1)
ttwu_runnable()
/* TaskA->blocked_on == 0 (4) */
...
set_curent_state(TASK_RUNNING)
...
mutex_lock(MutexB)
__mutex_lock_common(MutexB)
...
set_task_blocked_on(TaskA, MutexB)
set_current_state(TASK_UNINTERRUPTABLE)
schedule_preempt_disabled()
__schedule() /* (5) */ ... __mutex_unlock_slowpath(MutexB)
ttwu_do_wakeup(TaskA) /* (6) */ set_task_blocked_on_waking(TaskA) /* (7) */
rq_unlock(CPU1)
rq_lock(CPU1)
/* TaskA->__state == TASK_RUNNING */
next = TasKA;
if (TaskA->is_blocked /* False */ && TaskA->blocked_on /* PROXY_WAKING */)
/* Skip */
next = TaskA; /* (8) */
set_task_blocked_on(p, MutexB)
!!! p->blocked_on != MutexB !!!
Yup! That is a concern too then!
I think we can just squash the PROXY_WAKING removal with "p->is_blocked"
introduction and a part of this problem should go away since unlocks
always clear task->blocked_on then.
>
> The chunk you drop in this patch effectively clears blocked_on in
> whenever we notice current is TASK_RUNNING, which keeps us in sync to
> avoid this.
>
> I'm also wondering if dropping the shortcut returns in
> find_proxy_task() that clear blocked_on and is_blocked is maybe the
> right thing? Just to reduce the cases we have to wrangle here.
> Even so, at least in my initial testing in doing so with just this
> patch that doesn't seem sufficient as it seems ttwu() can still race
> setting A->__state TASK_RUNNING vs find_proxy_task() hitting a
> proxy_deactive() case tripping the TASK_RUNNING warn-on.
Yeah, I tripped on those bits in the parallel thread. You can
perhaps give those changes a try.
>
> Maybe we just need to move the removal of this chunk till after we
> drop PROXY_WAKING?
That would be safer, yes.
--
Thanks and Regards,
Prateek