Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753057Ab3IZQVj (ORCPT ); Thu, 26 Sep 2013 12:21:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32808 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751498Ab3IZQVi (ORCPT ); Thu, 26 Sep 2013 12:21:38 -0400 Date: Thu, 26 Sep 2013 18:14:26 +0200 From: Oleg Nesterov To: Peter Zijlstra Cc: "Paul E. McKenney" , Mel Gorman , Rik van Riel , Srikar Dronamraju , Ingo Molnar , Andrea Arcangeli , Johannes Weiner , Linux-MM , LKML , Thomas Gleixner , Steven Rostedt Subject: Re: [PATCH] hotplug: Optimize {get,put}_online_cpus() Message-ID: <20130926161426.GA14372@redhat.com> References: <20130923173203.GA20392@redhat.com> <20130924202423.GW12926@twins.programming.kicks-ass.net> <20130925155515.GA17447@redhat.com> <20130925174307.GA3220@laptop.programming.kicks-ass.net> <20130925175055.GA25914@redhat.com> <20130925184015.GC3657@laptop.programming.kicks-ass.net> <20130925212200.GA7959@linux.vnet.ibm.com> <20130926111042.GS3081@twins.programming.kicks-ass.net> <20130926155321.GA4342@redhat.com> <20130926161311.GG3657@laptop.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130926161311.GG3657@laptop.programming.kicks-ass.net> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1557 Lines: 44 On 09/26, Peter Zijlstra wrote: > > On Thu, Sep 26, 2013 at 05:53:21PM +0200, Oleg Nesterov wrote: > > On 09/26, Peter Zijlstra wrote: > > > void cpu_hotplug_done(void) > > > { > > > - cpu_hotplug.active_writer = NULL; > > > - mutex_unlock(&cpu_hotplug.lock); > > > + /* Signal the writer is done, no fast path yet. */ > > > + __cpuhp_state = readers_slow; > > > + wake_up_all(&cpuhp_readers); > > > + > > > + /* > > > + * The wait_event()/wake_up_all() prevents the race where the readers > > > + * are delayed between fetching __cpuhp_state and blocking. > > > + */ > > > + > > > + /* See percpu_up_write(); readers will no longer attempt to block. */ > > > + synchronize_sched(); > > > > Shouldn't you move wake_up_all(&cpuhp_readers) down after > > synchronize_sched() (or add another one) ? To ensure that a reader can't > > see state = BLOCK after wakeup(). > > Well, if they are blocked, the wake_up_all() will do an actual > try_to_wake_up() which issues a MB as per smp_mb__before_spinlock(). Yes. Everything is fine with the already blocked readers. I meant the new reader which still can see state = BLOCK after we do wakeup(), but I didn't notice it should do __wait_event() which takes the lock unconditionally, it must see the change after that. > Right? Yes, I was wrong, thanks. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/