From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 7C9EE110C for ; Tue, 16 Aug 2022 13:23:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 22AEEC433C1; Tue, 16 Aug 2022 13:23:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1660656190; bh=JumyuZeI/tr0PhD+GYJH6A/O4IFXRoWVZzgl4XKHznE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Efap14+pMVDok7IK+6QRPjif/aQmWMGBSjZrJPWz+qZTnKjt8fW1Uc7L2WkYF4kjQ G6oi1jK2jk9sr7bRVb8z/1ifDCA45tbY2evJ2Cv8Qy/CrKCwj+EdM8LkRHnpaFig8W /PVLfsIPt7ZR8HIO7bggfapXP8NUhtcz+GyAN6bXg4B6/1QSx+S953tIR9Wej0oX+M Bka5mKS63I0ZXllJpWzq4/bz5ORlrZFTFcrUBDL61B1lZKiUUWkcxzp+GFaXoghOkW 9LYpbnYWT+GOLKoWgF2yG8bp0zpGngSI1CwUMYjzy6AH24tTdVVCnwx5xoD1rx6hKh b6AH3B1Y5XJSQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oNwXD-003Umf-M3; Tue, 16 Aug 2022 14:23:07 +0100 Date: Tue, 16 Aug 2022 14:23:02 +0100 Message-ID: <87v8qswceh.wl-maz@kernel.org> From: Marc Zyngier To: Jon Nettleton Cc: Will Deacon , Arnd Bergmann , Hector Martin , Peter Zijlstra , Ingo Molnar , Alan Stern , Andrea Parri , Boqun Feng , Nicholas Piggin , David Howells , Jade Alglave , Luc Maranget , "Paul E. McKenney" , Akira Yokosawa , Daniel Lustig , Joel Fernandes , Mark Rutland , Jonathan Corbet , Tejun Heo , jirislaby@kernel.org, Catalin Marinas , Oliver Neukum , Linus Torvalds , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Asahi Linux , stable@vger.kernel.org Subject: Re: [PATCH] locking/atomic: Make test_and_*_bit() ordered on failure In-Reply-To: References: <20220816070311.89186-1-marcan@marcan.st> <20220816130048.GA11202@willie-the-truck> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: asahi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: jon@solid-run.com, will@kernel.org, arnd@arndb.de, marcan@marcan.st, peterz@infradead.org, mingo@kernel.org, stern@rowland.harvard.edu, parri.andrea@gmail.com, boqun.feng@gmail.com, npiggin@gmail.com, dhowells@redhat.com, j.alglave@ucl.ac.uk, luc.maranget@inria.fr, paulmck@kernel.org, akiyks@gmail.com, dlustig@nvidia.com, joel@joelfernandes.org, mark.rutland@arm.com, corbet@lwn.net, tj@kernel.org, jirislaby@kernel.org, catalin.marinas@arm.com, oneukum@suse.com, torvalds@linux-foundation.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-doc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, asahi@lists.linux.dev, stable@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Tue, 16 Aug 2022 14:05:54 +0100, Jon Nettleton wrote: > > On Tue, Aug 16, 2022 at 3:01 PM Will Deacon wrote: > > > > On Tue, Aug 16, 2022 at 02:29:49PM +0200, Jon Nettleton wrote: > > > On Tue, Aug 16, 2022 at 10:17 AM Arnd Bergmann wrote: > > > > > > > > On Tue, Aug 16, 2022 at 9:03 AM Hector Martin wrote: > > > > > > > > > > These operations are documented as always ordered in > > > > > include/asm-generic/bitops/instrumented-atomic.h, and producer-consumer > > > > > type use cases where one side needs to ensure a flag is left pending > > > > > after some shared data was updated rely on this ordering, even in the > > > > > failure case. > > > > > > > > > > This is the case with the workqueue code, which currently suffers from a > > > > > reproducible ordering violation on Apple M1 platforms (which are > > > > > notoriously out-of-order) that ends up causing the TTY layer to fail to > > > > > deliver data to userspace properly under the right conditions. This > > > > > change fixes that bug. > > > > > > > > > > Change the documentation to restrict the "no order on failure" story to > > > > > the _lock() variant (for which it makes sense), and remove the > > > > > early-exit from the generic implementation, which is what causes the > > > > > missing barrier semantics in that case. Without this, the remaining > > > > > atomic op is fully ordered (including on ARM64 LSE, as of recent > > > > > versions of the architecture spec). > > > > > > > > > > Suggested-by: Linus Torvalds > > > > > Cc: stable@vger.kernel.org > > > > > Fixes: e986a0d6cb36 ("locking/atomics, asm-generic/bitops/atomic.h: Rewrite using atomic_*() APIs") > > > > > Fixes: 61e02392d3c7 ("locking/atomic/bitops: Document and clarify ordering semantics for failed test_and_{}_bit()") > > > > > Signed-off-by: Hector Martin > > > > > --- > > > > > Documentation/atomic_bitops.txt | 2 +- > > > > > include/asm-generic/bitops/atomic.h | 6 ------ > > > > > > > > I double-checked all the architecture specific implementations to ensure > > > > that the asm-generic one is the only one that needs the fix. > > > > > > > > I assume this gets merged through the locking tree or that Linus picks it up > > > > directly, not through my asm-generic tree. > > > > > > > > Reviewed-by: Arnd Bergmann > > > > > > > > _______________________________________________ > > > > linux-arm-kernel mailing list > > > > linux-arm-kernel@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > > > Testing this patch on pre Armv8.1 specifically Cortex-A72 and > > > Cortex-A53 cores I am seeing > > > a huge performance drop with this patch applied. Perf is showing > > > lock_is_held_type() as the worst offender > > > > Hmm, that should only exist if LOCKDEP is enabled and performance tends to > > go out of the window if you have that on. Can you reproduce the same > > regression with lockdep disabled? > > > > Will > > Yep I am working on it. We should note that > > config LOCKDEP_SUPPORT > def_bool y > > is the default for arm64 Yes, as the architecture supports LOCKDEP. However, you probably have something like CONFIG_PROVE_LOCKING to see such a performance hit (and that's definitely not on by default). M. -- Without deviation from the norm, progress is not possible.