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=-2.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=no 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 915B8C433E0 for ; Thu, 2 Jul 2020 09:50:06 +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 6382E20748 for ; Thu, 2 Jul 2020 09:50:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="EeeVfd09"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="pvvSqoKu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6382E20748 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org 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=aE0g9yolvdEfHfOGR3XUt/HkfhNxApkCPKN3P2nIpvY=; b=EeeVfd09Q0btMCeEZ2m0ul5nr rGMYKaPQhXytGrYz2todbZIICz+7n2H2WIdIBgmDUx8WYp6Nu3fnsmfLM7cQ8hoYm7RT4q/1O1f9V QzJzXsqMlbgjCZRtx5gNgOLgG5ttKdYbC3JVekEokArfL1PBB2AHapHQgkzKCsLrq840bOCmjEzOr tSqi6yAXTPtYeT97CvzQrsVbSeFHiRpwNz4cKe6l8g1lMyokagYBbUJedWNM237Ta5RpPKuwoENEE BTOgdQjQC+cQe9TCRuVtwOwFfw/DCQ84NtuoeQWfEOSbt9Zde2VdumYMGh5sFob/Js3NNURMW1UFr yw4WuR4ZA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jqvpk-0004PP-Hd; Thu, 02 Jul 2020 09:48:44 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jqvph-0004NP-0d for linux-arm-kernel@lists.infradead.org; Thu, 02 Jul 2020 09:48:42 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id E995020874; Thu, 2 Jul 2020 09:48:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1593683320; bh=8To++DcBmhQUuz2eqkOVWHaWbqWUTZ6jhRKebFqKYqM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pvvSqoKuX6EoQp3iR1pOFMS1VuWVSn9bXK/IVDhTQ1Bb8G4k5KaaptoPD6grev0aX fNdI68lGWQGkuhsT0oBuiB3hXEx9ZLANBXAgOTDc+ixQmjHmi9pG8qTts8l/Xw1PHx R1JOSfYU0SMNZ/OPMRKZNPdqf1Ztg/D9u+MRtgqw= Date: Thu, 2 Jul 2020 10:48:33 +0100 From: Will Deacon To: Mark Rutland Subject: Re: [PATCH 04/18] alpha: Override READ_ONCE() with barriered implementation Message-ID: <20200702094833.GA16248@willie-the-truck> References: <20200630173734.14057-1-will@kernel.org> <20200630173734.14057-5-will@kernel.org> <20200702093239.GA15391@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200702093239.GA15391@C02TD0UTHF1T.local> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200702_054841_211408_A4EE8D61 X-CRM114-Status: GOOD ( 17.61 ) 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: Marco Elver , Kees Cook , "Paul E. McKenney" , "Michael S. Tsirkin" , Peter Zijlstra , Catalin Marinas , Jason Wang , Nick Desaulniers , linux-kernel@vger.kernel.org, Josh Triplett , Ivan Kokshaysky , linux-arm-kernel@lists.infradead.org, Sami Tolvanen , linux-alpha@vger.kernel.org, Alan Stern , Matt Turner , virtualization@lists.linux-foundation.org, kernel-team@android.com, Boqun Feng , Arnd Bergmann , Richard Henderson 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 Thu, Jul 02, 2020 at 10:32:39AM +0100, Mark Rutland wrote: > On Tue, Jun 30, 2020 at 06:37:20PM +0100, Will Deacon wrote: > > -#define read_barrier_depends() __asm__ __volatile__("mb": : :"memory") > > +#define __smp_load_acquire(p) \ > > +({ \ > > + __unqual_scalar_typeof(*p) ___p1 = \ > > + (*(volatile typeof(___p1) *)(p)); \ > > + compiletime_assert_atomic_type(*p); \ > > + ___p1; \ > > +}) > > Sorry if I'm being thick, but doesn't this need a barrier after the > volatile access to provide the acquire semantic? > > IIUC prior to this commit alpha would have used the asm-generic > __smp_load_acquire, i.e. > > | #ifndef __smp_load_acquire > | #define __smp_load_acquire(p) \ > | ({ \ > | __unqual_scalar_typeof(*p) ___p1 = READ_ONCE(*p); \ > | compiletime_assert_atomic_type(*p); \ > | __smp_mb(); \ > | (typeof(*p))___p1; \ > | }) > | #endif > > ... where the __smp_mb() would be alpha's mb() from earlier in the patch > context, i.e. > > | #define mb() __asm__ __volatile__("mb": : :"memory") > > ... so don't we need similar before returning ___p1 above in > __smp_load_acquire() (and also matching the old read_barrier_depends())? > > [...] > > > +#include > > + > > +/* > > + * Alpha is apparently daft enough to reorder address-dependent loads > > + * on some CPU implementations. Knock some common sense into it with > > + * a memory barrier in READ_ONCE(). > > + */ > > +#define __READ_ONCE(x) __smp_load_acquire(&(x)) > > As above, I don't see a memory barrier implied here, so this doesn't > look quite right. You're right, and Peter spotted the same thing off-list. I've reworked locally so that the mb() ends up in __READ_ONCE() and __smp_load_acquire() calles __READ_ONCE() instead of the other way round (which made more sense before the rework in the merge window). Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel