Messages in this thread | | | Subject | Re: hotplug lockdep splat (tip) | From | Mike Galbraith <> | Date | Mon, 04 Sep 2017 19:02:00 +0200 |
| |
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;
| |