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: Tue, 21 Jul 2026 09:51:47 +0800	[thread overview]
Message-ID: <52c53bc8-d22b-43e5-89c5-d2918940dc4d@huawei.com> (raw)
In-Reply-To: <20260720145847.QW7LB9jE@linutronix.de>



On 7/20/2026 10:58 PM, Sebastian Andrzej Siewior wrote:
> On 2026-07-20 10:40:03 [+0800], Yao Kai wrote:
>> 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.
> 
> So the suggestion is okay then?
> …
> 

Yes.

>>> 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:
> 
> But why?
> 

Just to be consistent with the code in futex_lock_pi():

         ret = rt_mutex_wait_proxy_lock(&q.pi_state->pi_mutex, to, &rt_waiter);
                                                                                     
    cleanup:
         /*
          * If we failed to acquire the lock (deadlock/signal/timeout), we must
          * unwind the above, however we canont lock hb->lock because
          * rt_mutex already has a waiter enqueued and hb->lock can itself try
          * and enqueue an rt_waiter through rtlock.
          *
          * Doing the cleanup without holding hb->lock can cause inconsistent
          * state between hb and pi_state, but only in the direction of not
          * seeing a waiter that is leaving.
          *
          * See futex_unlock_pi(), it deals with this inconsistency.
          *
          * There be dragons here, since we must deal with the inconsistency on
          * the way out (here), it is impossible to detect/warn about the race
          * the other way around (missing an incoming waiter).
          *
          * What could possibly go wrong...
          */
         if (ret && !rt_mutex_cleanup_proxy_lock(&q.pi_state->pi_mutex, &rt_waiter))
           ret = 0;
                                                                                     
         /*
          * Now that the rt_waiter has been dequeued, it is safe to use
          * spinlock/rtlock (which might enqueue its own rt_waiter) and fix up
          * the
          */
         futex_q_lockptr_lock(&q);
         /*
          * Waiter is unqueued.
          */
         rt_mutex_post_schedule();


>> 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();
> 
> But there is futex_q_lockptr_lock() from what I see in the context. This
> one should trigger the warning if it is done as you suggest.
> 

I don't think it would trigger the warning. At this point rt_mutex_wait_proxy_lock()
or rt_mutex_cleanup_proxy_lock() has cleared current->pi_blocked_on.

>>                  /*
>>                   * Fixup the pi_state owner and possibly acquire the lock if we
>>                   * haven't already.
>>
>> Thanks,
>> Yao
> 
> Sebastian


  reply	other threads:[~2026-07-21  1:51 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
2026-07-20 14:58       ` Sebastian Andrzej Siewior
2026-07-21  1:51         ` Yao Kai [this message]
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=52c53bc8-d22b-43e5-89c5-d2918940dc4d@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®