From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753961AbdIDRCd (ORCPT ); Mon, 4 Sep 2017 13:02:33 -0400 Received: from mout.gmx.net ([212.227.15.18]:50374 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753887AbdIDRCb (ORCPT ); Mon, 4 Sep 2017 13:02:31 -0400 Message-ID: <1504544520.22981.14.camel@gmx.de> Subject: Re: hotplug lockdep splat (tip) From: Mike Galbraith To: Peter Zijlstra Cc: LKML , Thomas Gleixner , Byungchul Park Date: Mon, 04 Sep 2017 19:02:00 +0200 In-Reply-To: <20170904153725.bxmsqb2fcvda5a2k@hirez.programming.kicks-ass.net> References: <1504350596.16793.44.camel@gmx.de> <1504421975.30792.32.camel@gmx.de> <20170904075502.74pgfd2twdfklspw@hirez.programming.kicks-ass.net> <1504531627.10288.5.camel@gmx.de> <20170904153725.bxmsqb2fcvda5a2k@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:73UTupS7aZ384BL3+RaoEnzFcwrmtilzDbksQNWBtj5F9TNLFtR XzgwyHOj9RihO5VGlmLr0SjdtzlIa/1MFxbgfOnm1xlXXnVFDZ5Bw1gYAuGp+SWUghd5I+y 40AGj3jVok0yWhvKV3wOTzq88elHhSuW42SvORGF5yetblMsEMuBNu8JfPXN2eqrKLSdDV0 VIx7kPV6iSdcKLlgd8GBA== X-UI-Out-Filterresults: notjunk:1;V01:K0:ZqmcHn237hQ=:H8+4inNR7NX0BZ+lSikXRX kiQVYbLeKgBeDp1GuvHNX4mYMrxULcUPX6M/QB5Qbh79xA0CvuAt0mialglatlodzdQgQexVr UVjK7k0qWLh9oC+9YEq3eFWexLJV5gqsCRI1c89wJv7jcP+FcLBANrAgbVVTBB40xmIEpciTD MxtO23wABVXA4LSxdIt0S5j22vo9sIO5E0EY9OVFRrKSVP9bhAadVoXsVkj5eKGsFjRH0AIF1 x8mnQu5kJ0UQBY8SxtdCyjNBK+An6AG7OleKG8bOCScnXJp8wVAETx/BtHXO1Adn49c3I6COP 4TVvyWT62RKpCaAPuGyQeCFrZe99spApuOtNv25O+ARLSZDWobsPNB0+Ei8j5nIYgxH59ZeHz 2LQvfxJieKbZ+ojNDfrreqAO+xB09Gnu24zH5K/vMPOhocopKE99LX4DuwxcFEoJofYMBIa/1 787BGZjQOPv/ObC9gzStrn7xuGFcuttNa1i61EHM/o8tp92rh0CIsnbA+ZC/DNqR5pWNDzd9P lPljUHm8qqw+4wOT19inCBo+/9Z+8h7HdUZP2/DyZhN3hgsUMio3MaiNlIafvcOsXg4mGndan 5o3TZYO4R+H62PsDUB0dc2iUEjb9I+7BZYAaKUn+5Kbb4w92ztfqfz03GXDLkK9aOxm74HFEX 1F0U71vu16EDtg2kOEBLZ9SlSyj8lP/5ni+a6rh1oWJePvXWr/IHrNE/QsYzCd4FyDQm4sA/j 6Kk1fzt3DrHE37ZpO9sz0y7QlAIedOLvO5cNop2cab1gj3OMpvO/0JX/Jyz1C71IoxcAIZhgp 3G8Ts1KBvBD0229Pp0BIebffDSGwMR7O/kfmq/nt2GlNyt8sAE= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2017-09-04 at 17:37 +0200, Peter Zijlstra wrote: > > Doth teh beloweth make nice? Yes, no more insta-gripe. > --- > kernel/cpu.c | 47 +++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 39 insertions(+), 8 deletions(-) > > diff --git a/kernel/cpu.c b/kernel/cpu.c > index acf5308fad51..2ab324d7ff7b 100644 > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -67,11 +67,15 @@ struct cpuhp_cpu_state { > static DEFINE_PER_CPU(struct cpuhp_cpu_state, cpuhp_state); > > #if defined(CONFIG_LOCKDEP) && defined(CONFIG_SMP) > -static struct lock_class_key cpuhp_state_key; > +static struct lock_class_key cpuhp_state_up_key; > +#ifdef CONFIG_HOTPLUG_CPU > +static struct lock_class_key cpuhp_state_down_key; > +#endif > static struct lockdep_map cpuhp_state_lock_map = > - STATIC_LOCKDEP_MAP_INIT("cpuhp_state", &cpuhp_state_key); > + STATIC_LOCKDEP_MAP_INIT("cpuhp_state-up", &cpuhp_state_up_key); > #endif > > + > /** > * cpuhp_step - Hotplug state machine step > * @name: Name of the step > @@ -533,6 +537,28 @@ void __init cpuhp_threads_init(void) > kthread_unpark(this_cpu_read(cpuhp_state.thread)); > } > > +/* > + * _cpu_down() and _cpu_up() have different lock ordering wrt st->done, but > + * because these two functions are globally serialized and st->done is private > + * to them, we can simply re-init st->done for each of them to separate the > + * lock chains. > + * > + * Must be macro to ensure we have two different call sites. > + */ > +#ifdef CONFIG_LOCKDEP > +#define lockdep_reinit_st_done() \ > +do { \ > + int __cpu; \ > + for_each_possible_cpu(__cpu) { \ > + struct cpuhp_cpu_state *st = \ > + per_cpu_ptr(&cpuhp_state, __cpu); \ > + init_completion(&st->done); \ > + } \ > +} while(0) > +#else > +#define lockdep_reinit_st_done() > +#endif > + > #ifdef CONFIG_HOTPLUG_CPU > /** > * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU > @@ -676,12 +702,6 @@ void cpuhp_report_idle_dead(void) > cpuhp_complete_idle_dead, st, 0); > } > > -#else > -#define takedown_cpu NULL > -#endif > - > -#ifdef CONFIG_HOTPLUG_CPU > - > /* Requires cpu_add_remove_lock to be held */ > static int __ref _cpu_down(unsigned int cpu, int tasks_frozen, > enum cpuhp_state target) > @@ -697,6 +717,10 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen, > > cpus_write_lock(); > > + lockdep_reinit_st_done(); > + lockdep_init_map(&cpuhp_state_lock_map, "cpuhp_state-down", > + &cpuhp_state_down_key, 0); > + > cpuhp_tasks_frozen = tasks_frozen; > > prev_state = st->state; > @@ -759,6 +783,9 @@ int cpu_down(unsigned int cpu) > return do_cpu_down(cpu, CPUHP_OFFLINE); > } > EXPORT_SYMBOL(cpu_down); > + > +#else > +#define takedown_cpu NULL > #endif /*CONFIG_HOTPLUG_CPU*/ > > /** > @@ -806,6 +833,10 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target) > > cpus_write_lock(); > > + lockdep_reinit_st_done(); > + lockdep_init_map(&cpuhp_state_lock_map, "cpuhp_state-up", > + &cpuhp_state_up_key, 0); > + > if (!cpu_present(cpu)) { > ret = -EINVAL; > goto out;