From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753885AbbJNTHX (ORCPT ); Wed, 14 Oct 2015 15:07:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45679 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753493AbbJNTHV (ORCPT ); Wed, 14 Oct 2015 15:07:21 -0400 Date: Wed, 14 Oct 2015 21:03:56 +0200 From: Oleg Nesterov To: Peter Zijlstra Cc: heiko.carstens@de.ibm.com, Tejun Heo , Ingo Molnar , Rik van Riel , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] stop_machine: ensure that a queued callback will be called before cpu_stop_park() Message-ID: <20151014190356.GA8905@redhat.com> References: <20151008145059.GA17916@redhat.com> <20151008145131.GA18139@redhat.com> <20151014153432.GU3604@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151014153432.GU3604@twins.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 On 10/14, Peter Zijlstra wrote: > > On Thu, Oct 08, 2015 at 04:51:31PM +0200, Oleg Nesterov wrote: > > cpu_stop_queue_work() checks stopper->enabled before it queues the > > work, but ->enabled == T can only guarantee cpu_stop_signal_done() > > if we race with cpu_down(). > > > > This is not enough for stop_two_cpus() or stop_machine(), they will > > deadlock if multi_cpu_stop() won't be called by one of the target > > CPU's. stop_machine/stop_cpus are fine, they rely on stop_cpus_mutex. > > But stop_two_cpus() has to check cpu_active() to avoid the same race > > with hotplug, and this check is very unobvious and probably not even > > correct if we race with cpu_up(). > > > > Change cpu_down() pass to clear ->enabled before cpu_stopper_thread() > > flushes the pending ->works and returns with KTHREAD_SHOULD_PARK set. > > > > Note also that smpboot_thread_call() calls cpu_stop_unpark() which > > sets enabled == T at CPU_ONLINE stage, so this CPU can't go away until > > cpu_stopper_thread() is called at least once. This all means that if > > cpu_stop_queue_work() succeeds, we know that work->fn() will be called. > > This hard relies on the fact that cpu_down uses stop machine, right? Not really. > IIRC part of the hotplug rework Thomas is doing is geared towards > breaking away from stop machine. There is nothing fundamental about > hot-unplug that requires stop machine. cpu_down() should park/kill/whatever the percpu stopper thread anyway. And this path should clear ->enabled, it can also flush the pending works. And we need this anyway even if cpu_down() won't use stop_machine(), I think. Oleg.