From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from canpmsgout02.his.huawei.com (canpmsgout02.his.huawei.com [113.46.200.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CE6E52BEFE8 for ; Tue, 25 Aug 2026 03:14:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=113.46.200.217 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787627653; cv=none; b=oDQTc/3J5HI1Av87UUORbhF5HVlz0f2AE3fTcopdb1eQZ4MQkplqilJ/qfmMMVjO44VHJ+ZJLmzJ+L4qNieafZl5K3UnQAMzMJEzo/Pg++4PHgxSfaqxEYEr4LgbjIkMzf2V/y/ekcBZxgyDPp7C68GVwXOZpy4/hDPvz5DFK4w= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787627653; c=relaxed/simple; bh=cQifwvM/KDsYw6b7CU1DAqDukx2TEpTQOPDn9r/CJLg=; h=Message-ID:Date:MIME-Version:Subject:To:CC:References:From: In-Reply-To:Content-Type; b=qxitV/UJji/tAYBZ2OgXN2MqlyurRjmVGDaaYrRkt7gTzFabLNHIKnM1JajKUJxLI06n7z5yWlzrL/pkIoa/wbEsUfju3b8kAyK/OfaZh0XtWzNDlwlKXQcdyBlt7WKIyDzJdg2vn053iNkZSpVXeZJDr31gO+Vbhcf91wPIc+M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com; spf=pass smtp.mailfrom=huawei.com; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b=Jll+7smK; arc=none smtp.client-ip=113.46.200.217 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=huawei.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=huawei.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=huawei.com header.i=@huawei.com header.b="Jll+7smK" dkim-signature: v=1; a=rsa-sha256; d=huawei.com; s=dkim; c=relaxed/relaxed; q=dns/txt; h=From; bh=ZXW3zjM5Dslg9MJw7f5t4pgo4JUVDNh7MzluqdWEeN8=; b=Jll+7smKNe3mnj+9ZEdtkm5wZsIXdx5mNpdkbw9QDNxYBYawbX+AFn+UeWsgY8CB+v3XgCip7 ROwzuN6tX5k3a3vctfNny/leLYdMx6nJyToyfYJ3sR25607BJXMgauZhpGYW873EK1oaW/dyj+/ i6WF/wUkdHFcpmXntFReoOA= Received: from mail.maildlp.com (unknown [172.19.163.0]) by canpmsgout02.his.huawei.com (SkyGuard) with ESMTPS id 4hTXfT1Pxpzcb0W; Tue, 25 Aug 2026 11:03:29 +0800 (CST) Received: from dggpemr500006.china.huawei.com (unknown [7.185.36.185]) by mail.maildlp.com (Postfix) with ESMTPS id B4D6540537; Tue, 25 Aug 2026 11:13:59 +0800 (CST) Received: from [100.103.109.15] (100.103.109.15) by dggpemr500006.china.huawei.com (7.185.36.185) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.2562.45; Tue, 25 Aug 2026 11:13:58 +0800 Message-ID: <5df8a43e-11bb-4881-877d-40034c414984@huawei.com> Date: Tue, 25 Aug 2026 11:13:58 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/2] futex/requeue: Fix rtmutex schedule preparation for requeue PI To: Sebastian Andrzej Siewior , Peter Zijlstra CC: , , , , , , , Crystal Wood References: <20260722085140.1949077-1-yaokai34@huawei.com> <20260722085140.1949077-2-yaokai34@huawei.com> <20260804122102.GI776954@noisy.programming.kicks-ass.net> <20260824140809.eJxsIRHn@linutronix.de> Content-Language: en-US From: Yao Kai In-Reply-To: <20260824140809.eJxsIRHn@linutronix.de> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: kwepems500001.china.huawei.com (7.221.188.70) To dggpemr500006.china.huawei.com (7.185.36.185) 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 -#include #include #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