From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751694AbdJEPwK (ORCPT ); Thu, 5 Oct 2017 11:52:10 -0400 Received: from mail.efficios.com ([167.114.142.141]:51331 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751613AbdJEPwG (ORCPT ); Thu, 5 Oct 2017 11:52:06 -0400 Date: Thu, 5 Oct 2017 15:53:36 +0000 (UTC) From: Mathieu Desnoyers To: Nicholas Piggin Cc: "Paul E. McKenney" , linux-kernel , Ingo Molnar , Lai Jiangshan , dipankar , Andrew Morton , Josh Triplett , Thomas Gleixner , Peter Zijlstra , rostedt , David Howells , Eric Dumazet , fweisbec , Oleg Nesterov , Boqun Feng , Andrew Hunter , maged michael , gromer , Avi Kivity , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Dave Watson , Alan Stern , Will Deacon , Andy Lutomirski , Ingo Molnar , Alexander Viro , linuxppc-dev , linux-arch Message-ID: <1411969338.31702.1507218816138.JavaMail.zimbra@efficios.com> In-Reply-To: <20171005142325.5a407f18@roar.ozlabs.ibm.com> References: <20171004213734.GA11463@linux.vnet.ibm.com> <1507153075-12345-1-git-send-email-paulmck@linux.vnet.ibm.com> <20171005142325.5a407f18@roar.ozlabs.ibm.com> Subject: Re: [PATCH tip/core/rcu 1/3] membarrier: Provide register expedited private 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: Provide register expedited private command Thread-Index: EnFeh5GStOUhrWWhbOUE8zP9hN7eBg== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- On Oct 5, 2017, at 12:23 AM, Nicholas Piggin npiggin@gmail.com wrote: > On Wed, 4 Oct 2017 14:37:53 -0700 > "Paul E. McKenney" wrote: > >> From: Mathieu Desnoyers >> >> Provide a new command allowing processes to register their intent to use >> the private expedited command. >> >> This allows PowerPC to skip the full memory barrier in switch_mm(), and >> only issue the barrier when scheduling into a task belonging to a >> process that has registered to use expedited private. >> >> Processes are now required to register before using >> MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM. >> >> Changes since v1: >> - Use test_ti_thread_flag(next, ...) instead of test_thread_flag() in >> powerpc membarrier_arch_sched_in(), given that we want to specifically >> check the next thread state. >> - Add missing ARCH_HAS_MEMBARRIER_HOOKS in Kconfig. >> - Use task_thread_info() to pass thread_info from task to >> *_ti_thread_flag(). >> >> Changes since v2: >> - Move membarrier_arch_sched_in() call to finish_task_switch(). >> - Check for NULL t->mm in membarrier_arch_fork(). >> - Use membarrier_sched_in() in generic code, which invokes the >> arch-specific membarrier_arch_sched_in(). This fixes allnoconfig >> build on PowerPC. >> - Move asm/membarrier.h include under CONFIG_MEMBARRIER, fixing >> allnoconfig build on PowerPC. >> - Build and runtime tested on PowerPC. >> >> Changes since v3: >> - Simply rely on copy_mm() to copy the membarrier_private_expedited mm >> field on fork. >> - powerpc: test thread flag instead of reading >> membarrier_private_expedited in membarrier_arch_fork(). >> - powerpc: skip memory barrier in membarrier_arch_sched_in() if coming >> from kernel thread, since mmdrop() implies a full barrier. >> - Set membarrier_private_expedited to 1 only after arch registration >> code, thus eliminating a race where concurrent commands could succeed >> when they should fail if issued concurrently with process >> registration. >> - Use READ_ONCE() for membarrier_private_expedited field access in >> membarrier_private_expedited. Matches WRITE_ONCE() performed in >> process registration. >> >> Changes since v4: >> - Move powerpc hook from sched_in() to switch_mm(), based on feedback >> from Nicholas Piggin. > > For now, the powerpc approach is okay by me. I plan to test > others (e.g., taking runqueue locks) on larger systems, but that can > be sent as an incremental patch at a later time. > > The main thing I would like is for people to review the userspace API. > > >> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h >> index 3a19c253bdb1..4af1b719c65f 100644 >> --- a/include/linux/sched/mm.h >> +++ b/include/linux/sched/mm.h >> @@ -205,4 +205,54 @@ static inline void memalloc_noreclaim_restore(unsigned int >> flags) >> current->flags = (current->flags & ~PF_MEMALLOC) | flags; >> } >> >> +#ifdef CONFIG_MEMBARRIER >> + >> +#ifdef CONFIG_ARCH_HAS_MEMBARRIER_HOOKS >> +#include >> +#else >> +static inline void membarrier_arch_switch_mm(struct mm_struct *prev, >> + struct mm_struct *next, struct task_struct *tsk) >> +{ >> +} > > This is no longer required in architecture independent code, is it? Yes, good point! I'll remove this unused code in a follow up patch. Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com