locking/mutex: Fix ww_mutex wait_list operations

Chaitanya, John and Mikhail reported commit 25500ba7e7 ("locking/mutex:
Remove the list_head from struct mutex") wrecked ww_mutex.

Specifically there were 2 issues:

 - __ww_waiter_prev() had the termination condition wrong; it would terminate
   when the previous entry was the first, which results in a truncated
   iteration: W3, W2, (no W1).

 - __mutex_add_waiter(@pos != NULL), as used by __ww_waiter_add() /
   __ww_mutex_add_waiter(); this inserts @waiter before @pos (which is what
   list_add_tail() does). But this should then also update lock->first_waiter.

Much thanks to Prateek for spotting the __mutex_add_waiter() issue!

Fixes: 25500ba7e7 ("locking/mutex: Remove the list_head from struct mutex")
Reported-by: "Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>
Closes: https://lore.kernel.org/r/af005996-05e9-4336-8450-d14ca652ba5d%40intel.com
Reported-by: John Stultz <jstultz@google.com>
Closes: https://lore.kernel.org/r/CANDhNCq%3Doizzud3hH3oqGzTrcjB8OwGeineJ3mwZuGdDWG8fRQ%40mail.gmail.com
Reported-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Closes: https://lore.kernel.org/r/CABXGCsO5fKq2nD9nO8yO1z50ZzgCPWqueNXHANjntaswoOh2Dg@mail.gmail.com
Debugged-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Link: https://patch.msgid.link/20260422092335.GH3102924%40noisy.programming.kicks-ass.net
master
Peter Zijlstra 2026-04-22 10:38:41 +02:00
parent 3bfdc63936
commit 0adc92b910
2 changed files with 59 additions and 15 deletions

View File

@ -198,27 +198,43 @@ static inline void __mutex_clear_flag(struct mutex *lock, unsigned long flag)
} }
/* /*
* Add @waiter to a given location in the lock wait_list and set the * Add @waiter to the @lock wait_list and set the FLAG_WAITERS flag if it's
* FLAG_WAITERS flag if it's the first waiter. * the first waiter.
*
* When @pos, @waiter is added before the waiter indicated by @pos. Otherwise
* @waiter will be added to the tail of the list.
*/ */
static void static void
__mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter, __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
struct mutex_waiter *first) struct mutex_waiter *pos)
__must_hold(&lock->wait_lock) __must_hold(&lock->wait_lock)
{ {
struct mutex_waiter *first = lock->first_waiter;
hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX); hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
debug_mutex_add_waiter(lock, waiter, current); debug_mutex_add_waiter(lock, waiter, current);
if (!first) if (pos) {
first = lock->first_waiter; /*
* Insert @waiter before @pos.
*/
list_add_tail(&waiter->list, &pos->list);
/*
* If @pos == @first, then @waiter will be the new first.
*/
if (pos == first)
lock->first_waiter = waiter;
return;
}
if (first) { if (first) {
list_add_tail(&waiter->list, &first->list); list_add_tail(&waiter->list, &first->list);
} else { return;
INIT_LIST_HEAD(&waiter->list);
lock->first_waiter = waiter;
__mutex_set_flag(lock, MUTEX_FLAG_WAITERS);
} }
INIT_LIST_HEAD(&waiter->list);
lock->first_waiter = waiter;
__mutex_set_flag(lock, MUTEX_FLAG_WAITERS);
} }
static void static void
@ -229,10 +245,8 @@ __mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
__mutex_clear_flag(lock, MUTEX_FLAGS); __mutex_clear_flag(lock, MUTEX_FLAGS);
lock->first_waiter = NULL; lock->first_waiter = NULL;
} else { } else {
if (lock->first_waiter == waiter) { if (lock->first_waiter == waiter)
lock->first_waiter = list_first_entry(&waiter->list, lock->first_waiter = list_next_entry(waiter, list);
struct mutex_waiter, list);
}
list_del(&waiter->list); list_del(&waiter->list);
} }

View File

@ -6,6 +6,19 @@
#define MUTEX_WAITER mutex_waiter #define MUTEX_WAITER mutex_waiter
#define WAIT_LOCK wait_lock #define WAIT_LOCK wait_lock
/*
* +--------+
* | first |
* +--------+
* |
* v
* +----+ +----+ +----+
* | W3 | <-> | W1 | <-> | W2 |
* +----+ +----+ +----+
* ^ ^
* +---------------------+
*/
static inline struct mutex_waiter * static inline struct mutex_waiter *
__ww_waiter_first(struct mutex *lock) __ww_waiter_first(struct mutex *lock)
__must_hold(&lock->wait_lock) __must_hold(&lock->wait_lock)
@ -13,26 +26,43 @@ __ww_waiter_first(struct mutex *lock)
return lock->first_waiter; return lock->first_waiter;
} }
/*
* for (cur = __ww_waiter_first(); cur; cur = __ww_waiter_next())
*
* Should iterate like: W1, W2, W3
*/
static inline struct mutex_waiter * static inline struct mutex_waiter *
__ww_waiter_next(struct mutex *lock, struct mutex_waiter *w) __ww_waiter_next(struct mutex *lock, struct mutex_waiter *w)
__must_hold(&lock->wait_lock) __must_hold(&lock->wait_lock)
{ {
w = list_next_entry(w, list); w = list_next_entry(w, list);
/*
* Terminate if the next entry is the first again, that has already
* been observed.
*/
if (lock->first_waiter == w) if (lock->first_waiter == w)
return NULL; return NULL;
return w; return w;
} }
/*
* for (cur = __ww_waiter_last(); cur; cur = __ww_waiter_prev())
*
* Should iterate like: W3, W2, W1
*/
static inline struct mutex_waiter * static inline struct mutex_waiter *
__ww_waiter_prev(struct mutex *lock, struct mutex_waiter *w) __ww_waiter_prev(struct mutex *lock, struct mutex_waiter *w)
__must_hold(&lock->wait_lock) __must_hold(&lock->wait_lock)
{ {
w = list_prev_entry(w, list); /*
* Terminate at the first entry, the previous entry of first is the
* last and that has already been observed.
*/
if (lock->first_waiter == w) if (lock->first_waiter == w)
return NULL; return NULL;
return w; return list_prev_entry(w, list);
} }
static inline struct mutex_waiter * static inline struct mutex_waiter *