mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Yao Kai <yaokai34@huawei.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: <linux-kernel@vger.kernel.org>, <tglx@kernel.org>,
	<mingo@redhat.com>, <peterz@infradead.org>,
	<dvhart@infradead.org>, <dave@stgolabs.net>,
	<andrealmeid@igalia.com>, <liuyongqiang13@huawei.com>
Subject: Re: [PATCH 1/2] futex/requeue: Fix rtmutex schedule preparation for requeue PI
Date: Mon, 20 Jul 2026 10:40:03 +0800	[thread overview]
Message-ID: <89dce8bf-8aec-4b3a-a588-f3dab35e8977@huawei.com> (raw)
In-Reply-To: <20260717085548.slwReTBC@linutronix.de>



On 7/17/2026 4:55 PM, Sebastian Andrzej Siewior wrote:
> On 2026-07-17 16:49:21 [+0800], Yao Kai wrote:
>> A waiter requeued onto a PI futex can reach
>> rt_mutex_wait_proxy_lock() without rtmutex schedule preparation:
> 
>>    WARNING: CPU: 0 PID: 293 at kernel/sched/core.c:7606
>>    RIP: rt_mutex_schedule+0x43/0x50
>>    Call Trace:
>>     rt_mutex_slowlock_block.constprop.0+0x5b/0x320
>>     rt_mutex_wait_proxy_lock+0x3e/0x80
>>     futex_wait_requeue_pi+0x3ba/0x590
>>     do_futex+0x171/0x1f0
>>
>> rt_mutex_schedule() requires current->sched_rt_mutex to be set. Normally,
>> rt_mutex_pre_schedule() sets it and submits pending work before the waiter
>> is enqueued. With requeue PI, another task can enqueue the waiter after its
>> futex_q becomes visible:
>>
>>          waiter                          requeue task
>>          ------                          ------------
>> futex_wait_requeue_pi()
>>    futex_wait_setup()
>>      futex_queue(&q)
>>                                          futex_requeue()
>>                                            rt_mutex_start_proxy_lock()
>>                                              enqueue rt_waiter
>>                                              install pi_blocked_on
>>                                            requeue_futex()
>>                                              plist_del(&q->list)
>>    futex_do_wait()
>>      plist_node_empty(&q->list)
>>      skip schedule()
>>                                              plist_add(&q->list)
>>                                            futex_requeue_pi_complete()
>>                                              IN_PROGRESS -> DONE
>>    futex_requeue_pi_wakeup_sync() // DONE
>>    rt_mutex_wait_proxy_lock()
>>      rt_mutex_schedule()
>>
>> futex_do_wait() mistakes the temporary removal for a wakeup and skips
>> schedule(). The waiter consequently enters rt_mutex_schedule() with
>> current->sched_rt_mutex clear. Its block plug remains unflushed, and
>> workqueue or io-wq users are not notified that the worker is going to
>> sleep.
> 
> This has nothing to do with, does it? It is just usually the lock is
> acquired and it is not blocked on. Do you have testcase for this or is
> this just audit?
> 

I have a test but it only triggers the warning. The concern about calling
rt_mutex_pre_schedule() after the proxy waiter has been enqueued came from
audit. A generic PREEMPT_RT path could be:

   rt_mutex_pre_schedule()
     sched_submit_work()
       blk_flush_plug()
         __blk_flush_plug()
           flush_plug_callbacks()
             drbd_unplug()
               spin_lock_irq()
                 rtlock_slowlock()
                   task_blocks_on_rt_mutex()
                     current->pi_blocked_on = waiter

However, I could not find a path for FUTEX_WAIT_REQUEUE_PI to enter with a
live current->plug or worker flags, so this recursion is not reachable from
this syscall.

>> rt_mutex_pre_schedule() cannot be used directly. Calling it before q is
>> published would set current->sched_rt_mutex while futex_do_wait() can still
>> call regular schedule(). Calling it after DONE would flush the block plug
>> after current->pi_blocked_on is installed, which can recurse into rtmutex
>> waiter setup.
>>
>> Split the preparation instead. Flush the plug before futex_wait_setup()
>> publishes q. After futex_requeue_pi_wakeup_sync() returns DONE, enter the
>> rtmutex scheduling state and notify workqueue and io-wq users without
>> flushing the plug again. Split sched_submit_work() to support this ordering
>> and leave the scheduling state after the waiter has acquired the lock or
>> has been removed.
>>
>> Fixes: d14f9e930b90 ("locking/rtmutex: Use rt_mutex specific scheduler helpers")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Yao Kai <yaokai34@huawei.com>
> 
> what about the following? This might compile but lacks all kind of testing.
> 
> diff --git a/kernel/futex/requeue.c b/kernel/futex/requeue.c
> index 79823ad136830..42a04e6e774c4 100644
> --- a/kernel/futex/requeue.c
> +++ b/kernel/futex/requeue.c
> @@ -865,7 +865,9 @@ 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;
> +		rt_mutex_pre_schedule();
>   		ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
> +		rt_mutex_post_schedule();
>   
>   		/*
>   		 * See futex_unlock_pi()'s cleanup: comment.

This also fixes the warning in my test. I would keep rt_mutex_post_schedule()
after the proxy waiter cleanup, as futex_lock_pi() does:

diff --git a/kernel/futex/requeue.c b/kernel/futex/requeue.c
index 79823ad13683..41ffc795d12c 100644
--- a/kernel/futex/requeue.c
+++ b/kernel/futex/requeue.c
@@ -865,6 +865,7 @@ 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;
+               rt_mutex_pre_schedule();
                 ret = rt_mutex_wait_proxy_lock(pi_mutex, to, &rt_waiter);
  
                 /*
@@ -875,6 +876,7 @@ int futex_wait_requeue_pi(u32 __user *uaddr, unsigned int flags,
  
                 futex_q_lockptr_lock(&q);
                 debug_rt_mutex_free_waiter(&rt_waiter);
+               rt_mutex_post_schedule();
                 /*
                  * Fixup the pi_state owner and possibly acquire the lock if we
                  * haven't already.

Thanks,
Yao

  reply	other threads:[~2026-07-20  2:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-17  8:49 [PATCH 0/2] futex/requeue: Fix requeue PI races Yao Kai
2026-07-17  8:49 ` [PATCH 1/2] futex/requeue: Fix rtmutex schedule preparation for requeue PI Yao Kai
2026-07-17  8:55   ` Sebastian Andrzej Siewior
2026-07-20  2:40     ` Yao Kai [this message]
2026-07-20 14:58       ` Sebastian Andrzej Siewior
2026-07-21  1:51         ` Yao Kai
2026-07-21  7:23           ` Sebastian Andrzej Siewior
2026-07-21  9:02             ` Yao Kai
2026-07-21  9:45               ` Sebastian Andrzej Siewior
2026-07-17  8:49 ` [PATCH 2/2] futex/requeue: Prevent rcuwait use-after-free during " Yao Kai
2026-07-17  9:38   ` Sebastian Andrzej Siewior
2026-07-20  2:50     ` Yao Kai
2026-07-20 14:55       ` Sebastian Andrzej Siewior
2026-07-21  2:19         ` Yao Kai
2026-07-21  7:19           ` Sebastian Andrzej Siewior

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=89dce8bf-8aec-4b3a-a588-f3dab35e8977@huawei.com \
    --to=yaokai34@huawei.com \
    --cc=andrealmeid@igalia.com \
    --cc=bigeasy@linutronix.de \
    --cc=dave@stgolabs.net \
    --cc=dvhart@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuyongqiang13@huawei.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@kernel.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

all inboxes | Powered by JetHome®