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

[Peter Zijlstra]

On Tue, Aug 18, 2026 at 08:24:07PM +0800, Yao Kai wrote:

> > +		__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();
> 
> I still think we should use smp_mb() here, smp_mb__after_spinlock() only
> orders accesses preceding the lock acquisition against later accesses. The
> waitqueue insertion happens after that acquisition, so I don't think
> smp_mb__after_spinlock() covers it here.

I can definitely make it smp_mb() just to make you feel better, this is
not a fast path. But the smp_mb__after_spinlock() is sufficient in this
case.

Specifically, the situation is

	LOCK(&waitq->lock)
	[STORE] list_add()
	UNLOCK(&waitq->lock)

	smp_mb__after_spinlock()

	LOCK(&mmph->lock)
 	[LOAD] refcount
	UNLOCK(&mmph->lock)

That means the STORE and LOAD are separated by UNLOCK+LOCK, and on every
architecture *EXCEPT* PPC that implies smp_mb(). On PPC we need the
extra smp_mb() to 'upgrade' the LWSYNC to SYNC.

Specifically, on TSO (x86,s390,sparc64) the atomic op in LOCK implies a
full barrier, on ARM64 RELEASE+ACQUIRE is RCsc, etc.

There is a section on LOCKING in
tools/memory-model/Documentation/explanation.txt on this very subject.

> > +
> > +		while (!futex_pivot_pending(mm) &&
> > +		       wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE,
> > +				  MAX_SCHEDULE_TIMEOUT))
> > +			/* empty */;
> 
> Since MAX_SCHEDULE_TIMEOUT never returns zero, so I think this can be:
> 
> 		while (!futex_pivot_pending(mm))
> 		       wait_woken(&__wbq_entry.wq_entry, TASK_UNINTERRUPTIBLE,
> 				  MAX_SCHEDULE_TIMEOUT));

Well, you missed the '&&', but that is exactly what I wrote, no?

I just put the ';' behind a comment, to indicate it was an empty loop on
purpose. Some people (and static checkers/linters/etc.) get
upset/confused if there is no body on a loop construct.

Anyway, I've tested all this, selftests/futex trips this WARN without,
and runs to completion with this patch, so it must be perfect.

Let me go write a Changelog.