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 X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65B34C3A5A1 for ; Wed, 28 Aug 2019 11:54:01 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 3907B2173E for ; Wed, 28 Aug 2019 11:54:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="d/3ksQVb" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3907B2173E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject: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=qPTQxZMNNqoVcW54lRm1yzoM5PU5WdAUpfZGV6dW6Yk=; b=d/3ksQVb6mkrCB QSJG48GWkf1HzW1m4SsE4LdE8XUkilOsxLxWoHtjXpLhJh20Z5ivVXGnkjVerpGjJ5AUOb8iOgfl9 3ZoakE9XASDsIPLFKiurMiO8ckicLgX8suqX437WPTjTT7ZOs7tl5OG5rJJnjKVdyK/HV4IFpha6w ZkLFJ2AHagHsNcXcdSY51BLvffoBiXDj07QI9s+RXoiqBxYhSH5O4wghIctM8smjXuH12hofWdYM2 Yv9AJebRsC7PvS2qlCTjtQmCF01GzZgylZJ7U9zgSAe0zbQU5gwmPLsTIIsYsBte5z3m3KoyRXU5A P/dYc07o3+x8V9iTOhZw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1i2wWP-0003B2-AS; Wed, 28 Aug 2019 11:53:53 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1i2wWK-0003Ag-Ji for linux-arm-kernel@lists.infradead.org; Wed, 28 Aug 2019 11:53:50 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D003D344; Wed, 28 Aug 2019 04:53:47 -0700 (PDT) Received: from localhost (unknown [10.37.6.20]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 510AE3F246; Wed, 28 Aug 2019 04:53:47 -0700 (PDT) Date: Wed, 28 Aug 2019 12:53:45 +0100 From: Andrew Murray To: Mark Rutland Subject: Re: [PATCH v3 3/5] arm64: atomics: avoid out-of-line ll/sc atomics Message-ID: <20190828115345.GU14582@e119886-lin.cambridge.arm.com> References: <20190812143625.42745-1-andrew.murray@arm.com> <20190812143625.42745-4-andrew.murray@arm.com> <20190822170155.GE33080@lakrids.cambridge.arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190822170155.GE33080@lakrids.cambridge.arm.com> User-Agent: Mutt/1.10.1+81 (426a6c1) (2018-08-26) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190828_045348_831219_0CB87EC1 X-CRM114-Status: GOOD ( 28.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Zijlstra , Catalin Marinas , Boqun Feng , Will Deacon , Ard.Biesheuvel@arm.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Aug 22, 2019 at 06:01:55PM +0100, Mark Rutland wrote: > On Mon, Aug 12, 2019 at 03:36:23PM +0100, Andrew Murray wrote: > > When building for LSE atomics (CONFIG_ARM64_LSE_ATOMICS), if the hardware > > or toolchain doesn't support it the existing code will fallback to ll/sc > > atomics. It achieves this by branching from inline assembly to a function > > that is built with specical compile flags. Further this results in the > > clobbering of registers even when the fallback isn't used increasing > > register pressure. > > > > Let's improve this by providing inline implementations of both LSE and > > ll/sc and use a static key to select between them. This allows for the > > compiler to generate better atomics code. > > > > To improve icache performance for the LL/SC fallback atomics, we put them > > in their own subsection. > > > > Please note that as atomic_arch.h is included indirectly by kernel.h > > (via bitops.h), we cannot depend on features provided later in the kernel.h > > file. This prevents us from placing the system_uses_lse_atomics function > > in cpu_feature.h due to its dependencies. > > > > Signed-off-by: Andrew Murray > > [...] > > > diff --git a/arch/arm64/include/asm/atomic_arch.h b/arch/arm64/include/asm/atomic_arch.h > > new file mode 100644 > > index 000000000000..255a284321c6 > > --- /dev/null > > +++ b/arch/arm64/include/asm/atomic_arch.h > > @@ -0,0 +1,154 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Selection between LSE and LL/SC atomics. > > + * > > + * Copyright (C) 2018 ARM Ltd. > > + * Author: Andrew Murray > > + */ > > + > > +#ifndef __ASM_ATOMIC_ARCH_H > > +#define __ASM_ATOMIC_ARCH_H > > + > > +#include > > +#include > > + > > +#include > > +#include > > I'm guessing that we have to include the headers first > due to the include dependencies. If that's the case, could we please > have a comment here to that effect? > > Minor nit, but could we also order those two alphabetically, please? > > The general style is to have headers alphabetically, with (for reasons > unknown) the headers before the headers. There doesn't appear to be any valid reason for ordering the includes in the way I have done, so I'll change it to the linux-followed-by-asm-alphabetical way. > > [...] > > > +#if IS_ENABLED(CONFIG_ARM64_LSE_ATOMICS) && IS_ENABLED(CONFIG_AS_LSE) > > +#define __LL_SC_FALLBACK(asm_ops) \ > > +" b 3f\n" \ > > +" .subsection 1\n" \ > > +"3:\n" \ > > +asm_ops "\n" \ > > +" b 4f\n" \ > > +" .previous\n" \ > > +"4:\n" > > +#else > > +#define __LL_SC_FALLBACK(asm_ops) asm_ops > > #endif > > Can we instead make the ll/sc functions with the cold attribute (wrapped > by as __cold)? > > IIUC that should have a similar effect, and might allow GCC to do better > (e.g. merging compatible instances of the ll/sc code in the same cold > subsection). > > https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Common-Function-Attributes.html#index-cold-function-attribute Thanks for suggesting this, I hadn't considered this previously. However the cold attribute only seems to have an effect when the function it is applied to isn't inline. I think we want to keep both LSE and fallback functions as inline. At present the fallback functions are inlined, due to the use of 'unlikely' they end up at the end of the function they are called from. Within this inline'd function we take unconditional branches to/from the actual LL/SC implementation, as we are using subsections the LL/SC implementations are grouped together at the end of the each compilation unit. As we are using unconditional branches, each call to an atomics function results in the LL/SC implementation being duplicated (just like any other inline function, except the code is elsewhere). We get some locality benefits from the use of subsection but that is per-compilation unit (so you'll get clusters of them across the vmlinux). This approach gives us bloat, we can mitigate this by not using inline functions, and further by optimising for size. We can put all the fallback atomics in a single section to benefit from vmlinux-wide code locality - however to benefit from all this we must use functions calls and their associated overhead. Thanks, Andrew Murray > > Otherwise, this is looking much nicer! > > Thanks, > Mark. > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel