Re: [RFC PATCH 1/2] locking: add mutex_lock_nospin()

[Peter Zijlstra]

On Wed, Mar 04, 2026 at 05:37:31PM +0800, Yafang Shao wrote:
> On Wed, Mar 4, 2026 at 5:03 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, Mar 04, 2026 at 03:46:49PM +0800, Yafang Shao wrote:
> > > Introduce mutex_lock_nospin(), a helper that disables optimistic spinning
> > > on the owner for specific heavy locks. This prevents long spinning times
> > > that can lead to latency spikes for other tasks on the same runqueue.
> >
> > This makes no sense; spinning stops on need_resched().
> 
> Hello Peter,
> 
> The condition to stop spinning on need_resched() relies on the mutex
> owner remaining unchanged. However, when multiple tasks contend for
> the same lock, the owner can change frequently. This creates a
> potential TOCTOU (Time of Check to Time of Use) issue.
> 
>   mutex_optimistic_spin
>       owner = __mutex_trylock_or_owner(lock);
>       mutex_spin_on_owner
>           // the __mutex_owner(lock) might get a new owner.
>           while (__mutex_owner(lock) == owner)
> 

How do these new owners become the owner? Are they succeeding the
__mutex_trylock() that sits before mutex_optimistic_spin() and
effectively starving the spinner?

Something like the below would make a difference if that were so.

---
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index c867f6c15530..0796e77a8c3b 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -521,7 +521,7 @@ static __always_inline bool
 mutex_optimistic_spin(struct mutex *lock, struct ww_acquire_ctx *ww_ctx,
 		      struct mutex_waiter *waiter)
 {
-	return false;
+	return __mutex_trylock(lock);
 }
 #endif
 
@@ -614,8 +614,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 	mutex_acquire_nest(&lock->dep_map, subclass, 0, nest_lock, ip);
 
 	trace_contention_begin(lock, LCB_F_MUTEX | LCB_F_SPIN);
-	if (__mutex_trylock(lock) ||
-	    mutex_optimistic_spin(lock, ww_ctx, NULL)) {
+	if (mutex_optimistic_spin(lock, ww_ctx, NULL)) {
 		/* got the lock, yay! */
 		lock_acquired(&lock->dep_map, ip);
 		if (ww_ctx)