From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933687AbdIYItv (ORCPT ); Mon, 25 Sep 2017 04:49:51 -0400 Received: from LGEAMRELO11.lge.com ([156.147.23.51]:34810 "EHLO lgeamrelo11.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933009AbdIYItt (ORCPT ); Mon, 25 Sep 2017 04:49:49 -0400 X-Original-SENDERIP: 156.147.1.125 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 10.177.222.33 X-Original-MAILFROM: byungchul.park@lge.com Date: Mon, 25 Sep 2017 17:49:35 +0900 From: Byungchul Park To: Peter Zijlstra Cc: mingo@kernel.org, tglx@linutronix.de, linux-kernel@vger.kernel.org, rostedt@goodmis.org, bigeasy@linutronix.de, efault@gmx.de, max.byungchul.park@gmail.com, kernel-team@lge.com Subject: Re: [PATCH 5/7] smp/hotplug: Differentiate the AP completion between up and down Message-ID: <20170925084935.GJ5994@X58A-UD3R> References: <20170920170014.548896195@infradead.org> <20170920170546.872472799@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170920170546.872472799@infradead.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 20, 2017 at 07:00:19PM +0200, Peter Zijlstra wrote: > With lockdep-crossrelease we get deadlock reports that span cpu-up and > cpu-down chains. Such deadlocks cannot possibly happen because cpu-up > and cpu-down are globally serialized. > > takedown_cpu() > irq_lock_sparse() > wait_for_completion(&st->done) > > cpuhp_thread_fun > cpuhp_up_callback > cpuhp_invoke_callback > irq_affinity_online_cpu > irq_local_spare() > irq_unlock_sparse() > complete(&st->done) > > Now that we have consistent AP state, we can trivially separate the > AP completion between up and down using st->bringup. Acked-by: Byungchul Park > Signed-off-by: Peter Zijlstra (Intel) > --- > kernel/cpu.c | 33 ++++++++++++++++++++++++--------- > 1 file changed, 24 insertions(+), 9 deletions(-) > > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -46,7 +46,8 @@ > * @bringup: Single callback bringup or teardown selector > * @cb_state: The state for a single callback (install/uninstall) > * @result: Result of the operation > - * @done: Signal completion to the issuer of the task > + * @done_up: Signal completion to the issuer of the task for cpu-up > + * @done_down: Signal completion to the issuer of the task for cpu-down > */ > struct cpuhp_cpu_state { > enum cpuhp_state state; > @@ -61,7 +62,8 @@ struct cpuhp_cpu_state { > struct hlist_node *last; > enum cpuhp_state cb_state; > int result; > - struct completion done; > + struct completion done_up; > + struct completion done_down; > #endif > }; > > @@ -90,6 +92,18 @@ static void inline cpuhp_lock_release(bo > > #endif > > +static inline void wait_for_ap_thread(struct cpuhp_cpu_state *st, bool bringup) > +{ > + struct completion *done = bringup ? &st->done_up : &st->done_down; > + wait_for_completion(done); > +} > + > +static inline void complete_ap_thread(struct cpuhp_cpu_state *st, bool bringup) > +{ > + struct completion *done = bringup ? &st->done_up : &st->done_down; > + complete(done); > +} > + > /** > * cpuhp_step - Hotplug state machine step > * @name: Name of the step > @@ -368,7 +382,7 @@ static void __cpuhp_kick_ap(struct cpuhp > smp_mb(); > st->should_run = true; > wake_up_process(st->thread); > - wait_for_completion(&st->done); > + wait_for_ap_thread(st, st->bringup); > } > > static int cpuhp_kick_ap(struct cpuhp_cpu_state *st, enum cpuhp_state target) > @@ -391,7 +405,7 @@ static int bringup_wait_for_ap(unsigned > struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu); > > /* Wait for the CPU to reach CPUHP_AP_ONLINE_IDLE */ > - wait_for_completion(&st->done); > + wait_for_ap_thread(st, true); > if (WARN_ON_ONCE((!cpu_online(cpu)))) > return -ECANCELED; > > @@ -490,7 +504,8 @@ static void cpuhp_create(unsigned int cp > { > struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu); > > - init_completion(&st->done); > + init_completion(&st->done_up); > + init_completion(&st->done_down); > } > > static int cpuhp_should_run(unsigned int cpu) > @@ -584,7 +599,7 @@ static void cpuhp_thread_fun(unsigned in > cpuhp_lock_release(bringup); > > if (!st->should_run) > - complete(&st->done); > + complete_ap_thread(st, bringup); > } > > /* Invoke a single callback on a remote cpu */ > @@ -780,7 +795,7 @@ static int takedown_cpu(unsigned int cpu > * > * Wait for the stop thread to go away. > */ > - wait_for_completion(&st->done); > + wait_for_ap_thread(st, false); > BUG_ON(st->state != CPUHP_AP_IDLE_DEAD); > > /* Interrupts are moved away from the dying cpu, reenable alloc/free */ > @@ -799,7 +814,7 @@ static void cpuhp_complete_idle_dead(voi > { > struct cpuhp_cpu_state *st = arg; > > - complete(&st->done); > + complete_ap_thread(st, false); > } > > void cpuhp_report_idle_dead(void) > @@ -938,7 +953,7 @@ void cpuhp_online_idle(enum cpuhp_state > return; > > st->state = CPUHP_AP_ONLINE_IDLE; > - complete(&st->done); > + complete_ap_thread(st, true); > } > > /* Requires cpu_add_remove_lock to be held */ >