From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E5877CE79A8 for ; Tue, 19 Sep 2023 19:00:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+K+K8epP8L03wBUhruqq8yJkA1YqyAi1UeXW+lN0wQs=; b=YYBSl4ZVTCRoa4 QqE/oBvoGOEbbVUF8y+mD+evfRbTau+IgelaOB6dkMdjB52reQ42CnrFmoLpZHUqytLFfWDRXFlPj AWU8k+SsP5saUVeoJ0XRnQ1rvj8gkfLhaUbJp8IguZuRt+VYikbL1D9yaZLME4CCL3WkvxuB7aNl3 JEAxzmbP3xSPWO2GiPAeJmgvs40e5hIcoeO9zq8Wa94i/vFQLlgtBYRrT3CJsn80ViXTsvQrnFpV8 KDClolWTLvs01TyFsOIv1FI4eLHDFCUp5Qy1SuLFVB+BPYfEVu01MbX25OowQXC5lah8gpux27QyS Xv6cCOoiQMtf8Q+sVZ/A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qifwx-0017IL-11; Tue, 19 Sep 2023 18:59:55 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qifww-0017Hp-1N for linux-arm-kernel@bombadil.infradead.org; Tue, 19 Sep 2023 18:59:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=e7f/dRgWja2bdNjRsZeOzDTy9nx/vdEU9AAAUFtkU2A=; b=O/v1f3ytNO6mjypAPlNroXtgRQ Abk3CfKjOgS3pzru76WLeOC2PPDPMwVfa3/+ahYQzLzM9QEpAHzO2a2zK3xLwaP8dVZ9u2HyvCPqi +ABRr2x4VfVPzlHwCuH3M0w1SSalWVeYdvsJILMh6zxPxP9OQi8yL3dGXo5BVU69xH5SEY3S6z79q nje+kfX4jeLGcI8Nhlywj2UbLa6DMMfZP42wWDeSmUnt9rGrkqjfr1IfEvPVaGWAKxdpskl2ncXwv UVyNk8vl3IiLizjdqNSgYbKWdNjyjRuJdQQdysR/Lv1n80cyA1k21epZ8ukPKZohuu8QHgqWawSBj 0xBuiieg==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.96 #2 (Red Hat Linux)) id 1qifwp-00Dp67-1J; Tue, 19 Sep 2023 18:59:49 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 81B1E30058C; Tue, 19 Sep 2023 20:59:48 +0200 (CEST) Date: Tue, 19 Sep 2023 20:59:48 +0200 From: Peter Zijlstra To: Mark Rutland Cc: linux-kernel@vger.kernel.org, boqun.feng@gmail.com, david@fromorbit.com, djwong@kernel.org, kent.overstreet@linux.dev, libaokun1@huawei.com, linux-arm-kernel@lists.infradead.org, ming.lei@redhat.com, will@kernel.org, yi.zhang@redhat.com Subject: Re: [PATCH] locking/atomic: scripts: fix fallback ifdeffery Message-ID: <20230919185948.GA424@noisy.programming.kicks-ass.net> References: <20230919171430.2697727-1-mark.rutland@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230919171430.2697727-1-mark.rutland@arm.com> X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 19, 2023 at 06:14:29PM +0100, Mark Rutland wrote: > Since commit: > > 9257959a6e5b4fca ("locking/atomic: scripts: restructure fallback ifdeffery") > > The ordering fallbacks for atomic*_read_acquire() and > atomic*_set_release() erroneously fall back to the implictly relaxed > atomic*_read() and atomic*_set() variants respectively, without any > additional barriers. This loses the ACQUIRE and RELEASE ordering > semantics, which can result in a wide variety of problems, even on > strongly-ordered architectures where the implementation of > atomic*_read() and/or atomic*_set() allows the compiler to reorder those > relative to other accesses. > > In practice this has been observed to break bit spinlocks on arm64, > resulting in dentry cache corruption. > > The fallback logic was intended to allow ACQUIRE/RELEASE/RELAXED ops to > be defined in terms of FULL ops, but where an op had RELAXED ordering by > default, this unintentionally permitted the ACQUIRE/RELEASE ops to be > defined in terms of the implicitly RELAXED default. > > This patch corrects the logic to avoid falling back to implicitly > RELAXED ops, resulting in the same behaviour as prior to commit > 9257959a6e5b4fca. > > I've verified the resulting assembly on arm64 by generating outlined > wrappers of the atomics. Prior to this patch the compiler generates > sequences using relaxed load (LDR) and store (STR) instructions, e.g. > > | : > | ldr x0, [x0] > | ret > | > | : > | str x1, [x0] > | ret > > With this patch applied the compiler generates sequences using the > intended load-acquire (LDAR) and store-release (STLR) instructions, e.g. > > | : > | ldar x0, [x0] > | ret > | > | : > | stlr x1, [x0] > | ret > > To make sure that there were no other victims of the ifdeffery rewrite, > I generated outlined copies of all of the {atomic,atomic64,atomic_long} > atomic operations before and after commit 9257959a6e5b4fca. A diff of > the generated assembly on arm64 shows that only the read_acquire() and > set_release() operations were changed, and only lost their intended > ordering: > > | [mark@lakrids:~/src/linux]% diff -u \ > | <(aarch64-linux-gnu-objdump -d before-9257959a6e5b4fca.o) > | <(aarch64-linux-gnu-objdump -d after-9257959a6e5b4fca.o) > | --- /proc/self/fd/11 2023-09-19 16:51:51.114779415 +0100 > | +++ /proc/self/fd/16 2023-09-19 16:51:51.114779415 +0100 > | @@ -1,5 +1,5 @@ > | > | -before-9257959a6e5b4fca.o: file format elf64-littleaarch64 > | +after-9257959a6e5b4fca.o: file format elf64-littleaarch64 > | > | > | Disassembly of section .text: > | @@ -9,7 +9,7 @@ > | 4: d65f03c0 ret > | > | 0000000000000008 : > | - 8: 88dffc00 ldar w0, [x0] > | + 8: b9400000 ldr w0, [x0] > | c: d65f03c0 ret > | > | 0000000000000010 : > | @@ -17,7 +17,7 @@ > | 14: d65f03c0 ret > | > | 0000000000000018 : > | - 18: 889ffc01 stlr w1, [x0] > | + 18: b9000001 str w1, [x0] > | 1c: d65f03c0 ret > | > | 0000000000000020 : > | @@ -1230,7 +1230,7 @@ > | 1070: d65f03c0 ret > | > | 0000000000001074 : > | - 1074: c8dffc00 ldar x0, [x0] > | + 1074: f9400000 ldr x0, [x0] > | 1078: d65f03c0 ret > | > | 000000000000107c : > | @@ -1238,7 +1238,7 @@ > | 1080: d65f03c0 ret > | > | 0000000000001084 : > | - 1084: c89ffc01 stlr x1, [x0] > | + 1084: f9000001 str x1, [x0] > | 1088: d65f03c0 ret > | > | 000000000000108c : > | @@ -2427,7 +2427,7 @@ > | 207c: d65f03c0 ret > | > | 0000000000002080 : > | - 2080: c8dffc00 ldar x0, [x0] > | + 2080: f9400000 ldr x0, [x0] > | 2084: d65f03c0 ret > | > | 0000000000002088 : > | @@ -2435,7 +2435,7 @@ > | 208c: d65f03c0 ret > | > | 0000000000002090 : > | - 2090: c89ffc01 stlr x1, [x0] > | + 2090: f9000001 str x1, [x0] > | 2094: d65f03c0 ret > | > | 0000000000002098 : > > I've build tested this with a variety of configs for alpha, arm, arm64, > csky, i386, m68k, microblaze, mips, nios2, openrisc, powerpc, riscv, > s390, sh, sparc, x86_64, and xtensa, for which I've seen no issues. I > was unable to build test for ia64 and parisc due to existing build > breakage in v6.6-rc2. > > Fixes: 9257959a6e5b4fca ("locking/atomic: scripts: restructure fallback ifdeffery") > Reported-by: Ming Lei > Link: https://lore.kernel.org/all/ZOWFtqA2om0w5Vmz@fedora/ > Reported-by: Darrick J. Wong > Link: https://lore.kernel.org/linux-fsdevel/20230912173026.GA3389127@frogsfrogsfrogs/ > Signed-off-by: Mark Rutland > Cc: Baokun Li > Cc: Boqun Feng > Cc: Darrick J. Wong > Cc: Dave Chinner > Cc: Kent Overstreet > Cc: Ming Lei > Cc: Peter Zijlstra > Cc: Will Deacon > Cc: Yi Zhang > Cc: linux-arm-kernel@lists.infradead.org > --- > include/linux/atomic/atomic-arch-fallback.h | 10 +--------- > scripts/atomic/gen-atomic-fallback.sh | 2 +- > 2 files changed, 2 insertions(+), 10 deletions(-) > > Peter, are you happy to queue this in the tip tree? It's a pretty nasty > regresssion in v6.5, and I'd like to get this in as a fix for v6.6 ASAP. Yes, I'll go queue it in tip/locking/urgent. Thanks! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel