Re: [PATCH 2/2] locking: mutex: Fix proxy-exec potentially deactivating tasks marked TASK_RUNNING

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

Hello John,

On 4/28/2026 12:08 AM, John Stultz wrote:
> Vineeth found came up with a test driver that could trip up
> workqueue stalls. After fixing one issue this test found,
> Vineeth reported the test was still failing.
> 
> Greatly simplified, a task that tries to take a mutex already
> owned by another task that is sleeping, can hit a edge case in
> the mutex_lock_common() case.
> 
> If the task fails to get the lock, calls into schedule, but gets
> a spurious wakeup, it will find that it is first waiter, and
> go into the mutex_optimistic_spin() logic. Though before calling
> mutex_optimistic_spin(), we clear task blocked_on state, since
> mutex_optimistic_spin() may call schedule() if need_resched() is
> set.
> 
> After mutex_optimistic_spin() fails, we set blocked_on again,
> restart the main mutex loop, try to take the lock and call into
> schedule_preempt_disabled().
> 
> From there, with proxy-execution, we'll see the task is
> blocked_on, follow the chain, see the owner is sleeping and
> dequeue the waiting task from the runqueue.
> 
> This all sounds fine and reasonable.  But what I had missed is
> that in mutex_optimistic_spin(), not only do we call schedule()
> but we set TASK_RUNNABLE right before doing so.
> 
> This is ok for that invocation of schedule(). But when we come
> back we re-set the blocked_on we had just cleared, but we do not
> re-set the task state to TASK_INTERRUPTIBLE/UNINTERRUPTIBLE.
> 
> This means we have a task that is blocked_on & TASK_RUNNABLE,
> so when the proxy execution code dequeues the task, we are
> in trouble since future wakeups will be shortcut by the
> ttwu_state_match() check.
> 
> Thus, to avoid this, after mutex_optimistic_spin(), set the task
> state back when we set blocked_on.
> 
> Many many thanks again to Vineeth for his very useful testing
> driver that uncovered this long hidden bug, that I hadn't
> tripped in all my testing! Very impressed with the problems he's
> uncovered!
> 
> Reported-by: Vineeth Pillai <vineethrp@google.com>
> Tested-by: Vineeth Pillai <vineethrp@google.com>
> Signed-off-by: John Stultz <jstultz@google.com>

I think this too requires a:

Fixes: be41bde4c3a8 ("sched: Add an initial sketch of the find_proxy_task() function")

With that, feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>

> ---
> Cc: Vineeth Pillai <vineethrp@google.com>
> Cc: Sonam Sanju <sonam.sanju@intel.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Kunwu Chan <kunwu.chan@linux.dev>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Joel Fernandes <joelagnelf@nvidia.com>
> Cc: Qais Yousef <qyousef@layalina.io>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Juri Lelli <juri.lelli@redhat.com>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Waiman Long <longman@redhat.com>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Metin Kaya <Metin.Kaya@arm.com>
> Cc: Xuewen Yan <xuewen.yan94@gmail.com>
> Cc: K Prateek Nayak <kprateek.nayak@amd.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Suleiman Souhlal <suleiman@google.com>
> Cc: kuyo chang <kuyo.chang@mediatek.com>
> Cc: hupu <hupu.gm@gmail.com>
> Cc: kernel-team@android.com
> ---
>  kernel/locking/mutex.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index 09534628dc01a..a93d4c6bee1a3 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -763,6 +763,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
>  			raw_spin_lock_irqsave(&lock->wait_lock, flags);
>  			raw_spin_lock(&current->blocked_lock);
>  			__set_task_blocked_on(current, lock);
> +			set_current_state(state);
>  
>  			if (opt_acquired)
>  				break;

nit.

As a micro-optimization, you can probably move the
__set_task_blocked_on() and set_current_state() after this break.

-- 
Thanks and Regards,
Prateek