Messages in this thread | | | Date | Thu, 9 Nov 2023 20:20:22 +0800 | From | "liutie (A)" <> | Subject | Re: [Bug report] A variant deadlock issue of CPU hot-unplug operation vs. the CFS bandwidth timer |
| |
On 2023/11/7 22:57, Thomas Gleixner wrote: > > Bah. > > So we can actually migrate the hrtimers away from the outgoing CPU in > the dying callbacks. That's safe as nothing can queue an hrtimer remote > on the outgoing CPU because all other CPUs are spinwaiting with > interrupts disabled in stomp_machine() until the CPU marked itself > offline. > > Survived a quick test, but needs some scrunity and probably a sanity > check in the post dead stage. > > Thanks, > > tglx > --- > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -195,6 +195,7 @@ enum cpuhp_state { > CPUHP_AP_ARM_CORESIGHT_CTI_STARTING, > CPUHP_AP_ARM64_ISNDEP_STARTING, > CPUHP_AP_SMPCFD_DYING, > + CPUHP_AP_HRTIMERS_DYING, > CPUHP_AP_X86_TBOOT_DYING, > CPUHP_AP_ARM_CACHE_B15_RAC_DYING, > CPUHP_AP_ONLINE, > --- a/include/linux/hrtimer.h > +++ b/include/linux/hrtimer.h > @@ -531,9 +531,9 @@ extern void sysrq_timer_list_show(void); > > int hrtimers_prepare_cpu(unsigned int cpu); > #ifdef CONFIG_HOTPLUG_CPU > -int hrtimers_dead_cpu(unsigned int cpu); > +int hrtimers_cpu_dying(unsigned int cpu); > #else > -#define hrtimers_dead_cpu NULL > +#define hrtimers_cpu_dying NULL > #endif > > #endif > --- a/kernel/cpu.c > +++ b/kernel/cpu.c > @@ -2116,7 +2116,7 @@ static struct cpuhp_step cpuhp_hp_states > [CPUHP_HRTIMERS_PREPARE] = { > .name = "hrtimers:prepare", > .startup.single = hrtimers_prepare_cpu, > - .teardown.single = hrtimers_dead_cpu, > + .teardown.single = NULL, > }, > [CPUHP_SMPCFD_PREPARE] = { > .name = "smpcfd:prepare", > @@ -2208,6 +2208,12 @@ static struct cpuhp_step cpuhp_hp_states > .startup.single = NULL, > .teardown.single = smpcfd_dying_cpu, > }, > + [CPUHP_AP_HRTIMERS_DYING] = { > + .name = "hrtimers:dying", > + .startup.single = NULL, > + .teardown.single = hrtimers_cpu_dying, > + }, > + > /* Entry state on starting. Interrupts enabled from here on. Transient > * state for synchronsization */ > [CPUHP_AP_ONLINE] = { > --- a/kernel/time/hrtimer.c > +++ b/kernel/time/hrtimer.c > @@ -2219,29 +2219,22 @@ static void migrate_hrtimer_list(struct > } > } > > -int hrtimers_dead_cpu(unsigned int scpu) > +int hrtimers_cpu_dying(unsigned int dying_cpu) > { > struct hrtimer_cpu_base *old_base, *new_base; > - int i; > + int i, ncpu = cpumask_first(cpu_active_mask); > > - BUG_ON(cpu_online(scpu)); > - tick_cancel_sched_timer(scpu); > + tick_cancel_sched_timer(dying_cpu); > + > + old_base = this_cpu_ptr(&hrtimer_bases); > + new_base = &per_cpu(hrtimer_bases, ncpu); > > - /* > - * this BH disable ensures that raise_softirq_irqoff() does > - * not wakeup ksoftirqd (and acquire the pi-lock) while > - * holding the cpu_base lock > - */ > - local_bh_disable(); > - local_irq_disable(); > - old_base = &per_cpu(hrtimer_bases, scpu); > - new_base = this_cpu_ptr(&hrtimer_bases); > /* > * The caller is globally serialized and nobody else > * takes two locks at once, deadlock is not possible. > */ > - raw_spin_lock(&new_base->lock); > - raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING); > + raw_spin_lock(&old_base->lock); > + raw_spin_lock_nested(&new_base->lock, SINGLE_DEPTH_NESTING); > > for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) { > migrate_hrtimer_list(&old_base->clock_base[i], > @@ -2252,15 +2245,13 @@ int hrtimers_dead_cpu(unsigned int scpu) > * The migration might have changed the first expiring softirq > * timer on this CPU. Update it. > */ > - hrtimer_update_softirq_timer(new_base, false); > + __hrtimer_get_next_event(new_base, HRTIMER_ACTIVE_SOFT); > + /* Tell the other CPU to retrigger the next event */ > + smp_call_function_single(ncpu, retrigger_next_event, NULL, 0); > > - raw_spin_unlock(&old_base->lock); > raw_spin_unlock(&new_base->lock); > + raw_spin_unlock(&old_base->lock); > > - /* Check, if we got expired work to do */ > - __hrtimer_peek_ahead_timers(); > - local_irq_enable(); > - local_bh_enable(); > return 0; > } >
Thanks for the patch. Tested in v6.6 and this patch works.
Tested-by: Liu Tie <liutie4@huawei.com>
Best regards, Tie
| |