From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754362AbdKITe7 (ORCPT ); Thu, 9 Nov 2017 14:34:59 -0500 Received: from mail.efficios.com ([167.114.142.141]:54282 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753702AbdKITe6 (ORCPT ); Thu, 9 Nov 2017 14:34:58 -0500 Date: Thu, 9 Nov 2017 19:35:25 +0000 (UTC) From: Mathieu Desnoyers To: Andy Lutomirski Cc: "Paul E. McKenney" , linux-kernel , linux-api , Peter Zijlstra , Boqun Feng , Andrew Hunter , maged michael , gromer , Avi Kivity , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Dave Watson , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Andrea Parri , x86 Message-ID: <452803516.12441.1510256125252.JavaMail.zimbra@efficios.com> In-Reply-To: References: <20171108183514.3306-1-mathieu.desnoyers@efficios.com> <20171108183514.3306-6-mathieu.desnoyers@efficios.com> Subject: Re: [RFC PATCH for 4.15 5/6] membarrier: x86: Provide core serializing command MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [167.114.142.141] X-Mailer: Zimbra 8.7.11_GA_1854 (ZimbraWebClient - FF52 (Linux)/8.7.11_GA_1854) Thread-Topic: membarrier: x86: Provide core serializing command Thread-Index: 47tVJ/O5E4UrbcQ71tN/pvgD9Gb3ag== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- On Nov 9, 2017, at 2:07 PM, Andy Lutomirski luto@kernel.org wrote: > On Wed, Nov 8, 2017 at 10:35 AM, Mathieu Desnoyers > wrote: > >> +/* >> + * x86-64 implements return to user-space through sysret, which is not a >> + * core-serializing instruction. Therefore, we need an explicit core >> + * serializing instruction after going from kernel thread back to >> + * user-space thread (active_mm moved back to current mm). >> + */ >> +static inline void membarrier_arch_mm_sync_core(struct mm_struct *mm) >> +{ >> + if (likely(!(atomic_read(&mm->membarrier_state) & >> + MEMBARRIER_STATE_SYNC_CORE))) >> + return; >> + sync_core(); >> +} > > IMO there should be an extremely clear specification somewhere for > what this function is supposed to do. > > If I remember correctly, it's supposed to promise that the icache is > synced before the next time we return to usermode for the current mm > on this CPU. If that's correct, then let's document it very > explicitly and let's also drop the "membarrier" from the name -- it's > a primitive we'll need anyway given the existing migration bug. I understand that on x86 (specifically), synchronizing the icache and doing a core serializing instruction may mean the same thing. However, on architectures like ARM, icache sync differs from core serialization. Those architectures typically have either a user-space accessible instruction or a system call to perform the icache flush. The missing part for JIT is core serialization (also called context synchronization). icache flush is already handled by pre-existing means. So the promise here given by membarrier_arch_mm_sync_core() is that a core serializing instruction is issued before the next time we return to usermode on the current thread. However, we only need that guarantee if the current thread's mm is a registered MEMBARRIER_{...}_SYNC_CORE user. Regarding the existing migration bug, what I think you want is a kind of weaker "sync_core()", which ensures that a core serializing instruction is issued before the next time the current thread returns to usermode. It could be e.g.: set_tsk_need_core_sync() which would set a TIF_NEED_CORE_SYNC thread flag on the current thread. Clearly, when this kind of thread flag is introduced as an optimization over sync_core(), I would like to use that. However, I don't think it replaces the membarrier_arch_mm_sync_core() entirely, given that it would not check for the mm membarrier "SYNC_CORE" registration state. It appears to me to be merely an optimization over directly invoking sync_core. What I suggest is that I update the comment above membarrier_arch_mm_sync_core to spell out more clearly that all we need is to have a core serializing instruction issued before the next time the current thread returns to user-space. I can still use sync_core for now, and we can then improve the implementation whenever a new thread flag is introduced. The new comment would look like: /* * x86-64 implements return to user-space through sysret, which is not a * core-serializing instruction. Therefore, we need an explicit core * serializing instruction after going from kernel thread back to * user-space thread (active_mm moved back to current mm). * * This function ensures that a core serializing instruction is issued * before the current thread returns to user-space. */ Thoughts ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com