Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752405AbdG1PgF (ORCPT ); Fri, 28 Jul 2017 11:36:05 -0400 Received: from mail.efficios.com ([167.114.142.141]:39838 "EHLO mail.efficios.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751975AbdG1PgD (ORCPT ); Fri, 28 Jul 2017 11:36:03 -0400 Date: Fri, 28 Jul 2017 15:38:15 +0000 (UTC) From: Mathieu Desnoyers To: Peter Zijlstra Cc: "Paul E. McKenney" , linux-kernel , Boqun Feng , Andrew Hunter , maged michael , gromer , Avi Kivity , Michael Ellerman , Nicholas Piggin , Benjamin Herrenschmidt , Palmer Dabbelt Message-ID: <2118431661.29566.1501256295573.JavaMail.zimbra@efficios.com> In-Reply-To: <20170728115702.5vgnvwhmbbmyrxbf@hirez.programming.kicks-ass.net> References: <20170727211314.32666-1-mathieu.desnoyers@efficios.com> <20170728085532.ylhuz2irwmgpmejv@hirez.programming.kicks-ass.net> <20170728115702.5vgnvwhmbbmyrxbf@hirez.programming.kicks-ass.net> Subject: Re: [RFC PATCH v2] membarrier: 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.9_GA_1794 (ZimbraWebClient - FF52 (Linux)/8.7.9_GA_1794) Thread-Topic: membarrier: expedited private command Thread-Index: sPaxwdc7r5cmFFDYWxiHdYZBF7Lxcw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2482 Lines: 77 ----- On Jul 28, 2017, at 7:57 AM, Peter Zijlstra peterz@infradead.org wrote: > On Fri, Jul 28, 2017 at 10:55:32AM +0200, Peter Zijlstra wrote: >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index e9785f7aed75..33f34a201255 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -2641,8 +2641,18 @@ static struct rq *finish_task_switch(struct task_struct >> *prev) >> finish_arch_post_lock_switch(); >> >> fire_sched_in_preempt_notifiers(current); >> + >> + /* >> + * For CONFIG_MEMBARRIER we need a full memory barrier after the >> + * rq->curr assignment. Not all architectures have one in either >> + * switch_to() or switch_mm() so we use (and complement) the one >> + * implied by mmdrop()'s atomic_dec_and_test(). >> + */ >> if (mm) >> mmdrop(mm); >> + else if (IS_ENABLED(CONFIG_MEMBARRIER)) >> + smp_mb(); >> + >> if (unlikely(prev_state == TASK_DEAD)) { >> if (prev->sched_class->task_dead) >> prev->sched_class->task_dead(prev); >> >> > >> a whole bunch of architectures don't in fact need this extra barrier at all. > > In fact, I'm fairly sure its only PPC. > > Because only ARM64 and PPC actually implement ACQUIRE/RELEASE with > anything other than smp_mb() (for now, Risc-V is in this same boat and > MIPS could be if they ever sort out their fancy barriers). > > TSO archs use a regular STORE for RELEASE, but all their atomics imply a > smp_mb() and there are enough around to make one happen (typically > mm_cpumask updates). > > Everybody else, aside from ARM64 and PPC must use smp_mb() for > ACQUIRE/RELEASE. > > ARM64 has a super duper barrier in switch_to(). > > Which only leaves PPC stranded.. but the 'good' news is that mpe says > they'll probably need a barrier in switch_mm() in any case. As I pointed out in my other email, I plan to do this: --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2636,6 +2636,11 @@ static struct rq *finish_task_switch(struct task_struct *prev) vtime_task_switch(prev); perf_event_task_sched_in(prev, current); finish_lock_switch(rq, prev); + /* + * The membarrier system call requires a full memory barrier + * after storing to rq->curr, before going back to user-space. + */ + smp_mb__after_unlock_lock(); finish_arch_post_lock_switch(); fire_sched_in_preempt_notifiers(current); Thoughts ? Thanks, Mathieu -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com