Re: [PATCH v2 1/2] futex/requeue: Fix rtmutex schedule preparation for requeue PI

[Yao Kai <yaokai34@huawei.com>]

On 8/24/2026 10:08 PM, Sebastian Andrzej Siewior wrote:
> On 2026-08-04 14:21:02 [+0200], Peter Zijlstra wrote:
>>> @@ -865,7 +866,14 @@ int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
>>>   	case Q_REQUEUE_PI_DONE:
>>>   		/* Requeue completed. Current is 'pi_blocked_on' the rtmutex */
>>>   		pi_mutex = &q.pi_state->pi_mutex;
>>> +		/*
>>> +		 * Requeue temporarily removes q from the hash bucket, so
>>> +		 * futex_do_wait() may skip schedule() even though the proxy
>>> +		 * waiter still has to block on the rtmutex.
>>> +		 */
>>> +		rt_mutex_pre_schedule();
>>>   		ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
>>> +		rt_mutex_post_schedule();
>>
> …
>> So the purpose of rt_mutex_pre_schedule() was to avoid the double waiter
>> enqueue for rt_mutex on RT, where sched_submit_work() will hit a
>> spinlock-nee-rtlock.
>>
>> So rt_mutex_pre_schedule() must happen before the rt_mutex is added as a
>> waiter. However, AFAICT we're already a waiter at the above spot, no? So
>> this cannot be right.
> 
> Urgh. So I missed this part entirely while reading it.
> Isn't the usage of this in futex_lock_pi() just to keep the assert
> quiet?
> We do add a waiter there (futex_lock_pi()) and this
> (rt_mutex_pre_schedule()) must be done before a waiter is enqueued so we
> can acquire the lock (mutex) during the blk_flush_plug() which is in
> general part of schedule().
> While doing all this, we moved the flush outside for mutex_t locking
> (mutex_lock() and others like it) and use rt_mutex_schedule() instead of
> schedule().
> We do rt_mutex_pre_schedule() to flush the possible plug to avoid a
> deadlock in case we didn't flush it and someone waits for it. But need
> to do it before we add a waiter because we can be only have on one
> pi_waiter.
> spinlock_t doesn't flush it so we use schedule_rtlock() there instead
> because as per definition this kind of lock can't have any dependency so
> we don't flush the plug.
> 
> Since the futex's rt_mutex usage matches more the mutex we ended up
> with rt_mutex_pre_schedule() + rt_mutex_schedule() around the wait
> schedule.
> 
> blk_flush_plug() is not preserved across syscalls boundaries, there is
> usually blk_start_plug() followed by blk_finish_plug(). So the futex
> code shouldn't have the need to flush it at all because there shouldn't
> be any.
> 
> I suppose for futex's usage it should be enough to simple have a dummy
> to skip the assert and not flush the plug at all.
> 
> Otherwise we would have to flush the possible plug before
> futex_wait_setup() (since starting here the requeue code could act) and
> need to clear task_struct::sched_rt_mutex before futex_do_wait() again
> because it's schedule() usage will trigger another assert. And then we
> need something for rt_mutex_wait_proxy_lock().
> 
> So maybe, we have something to satisfy the assert without plug flush for
> futex/pi/requeue.
> 
>> The changelogs doesn't at all explain why this is correct. Please help?
> 
> I updated it for other reasons. Probably not what you have expected.
> Maybe the above is better reasoning but then we might want a different
> solution…
> 
> Sebastian

How about the patch below?

diff --git a/include/linux/sched/rt.h b/include/linux/sched/rt.h
index 4e3338103654..b7c1dbc193de 100644
--- a/include/linux/sched/rt.h
+++ b/include/linux/sched/rt.h
@@ -51,6 +51,8 @@ static inline bool rt_or_dl_task_policy(struct task_struct *tsk)
  }
  
  #ifdef CONFIG_RT_MUTEXES
+void rt_mutex_futex_pre_schedule(void);
+void rt_mutex_futex_post_schedule(void);
  extern void rt_mutex_pre_schedule(void);
  extern void rt_mutex_schedule(void);
  extern void rt_mutex_post_schedule(void);
diff --git a/kernel/futex/pi.c b/kernel/futex/pi.c
index 795011ea1202..169e6844cecb 100644
--- a/kernel/futex/pi.c
+++ b/kernel/futex/pi.c
@@ -1,7 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0-or-later
  
  #include <linux/slab.h>
-#include <linux/sched/rt.h>
  #include <linux/sched/task.h>
  
  #include "futex.h"
@@ -1002,20 +1001,11 @@ int futex_lock_pi(u32 __user *uaddr, unsigned int flags, ktime_t *time, int tryl
  		}
  
  		/*
-		 * Caution; releasing @hb in-scope. The hb->lock is still locked
-		 * while the reference is dropped. The reference can not be dropped
-		 * after the unlock because if a user initiated resize is in progress
-		 * then we might need to wake him. This can not be done after the
-		 * rt_mutex_pre_schedule() invocation. The hb will remain valid because
-		 * the thread, performing resize, will block on hb->lock during
-		 * the requeue.
+		 * Drop the hash reference before releasing hb->lock below. If this
+		 * wakes a concurrent resize, holding hb->lock keeps the bucket valid
+		 * until the lock handoff is complete.
  		 */
  		futex_private_hash_put(no_free_ptr(hbr.fph));
-		/*
-		 * Must be done before we enqueue the waiter, here is unfortunately
-		 * under the hb lock, but that *should* work because it does nothing.
-		 */
-		rt_mutex_pre_schedule();
  
  		rt_mutex_init_waiter(&rt_waiter);
  
@@ -1081,10 +1071,6 @@ int futex_lock_pi(u32 __user *uaddr, unsigned int flags, ktime_t *time, int tryl
  		 * the
  		 */
  		futex_q_lockptr_lock(&q);
-		/*
-		 * Waiter is unqueued.
-		 */
-		rt_mutex_post_schedule();
  no_block:
  		/*
  		 * Fixup the pi_state owner and possibly acquire the lock if we
diff --git a/kernel/locking/rtmutex_api.c b/kernel/locking/rtmutex_api.c
index 5d48d64725b1..eb18b094473c 100644
--- a/kernel/locking/rtmutex_api.c
+++ b/kernel/locking/rtmutex_api.c
@@ -423,6 +423,7 @@ int __sched rt_mutex_wait_proxy_lock(struct rt_mutex_base *lock,
  {
  	int ret;
  
+	rt_mutex_futex_pre_schedule();
  	raw_spin_lock_irq(&lock->wait_lock);
  	/* sleep on the mutex */
  	set_current_state(TASK_INTERRUPTIBLE);
@@ -433,6 +434,7 @@ int __sched rt_mutex_wait_proxy_lock(struct rt_mutex_base *lock,
  	 */
  	fixup_rt_mutex_waiters(lock, true);
  	raw_spin_unlock_irq(&lock->wait_lock);
+	rt_mutex_futex_post_schedule();
  
  	return ret;
  }
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 96226707c2f6..eb6d7db5ad23 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7595,6 +7595,23 @@ const struct sched_class *__setscheduler_class(int policy, int prio)
   */
  #define fetch_and_set(x, v) ({ int _x = (x); (x) = (v); _x; })
  
+/*
+ * PI futex waits run only in syscall context, so current cannot be a worker
+ * or carry plugged I/O across the userspace boundary. Only establish the
+ * state required by rt_mutex_schedule().
+ */
+void rt_mutex_futex_pre_schedule(void)
+{
+	lockdep_assert(!(current->flags & (PF_WQ_WORKER | PF_IO_WORKER)));
+	lockdep_assert(!current->plug);
+	lockdep_assert(!fetch_and_set(current->sched_rt_mutex, 1));
+}
+
+void rt_mutex_futex_post_schedule(void)
+{
+	lockdep_assert(fetch_and_set(current->sched_rt_mutex, 0));
+}
+
  void rt_mutex_pre_schedule(void)
  {
  	lockdep_assert(!fetch_and_set(current->sched_rt_mutex, 1));

Thanks,
Yao