The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Yao Kai <yaokai34@huawei.com>,
	linux-kernel@vger.kernel.org, tglx@kernel.org, mingo@redhat.com,
	dvhart@infradead.org, dave@stgolabs.net, andrealmeid@igalia.com,
	liuyongqiang13@huawei.com, Crystal Wood <swood@redhat.com>
Subject: Re: [PATCH v2 1/2] futex/requeue: Fix rtmutex schedule preparation for requeue PI
Date: Mon, 24 Aug 2026 16:08:09 +0200	[thread overview]
Message-ID: <20260824140809.eJxsIRHn@linutronix.de> (raw)
In-Reply-To: <20260804122102.GI776954@noisy.programming.kicks-ass.net>

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

  parent reply	other threads:[~2026-08-24 14:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22  8:51 [PATCH v2 0/2] futex/requeue: Fix requeue PI races Yao Kai
2026-07-22  8:51 ` [PATCH v2 1/2] futex/requeue: Fix rtmutex schedule preparation for requeue PI Yao Kai
2026-08-04 12:21   ` Peter Zijlstra
2026-08-05  8:00     ` Yao Kai
2026-08-24  3:07       ` Yao Kai
2026-08-24 14:08     ` Sebastian Andrzej Siewior [this message]
2026-08-25  3:13       ` Yao Kai
2026-07-22  8:51 ` [PATCH v2 2/2] futex/requeue: Prevent rcuwait use-after-free during " Yao Kai

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=20260824140809.eJxsIRHn@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=andrealmeid@igalia.com \
    --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=swood@redhat.com \
    --cc=tglx@kernel.org \
    --cc=yaokai34@huawei.com \
    /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