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=-6.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 25BE4C433E6 for ; Tue, 29 Dec 2020 00:37:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E02AD207CF for ; Tue, 29 Dec 2020 00:37:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730549AbgL2AhH (ORCPT ); Mon, 28 Dec 2020 19:37:07 -0500 Received: from mail.kernel.org ([198.145.29.99]:46652 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730191AbgL2AhG (ORCPT ); Mon, 28 Dec 2020 19:37:06 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id F27B8221F8 for ; Tue, 29 Dec 2020 00:36:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1609202185; bh=VX6lw2IJ/z/WjavzOW0lIGvJ2PJ7vI+cPiWmgFXCEi4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=klsW25bgbw2Tvtt4HCiG69FQv4hapU7eCpnsPT82tb4h4tFP+WN5MPXW0TPAeMUpE W1Bb4qLn/j8DCfX8hQQAiqGNj6lXOleMlM9evE8WGtODI/8ERWwzB3l6X0EzFK+iXl aG8FI0bAypxmmj1Cy8x44bo2yHoNpPH/NHkJKPAgCIV0dUb1vP3BbS8Q9dZ/pLQvqM JZtvRnlxRT1vQPPEfvC8rvHwMH6aovwMSGTBsxsIRvSryxDzpGLkz8A85O/i6JFVz8 7hZEJ81kX43ATdY1/1QvFDU5BZScQNXGrav0we/3oQWufomEIHb9hrB9kNKBfqj888 5Bi+xNXHb2INg== Received: by mail-wr1-f41.google.com with SMTP id m5so12780453wrx.9 for ; Mon, 28 Dec 2020 16:36:24 -0800 (PST) X-Gm-Message-State: AOAM532+J6HOZugLpQQVCRWpLr4o2mE1IMKNFOg6R/RjboAEi+7bq0Qu lRYAaWhauq9PBBqvsZAPVpdMSNp2WffOBeB3pEjAxA== X-Google-Smtp-Source: ABdhPJztatywU6YOwSIVKLEbxZBNtEBa5iKSCwJTklOqEdD0ibLsWDGLhd7b2Cd3Fhyjte4DtXNSQn+FYl2eIOzIjDE= X-Received: by 2002:a5d:43c3:: with SMTP id v3mr52282122wrr.184.1609202183585; Mon, 28 Dec 2020 16:36:23 -0800 (PST) MIME-Version: 1.0 References: <1609199804.yrsu9vagzk.astroid@bobo.none> In-Reply-To: <1609199804.yrsu9vagzk.astroid@bobo.none> From: Andy Lutomirski Date: Mon, 28 Dec 2020 16:36:11 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC please help] membarrier: Rewrite sync_core_before_usermode() To: Nicholas Piggin Cc: Andy Lutomirski , Mathieu Desnoyers , X86 ML , Arnd Bergmann , Benjamin Herrenschmidt , Catalin Marinas , linux-arm-kernel , LKML , linuxppc-dev , Michael Ellerman , Paul Mackerras , stable , Will Deacon Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Dec 28, 2020 at 4:11 PM Nicholas Piggin wrote: > > Excerpts from Andy Lutomirski's message of December 28, 2020 4:28 am: > > The old sync_core_before_usermode() comments said that a non-icache-syncing > > return-to-usermode instruction is x86-specific and that all other > > architectures automatically notice cross-modified code on return to > > userspace. Based on my general understanding of how CPUs work and based on > > my atttempt to read the ARM manual, this is not true at all. In fact, x86 > > seems to be a bit of an anomaly in the other direction: x86's IRET is > > unusually heavyweight for a return-to-usermode instruction. > > "sync_core_before_usermode" as I've said says nothing to arch, or to the > scheduler, or to membarrier. Agreed. My patch tries to fix this. I agree that the name is bad and could be improved further. We should define what membarrier(...SYNC_CORE) actually does and have arch hooks to make it happen. > > So let's drop any pretense that we can have a generic way implementation > > behind membarrier's SYNC_CORE flush and require all architectures that opt > > in to supply their own. This means x86, arm64, and powerpc for now. Let's > > also rename the function from sync_core_before_usermode() to > > membarrier_sync_core_before_usermode() because the precise flushing details > > may very well be specific to membarrier, and even the concept of > > "sync_core" in the kernel is mostly an x86-ism. > > The concept of "sync_core" (x86: serializing instruction, powerpc: context > synchronizing instruction, etc) is not an x86-ism at all. x86 just wanted > to add a serializing instruction to generic code so it grew this nasty API, > but the concept applies broadly. I mean that the mapping from the name "sync_core" to its semantics is x86 only. The string "sync_core" appears in the kernel only in arch/x86, membarrier code, membarrier docs, and a single SGI driver that is x86-only. Sure, the idea of serializing things is fairly generic, but exactly what operations serialize what, when things need serialization, etc is quite architecture specific. Heck, on 486 you serialize the instruction stream with JMP. > > +static inline void membarrier_sync_core_before_usermode(void) > > +{ > > + /* > > + * XXX: I know basically nothing about powerpc cache management. > > + * Is this correct? > > + */ > > + isync(); > > This is not about memory ordering or cache management, it's about > pipeline management. Powerpc's return to user mode serializes the > CPU (aka the hardware thread, _not_ the core; another wrongness of > the name, but AFAIKS the HW thread is what is required for > membarrier). So this is wrong, powerpc needs nothing here. Fair enough. I'm happy to defer to you on the powerpc details. In any case, this just illustrates that we need feedback from a person who knows more about ARM64 than I do.