Re: [PATCH] futex: Fix might_sleep() warning in futex_pivot_pending()

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Aug 17, 2026 at 03:29:14PM +0800, Yao Kai wrote:
> Thanks! I think there is still a lost-wakeup window:
> 
>         T1                              T2
> 
>         add_wait_queue()
>           /* not visible to T2 */
>         futex_pivot_pending()
>           futex_ref_is_dead() = false
>                                         futex_ref_put() = true
>                                         wake_up_var()
>                                           waitqueue_active() = false
>                                             /* observes empty */
>                                           return
>         wait_woken()
>         schedule()
> 
> Since wake_up_var() uses a lockless waitqueue_active() check, I think
> we need to order the waitqueue insertion before the first condition
> check:
> 
>   add_wait_queue(__wq_head, &__wbq_entry.wq_entry);
> 
>   /*
>    * Pairs with the fully ordered refcount operation before wake_up_var().
>    * Ensures either the waker sees this waiter or we see the dead refcount.
>    */
>   smp_mb();

Well, add_wait_queue() has UNLOCK(&wq_head->lock) and
futex_pivot_pending() has LOCK(&mmph->lock), giving an UNLOCK+LOCK
consistency, which IIRC is RCtso if you're on PowerPC and RCsc
everywhere else.

So yeah, this needs more. But I would instead suggest we use:

	smp_mb__after_spinlock().

Anyway, for this to matter one way or the other, the other side of this
also needs a barrier. But it looks like futex_ref_put() already implies
enough. When in atomic mode it implies a full smp_mb().

>   while (!futex_pivot_pending(mm))
>           wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE,
>                      MAX_SCHEDULE_TIMEOUT);
> 
> The rc check can be dropped because MAX_SCHEDULE_TIMEOUT does not expire.

Indeed, I had realized this after sending :-)

Something like so then?

---
diff --git a/include/linux/wait.h b/include/linux/wait.h
index dce055e6add3..7e215330199c 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -1228,6 +1228,7 @@ long prepare_to_wait_event(struct wait_queue_head *wq_head, struct wait_queue_en
 void finish_wait(struct wait_queue_head *wq_head, struct wait_queue_entry *wq_entry);
 long wait_woken(struct wait_queue_entry *wq_entry, unsigned mode, long timeout);
 int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key);
+int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key);
 int autoremove_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key);
 
 #define DEFINE_WAIT_FUNC(name, function)					\
diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
index ace7379d627d..553d7b23e3ad 100644
--- a/include/linux/wait_bit.h
+++ b/include/linux/wait_bit.h
@@ -32,6 +32,7 @@ int out_of_line_wait_on_bit_timeout(unsigned long *word, int, wait_bit_action_f
 int out_of_line_wait_on_bit_lock(unsigned long *word, int, wait_bit_action_f *action, unsigned int mode);
 struct wait_queue_head *bit_waitqueue(unsigned long *word, int bit);
 extern void __init wait_bit_init(void);
+extern struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg);
 
 int wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key);
 
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index a7c2a6242718..bd9fb0b17ee6 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -46,6 +46,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 #include <linux/kmemleak.h>
+#include <linux/wait_bit.h>
 
 #include <vdso/futex.h>
 
@@ -1886,11 +1887,34 @@ static int futex_hash_allocate(unsigned int hash_slots, unsigned int flags)
 		futex_hash_bucket_init(&fph->queues[i]);
 
 	if (custom) {
+		struct wait_bit_queue_entry __wbq_entry;
+		struct wait_queue_head *__wq_head;
+
 		/*
 		 * Only let prctl() wait / retry; don't unduly delay clone().
 		 */
 again:
-		wait_var_event(mm, futex_pivot_pending(mm));
+		__wq_head = __var_waitqueue(mm);
+		init_wait_var_entry(&__wbq_entry, mm, 0);
+		__wbq_entry.wq_entry.func = woken_wake_bit_function;
+		add_wait_queue(__wq_head, &__wbq_entry.wq_entry);
+
+		/*
+		 * add_wait_queue()		futex_ref_put()
+		 * MB (this)			MB (implied)
+		 * futex_pivot_pending()	wake_up_var()
+		 *                                waitqueue_active()
+		 *
+		 * Notably, it must not be possible to see
+		 * !futex_pivot_pending() && !waitqueue_active().
+		 */
+		smp_mb__after_spinlock();
+
+		while (!futex_pivot_pending(mm) &&
+		       wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE,
+				  MAX_SCHEDULE_TIMEOUT))
+			/* empty */;
+		remove_wait_queue(__wq_head, &__wbq_entry.wq_entry);
 	}
 
 	scoped_guard(mutex, &mm->futex.phash.lock) {
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 20f27e2cf7ae..d033f600f48c 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -5,6 +5,7 @@
  * (C) 2004 Nadia Yvette Chambers, Oracle
  */
 #include "sched.h"
+#include <linux/wait_bit.h>
 
 void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *name, struct lock_class_key *key)
 {
@@ -463,3 +464,17 @@ int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sy
 	return default_wake_function(wq_entry, mode, sync, key);
 }
 EXPORT_SYMBOL(woken_wake_function);
+
+int woken_wake_bit_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *arg)
+{
+	struct wait_bit_key *key = __var_wake_key(wq_entry, arg);
+	if (!key)
+		return 0;
+
+	/* Pairs with the smp_store_mb() in wait_woken(). */
+	smp_mb(); /* C */
+	wq_entry->flags |= WQ_FLAG_WOKEN;
+
+	return default_wake_function(wq_entry, mode, sync, key);
+}
+EXPORT_SYMBOL(woken_wake_bit_function);
diff --git a/kernel/sched/wait_bit.c b/kernel/sched/wait_bit.c
index 1088d3b7012c..e8127e83a48f 100644
--- a/kernel/sched/wait_bit.c
+++ b/kernel/sched/wait_bit.c
@@ -167,9 +167,7 @@ wait_queue_head_t *__var_waitqueue(void *p)
 }
 EXPORT_SYMBOL(__var_waitqueue);
 
-static int
-var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode,
-		  int sync, void *arg)
+struct wait_bit_key *__var_wake_key(struct wait_queue_entry *wq_entry, void *arg)
 {
 	struct wait_bit_key *key = arg;
 	struct wait_bit_queue_entry *wbq_entry =
@@ -177,6 +175,17 @@ var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode,
 
 	if (wbq_entry->key.flags != key->flags ||
 	    wbq_entry->key.bit_nr != key->bit_nr)
+		return NULL;
+
+	return key;
+}
+
+static int
+var_wake_function(struct wait_queue_entry *wq_entry, unsigned int mode,
+		  int sync, void *arg)
+{
+	struct wait_bit_key *key = __var_wake_key(wq_entry, arg);
+	if (!key)
 		return 0;
 
 	return autoremove_wake_function(wq_entry, mode, sync, key);