From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathieu Desnoyers Subject: Re: [RFC PATCH for 4.15 05/10] x86: Introduce sync_core_before_usermode Date: Fri, 10 Nov 2017 23:13:50 +0000 (UTC) Message-ID: <641795751.13070.1510355630950.JavaMail.zimbra@efficios.com> References: <20171110213717.12457-1-mathieu.desnoyers@efficios.com> <20171110213717.12457-6-mathieu.desnoyers@efficios.com> <1007217384.13056.1510352430448.JavaMail.zimbra@efficios.com> <115800448.13060.1510353174125.JavaMail.zimbra@efficios.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <115800448.13060.1510353174125.JavaMail.zimbra@efficios.com> Sender: linux-arch-owner@vger.kernel.org To: Andy Lutomirski Cc: Boqun Feng , Peter Zijlstra , "Paul E. McKenney" , linux-kernel , linux-api , Andrew Hunter , maged michael , Avi Kivity , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Dave Watson , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Andrea Parri , "Russell King, ARM Linux" , Greg Hackmann , Will Deacon , David List-Id: linux-api@vger.kernel.org ----- On Nov 10, 2017, at 5:32 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote: > ----- On Nov 10, 2017, at 5:20 PM, Mathieu Desnoyers > mathieu.desnoyers@efficios.com wrote: > >> ----- On Nov 10, 2017, at 5:02 PM, Andy Lutomirski luto@kernel.org wrote: >> >>> On Fri, Nov 10, 2017 at 1:37 PM, Mathieu Desnoyers >>> wrote: >>>> Introduce an architecture function that ensures the current CPU >>>> issues a core serializing instruction before returning to usermode. >>>> >>>> This is needed to fix an existing core serialization bug on >>>> thread migration, and also needed by the membarrier "sync_core" command. >>>> >>>> Architectures defining the sync_core_before_usermode() static inline >>>> need to define ARCH_HAS_SYNC_CORE_BEFORE_USERMODE. >>>> >>>> Signed-off-by: Mathieu Desnoyers >>>> CC: Peter Zijlstra >>>> CC: Andy Lutomirski >>>> CC: Paul E. McKenney >>>> CC: Boqun Feng >>>> CC: Andrew Hunter >>>> CC: Maged Michael >>>> CC: Avi Kivity >>>> CC: Benjamin Herrenschmidt >>>> CC: Paul Mackerras >>>> CC: Michael Ellerman >>>> CC: Dave Watson >>>> CC: Thomas Gleixner >>>> CC: Ingo Molnar >>>> CC: "H. Peter Anvin" >>>> CC: Andrea Parri >>>> CC: Russell King >>>> CC: Greg Hackmann >>>> CC: Will Deacon >>>> CC: David Sehr >>>> CC: Linus Torvalds >>>> CC: x86@kernel.org >>>> CC: linux-arch@vger.kernel.org >>>> --- >>>> arch/x86/Kconfig | 1 + >>>> arch/x86/include/asm/processor.h | 10 ++++++++++ >>>> include/linux/processor.h | 6 ++++++ >>>> 3 files changed, 17 insertions(+) >>>> >>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>>> index 01f78c1d40b5..54fbb8960d94 100644 >>>> --- a/arch/x86/Kconfig >>>> +++ b/arch/x86/Kconfig >>>> @@ -62,6 +62,7 @@ config X86 >>>> select ARCH_HAS_SG_CHAIN >>>> select ARCH_HAS_STRICT_KERNEL_RWX >>>> select ARCH_HAS_STRICT_MODULE_RWX >>>> + select ARCH_HAS_SYNC_CORE_BEFORE_USERMODE >>>> select ARCH_HAS_UBSAN_SANITIZE_ALL >>>> select ARCH_HAS_ZONE_DEVICE if X86_64 >>>> select ARCH_HAVE_NMI_SAFE_CMPXCHG >>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h >>>> index bdac19ab2488..6ce996a7c730 100644 >>>> --- a/arch/x86/include/asm/processor.h >>>> +++ b/arch/x86/include/asm/processor.h >>>> @@ -706,6 +706,16 @@ static inline void sync_core(void) >>>> #endif >>>> } >>>> >>>> +/* >>>> + * Ensure that a core serializing instruction is issued before returning >>>> + * to user-mode. x86 implements return to user-space through sysexit and >>>> + * sysretq, which are not core serializing. >>>> + */ >>>> +static inline void sync_core_before_usermode(void) >>>> +{ >>>> + sync_core(); >>>> +} >>> >>> Make this if (!in_interrupt()) sync_core(); please. We can optimize >>> it better later on. >> >> Sure, done. It will be part of the next version of that patch. > > Of course, using in_interrupt() from linux/interrupt.h from > asm/processor.h is not such a good idea (circular dependency). > > Any recommendation on where to move that static inline on x86 ? Actually, for the needs of the fix, I plan on not putting the if (!in_interrupt()) check, given that the only caller will be the scheduler, not in an interrupt context. We can always revisit this optimization-wise if we end up using in interrupt handlers in the future. Is that ok with you ? Thanks, Mathieu > > Thanks, > > Mathieu > > >> >> Thanks! >> >> Mathieu >> >> >> -- >> Mathieu Desnoyers >> EfficiOS Inc. >> http://www.efficios.com > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com