From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 410C8370D4D; Tue, 25 Aug 2026 23:29:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787700543; cv=none; b=hPmquaQC77ZK80pcPfzwc/TIkeHcwr4Le/ojUOpbURF5W2iR7COkROMKmHZ3cv8VjEFluKQww8FJxQ5JUreu46h6A1Ur66IvtJ4qMf3EYFICIHANs/fZFpxI/hGtTsBXvX6bzBsvGlgOi8HqIJu79FJnc0frA8OtN127PM8X5Ko= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787700543; c=relaxed/simple; bh=bdxkwjpp/fzCA0hl/H1phJb6/BYtjnrFrbzq0Rj+mew=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ttCry5Ky3llHq/M6NmC2hJQK7UCS//Ovo9JWY74wBWUhcL7ssAqH4kbw7X6yeNko7QxccR2nurkwu6l40zYWeXX9mhKNUVY0cvkIluGpz93+JIxLfcvrY4YHM+KmGZkoJcGIfLYzEmx97U+3yIIl7Rqtx3O4pilGmTH8xHwFXLk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=hIbwp9dw; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="hIbwp9dw" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8C4C01F00A3D; Tue, 25 Aug 2026 23:29:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787700541; bh=xTztJ0TnehvK2U7Dmw0af1p/684PxvRMcw0COt99w9A=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=hIbwp9dwuiy/j8HydCULYzN11B3PXEyFpYWAROAuK8QCD2vq2McGAkZW0lmbK0MO5 NueOPZ5PnwrmdDxS7LwuB0YEHkmBXFxjisdw82P4iYKyvOlaV7yF1ltcFS2P833+Xj piOMtSH3cBARWpsND209EG2X7inRRsbSecsHNZqmrCs545B8UMim38T2Egy19yNHlW udkpDVJqz92mqvNF3SuzBdZ9z/oc+1g4CNJm1QKQ9NGc+c5VsDlDqWFk3h7SGVqFAP RGqfMvRQK/PUq1oN83vNEf0JYToUsdnog1YAZjQvn05OqnnnpY4r7r7SNUcM6ffyYa IeUlFogJYAS4w== Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfauth.phl.internal (Postfix) with ESMTP id ABC21F40066; Tue, 25 Aug 2026 19:29:00 -0400 (EDT) Received: from phl-frontend-03 ([10.202.2.162]) by phl-compute-06.internal (MEProxy); Tue, 25 Aug 2026 19:29:00 -0400 X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTFuqHG3ur4mEzGC/K32iyT+6fcg8gvWVnBvoJ9E75NspQvwL0VdO6JeSI4YNlbfmF pVU8zJdIobgWGjDhiG251i6Y5GiZfZe5tGIyz+RvgY8WlEN6HZyEZht5F9wkpR77KSHu6l xjYWb0xKCAUNLqV1U8HrcdWjBKuZqxfyB2qESR3l33mKiNYeW+fgCyTFY4sSCAwiqBwbPc A8g3PBR2muT5XF1sj2FfAyYUFMGykfcs0JUZClYckmPBAVUeDW+o27VAsHLGVX3m0gO8X2 52Nl+r7+GRFghl2A69Okk9RQf1/35E0qHtAzrEaEshiHlcycBdm5Va8FDjmTPfArrUGjUJ 9QiTCiWB4XuEJK2eLboUo80xDc2UvMjIAbiCNmPC1M6zJt339Y+94fa9sG1zMP+npqvPwP WZYyuWR0hnLGK3cOIpe7uEb9NQdCNBEP8z4/BMJV9sdjgr/aIZ43uAxz6ZXs5vK1OgKSzY u3asi7LXk3QP7mUI8sd22hRZf/GVjfogBuv/PYhAil1mqtLXTJR18XWWAOcj13cYJxWKWC qzQNjqEXmoa1j1te9q6Vip/MRETKsrWpCAbKQcAn9OWNO4oQzkLoXr5k9WeS0nKx3ZrEKR GRePTaKGnQ1ft8FbH68PaxwVbvvi2tewb1AROroBwZ9C7EN+8uYOKHV6lpsQ X-ME-Proxy: Feedback-ID: i8dbe485b:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 25 Aug 2026 19:29:00 -0400 (EDT) Date: Tue, 25 Aug 2026 16:28:59 -0700 From: Boqun Feng To: Thomas Gleixner Cc: Peter Zijlstra , linux-kernel@vger.kernel.org, linux-tip-commits@vger.kernel.org, x86@kernel.org Subject: Re: [PATCH] locking: Revert switching guards to _irq_{disable,enable}() Message-ID: References: <20260804161447.84806-8-boqun@kernel.org> <178635226387.442315.3868294476114711805.tip-bot2@tip-bot2> <20260824104704.GA4121339@noisy.programming.kicks-ass.net> <20260824105523.GA4121620@noisy.programming.kicks-ass.net> <877bldhkmq.ffs@fw13> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <877bldhkmq.ffs@fw13> On Wed, Aug 26, 2026 at 12:59:25AM +0200, Thomas Gleixner wrote: > On Mon, Aug 24 2026 at 18:33, Boqun Feng wrote: > > On Mon, Aug 24, 2026 at 12:55:23PM +0200, Peter Zijlstra wrote: > >> > >> While the guards are properly nested, not all wrapped code is nice, as already > >> highlighted by that fair.c hunk. > >> > >> Syzbot found another instance of this pattern in posix_timer_delete(), which > >> does spin_unlock_irq()+spin_lock_irq() inside scoped_guard(spinlock_irq). > >> Combined with this patch, that goes sideways most spectacular. > >> > >> Undo this change, until we've developed stronger tools / debug for such issues. > >> > > > > Mainly hand-waving, but if we make _irq(), irqsave(), _disable() > > __acquires() different contexts, we may be able to catch these issues at > > compile time. I will explore a bit on this. > > No. > > Just do a wholesale conversion of all functions which affect the CPU > interrupt disabled state directly (local_irq_*) and indirectly (locking > functions etc.) > > Anything else is just a whack a mole game. > Alright. But I'm afraid that's just another type of whack-a-mole games. As I mentioned here [1], we are a few unpaired local_irq_disable() + local_irq_enable(), we can spend time to clean them up, but no guarantee people will not introduce more, plus we have code that does spin_lock_irqsave(); spin_unlock_irq(); spin_lock_irq(); spin_unlock_irqrestore(); and expect it works. A more reasonable approach to me is introducing the new API and fixing the problematic usage one-by-one and then when we are certain about only a few cases left, we do a flag day change. Trying to do it (new API and whole conversion) in one go is easier said than done. Of course I might miss something subtle here, looking forwards to your suggestion. > TBH, I do not understand why you thought that you can get away with this > lazy approach especially after you discovered the same nasty problem in > do_sched_cfs_period_timer(). The resolution of that got buried in > > 1b0866874833 ("locking: Switch to _irq_{disable,enable}() variants in cleanup guards") > > without even being mentioned. > I have this in the commit log: [boqun: Adjust the user-side changes in do_sched_cfs_*_timer() provided by Peter and Lyude] but sure, I should have done a better job mentioning it. A bit more context of switching the guard implementation: I wanted to have some test/usage coverage other than Rust for the new API, and since the guard() API is relatively new, so I thought people will not use it "creatively" (but obviously I was wrong). Hence I add the conversation for the guard APIs only. It is not a lazy approach IMO, but rather a way to test how the new API works. Of course, a bug is a bug, I don't have any excuse on that. > When I was discussing the non-sensical syzbot messages earlier today > with Peter it immediately occurred to me that this undocumented change in > do_sched_cfs_period_timer() is not the only pattern which causes this to > go belly up. It took me five seconds to find the posix timer one. > > TBH, my hope really was that the RUST people take the only valid > engineering principle "Correctness first" serious, but sadly they seem > to be the same lazy sods than everyone else who want to push their > agenda through no matter what. > There seems some misunderstandings here. The only "lazy" part is we defer the whole conversion because of the problems I mentioned above, and that is because Correctness is valued. [1]: https://lore.kernel.org/rust-for-linux/aPHlySQJQpDmgHAm@tardis.local/ Regards, Boqun > Thanks, > > tglx