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=-5.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 5DCB4C433DF for ; Tue, 7 Jul 2020 10:12:03 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 0F010206DF for ; Tue, 7 Jul 2020 10:12:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="zipMeC3/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0F010206DF 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+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=merlin.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=0JX+8ZPxbzjWPd07ZUQfEqrDlnxwZc7wgh/iJS+9jbw=; b=zipMeC3/H40zsZqRRKyhPivMN XXfyTkTuVD6JjKuzVmPHWHnQzi7VcdcTcSSvxx3VarDTDTR7pZCWX1sMx59mOIbqyHkaYTdW/G73a /SU7SvJLj137A6/JZoiCUTqLz2OU8npA5+2zhRXj2V5UQbX11g3nmsG7/wRpgyqcbUSaOg0NygJCH 2DJ0VzAqvpdx8pRZ5V5YJTd1z+YbvuyCx6cPRQ4iNZVRWRmN2OwTWbee7eVexWmvV6aqMaV8QNqxY 5FQA2eb6giQIIOqXxWq0MPuC2PSXXVYISlJkA/iUVXQpHIIdDkBg6M2ZFFA2/3k1CttXpDmoIL7hm yu3Ey4X/w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jskYU-0003V9-HH; Tue, 07 Jul 2020 10:10:26 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jskYS-0003Uc-2I for linux-arm-kernel@lists.infradead.org; Tue, 07 Jul 2020 10:10:24 +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 87A21C0A; Tue, 7 Jul 2020 03:10:22 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C4C603F71E; Tue, 7 Jul 2020 03:10:19 -0700 (PDT) Date: Tue, 7 Jul 2020 11:10:17 +0100 From: Dave Martin To: Will Deacon Subject: Re: [PATCH 18/18] arm64: lto: Strengthen READ_ONCE() to acquire when CLANG_LTO=y Message-ID: <20200707101015.GH10992@arm.com> References: <20200630173734.14057-1-will@kernel.org> <20200630173734.14057-19-will@kernel.org> <20200706160820.GC10992@arm.com> <20200706183510.GA23766@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200706183510.GA23766@willie-the-truck> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200707_061024_207736_26EAB258 X-CRM114-Status: GOOD ( 29.06 ) 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: Mark Rutland , "Michael S. Tsirkin" , Peter Zijlstra , Catalin Marinas , Jason Wang , virtualization@lists.linux-foundation.org, Arnd Bergmann , Alan Stern , Sami Tolvanen , Matt Turner , kernel-team@android.com, Marco Elver , Kees Cook , "Paul E. McKenney" , Boqun Feng , Josh Triplett , Ivan Kokshaysky , linux-arm-kernel@lists.infradead.org, Richard Henderson , Nick Desaulniers , linux-kernel@vger.kernel.org, linux-alpha@vger.kernel.org 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 Mon, Jul 06, 2020 at 07:35:11PM +0100, Will Deacon wrote: > On Mon, Jul 06, 2020 at 05:08:20PM +0100, Dave Martin wrote: > > On Tue, Jun 30, 2020 at 06:37:34PM +0100, Will Deacon wrote: > > > diff --git a/arch/arm64/include/asm/rwonce.h b/arch/arm64/include/asm/rwonce.h > > > new file mode 100644 > > > index 000000000000..515e360b01a1 > > > --- /dev/null > > > +++ b/arch/arm64/include/asm/rwonce.h > > > @@ -0,0 +1,63 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +/* > > > + * Copyright (C) 2020 Google LLC. > > > + */ > > > +#ifndef __ASM_RWONCE_H > > > +#define __ASM_RWONCE_H > > > + > > > +#ifdef CONFIG_CLANG_LTO > > > + > > > +#include > > > +#include > > > + > > > +#ifndef BUILD_VDSO > > > + > > > +#ifdef CONFIG_AS_HAS_LDAPR > > > +#define __LOAD_RCPC(sfx, regs...) \ > > > + ALTERNATIVE( \ > > > + "ldar" #sfx "\t" #regs, \ > > > > ^ Should this be here? It seems that READ_ONCE() will actually read > > twice... even if that doesn't actually conflict with the required > > semantics of READ_ONCE(), it looks odd. > > It's patched at runtime, so it's either LDAR or LDAPR. Agh ignore me, I somehow failed to sport the ALTERNATIVE(). For my understanding -- my background here is a bit shaky -- the LDAPR gives us load-to-load order even if there is just a control dependency? If so (possibly dumb question): why can't we just turn this on unconditionally? Is there a significant performance impact? I'm still confused (or ignorant) though. If both loads are READ_ONCE() then switching to LDAPR presumably helps, but otherwise, once the compiler has reduced the address dependency to a control dependency can't it then go one step further and reverse the order of the loads? LDAPR wouldn't rescue us from that. Or does the "memory" clobber in READ_ONCE() fix that for all important cases? I can't see this mattering for local variables (where it definitely won't work), but I wonder whether static variables might not count as "memory" in some situations. Discounting ridiculous things like static register variables, I think the only way for a static variable not to count as memory would be if there are no writes to it that are reachable from any translation unit entry point (possibly after dead code removal). If so, maybe that's enough. > > Making a direct link between LTO and the memory model also seems highly > > spurious (as discussed in the other subthread) so can we have a comment > > explaining the reasoning? > > Sure, although like I say, this is more about helping to progress that > conversation. That's fair enough, but when there is a consensus it would be good to see it documented in the code _especially_ if we know that the fix won't address all instances of the problem and in any case works partly by accident. That doesn't mean it's not a good practical compromise, but it could be very confusing to unpick later on. Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel