public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: John Stultz <jstultz@google.com>,
	"Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>,
	willy@infradead.org, linux-kernel@vger.kernel.org,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
	"Kurmi, Suresh Kumar" <suresh.kumar.kurmi@intel.com>,
	"Saarinen, Jani" <jani.saarinen@intel.com>,
	ravitejax.veesam@intel.com
Subject: Re: Regression on linux-next (next-20260324 )
Date: Tue, 21 Apr 2026 16:37:52 +0200	[thread overview]
Message-ID: <20260421143752.GD1064669@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <95651a71-1adf-45ba-83eb-5744bc6d4a52@amd.com>

On Tue, Apr 21, 2026 at 06:24:20PM +0530, K Prateek Nayak wrote:
> 
> 
> On 4/21/2026 3:45 PM, Peter Zijlstra wrote:
> > On Mon, Apr 20, 2026 at 11:45:12PM -0700, John Stultz wrote:
> > 
> >> So I tripped over this in my own testing today preping proxy patches,
> >> bisecting it down to the same problematic commit 25500ba7e77c
> >> ("locking/mutex: Remove the list_head from struct mutex").
> >>
> >> Inteed it does seem related to ww_mutexes, as I can pretty easily
> >> reproduce it with defconfig + CONFIG_WW_MUTEX_SELFTEST=y  using
> >> qemu-system-x86
> >>
> >> Where the test will basically hang on bootup.
> > 
> > *groan* indeed. This of course means no CI is running this thing :-(
> > 
> > Anyway, yay for deterministic reproducer. Let me go prod at this.
> 
> So I managed to unblock the ww-mutext_test with:
> 
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index 186b463fe326..623c892c3742 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -209,8 +209,13 @@ __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
>  	hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
>  	debug_mutex_add_waiter(lock, waiter, current);
>  
> -	if (!first)
> +	if (!first) {
>  		first = lock->first_waiter;
> +	} else if (first == lock->first_waiter) {
> +		list_add_tail(&waiter->list, &first->list);
> +		lock->first_waiter = waiter;
> +		return;
> +	}
>  
>  	if (first) {
>  		list_add_tail(&waiter->list, &first->list);

> First hunk orders the first_waiter if we are attaching to the
> tail of current first_waiter which would have previously ended
> up next to list_head.

This is the case in __ww_mutex_add_waiter() where pos == first, right?

Argh, I see... yes. Perhaps something like the below though?

> diff --git a/kernel/locking/ww_mutex.h b/kernel/locking/ww_mutex.h
> index 016f0db892a5..2fcd6221fc64 100644
> --- a/kernel/locking/ww_mutex.h
> +++ b/kernel/locking/ww_mutex.h
> @@ -28,10 +28,9 @@ static inline struct mutex_waiter *
>  __ww_waiter_prev(struct mutex *lock, struct mutex_waiter *w)
>  	__must_hold(&lock->wait_lock)
>  {
> -	w = list_prev_entry(w, list);
>  	if (lock->first_waiter == w)
>  		return NULL;
> -
> +	w = list_prev_entry(w, list);
>  	return w;
>  }

> The second hunk deals with __ww_waiter_prev() - since we are
> traversing back from w, I guess we must first check if we are
> at the first_waiter already or not.

Yes, that second hunk is what I found yesterday, although my fix was
far more verbose. I like this one better ;-)


---
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 95f1822122a1..7d48d6f49f71 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -200,21 +200,34 @@ static inline void __mutex_clear_flag(struct mutex *lock, unsigned long flag)
  */
 static void
 __mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
-		   struct mutex_waiter *first)
+		   struct mutex_waiter *pos)
 {
+	struct mutex_waiter *first = lock->first_waiter;
+
 	hung_task_set_blocker(lock, BLOCKER_TYPE_MUTEX);
 	debug_mutex_add_waiter(lock, waiter, current);
 
-	if (!first)
-		first = lock->first_waiter;
+	if (pos) {
+		/*
+		 * 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) {
 		list_add_tail(&waiter->list, &first->list);
-	} else {
-		INIT_LIST_HEAD(&waiter->list);
-		lock->first_waiter = waiter;
-		__mutex_set_flag(lock, MUTEX_FLAG_WAITERS);
+		return;
 	}
+
+	INIT_LIST_HEAD(&waiter->list);
+	lock->first_waiter = waiter;
+	__mutex_set_flag(lock, MUTEX_FLAG_WAITERS);
 }
 
 static void
@@ -224,10 +237,8 @@ __mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
 		__mutex_clear_flag(lock, MUTEX_FLAGS);
 		lock->first_waiter = NULL;
 	} else {
-		if (lock->first_waiter == waiter) {
-			lock->first_waiter = list_first_entry(&waiter->list,
-							      struct mutex_waiter, list);
-		}
+		if (lock->first_waiter == waiter)
+			lock->first_waiter = list_next_entry(waiter, list);
 		list_del(&waiter->list);
 	}
 

  reply	other threads:[~2026-04-21 14:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-27 13:39 Regression on linux-next (next-20260324 ) Borah, Chaitanya Kumar
2026-03-27 16:31 ` Peter Zijlstra
2026-03-27 16:43   ` Peter Zijlstra
2026-03-30  8:26     ` Borah, Chaitanya Kumar
2026-03-30 19:50       ` Peter Zijlstra
2026-04-20 13:03         ` Peter Zijlstra
2026-04-21  6:45           ` John Stultz
2026-04-21 10:15             ` Peter Zijlstra
2026-04-21 12:54               ` K Prateek Nayak
2026-04-21 14:37                 ` Peter Zijlstra [this message]
2026-04-21 14:45                   ` Matthew Wilcox
2026-04-21 15:03                     ` Peter Zijlstra
2026-04-21 15:48                   ` K Prateek Nayak
2026-04-21 17:29                     ` John Stultz
2026-04-21 14:31           ` Borah, Chaitanya Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260421143752.GD1064669@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=chaitanya.kumar.borah@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=jani.saarinen@intel.com \
    --cc=jstultz@google.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ravitejax.veesam@intel.com \
    --cc=suresh.kumar.kurmi@intel.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox