Messages in this thread | | | From | Hsin-Yi Wang <> | Date | Mon, 13 Jan 2020 23:12:09 +0800 | Subject | Re: [PATCH RFC v3 1/3] reboot: support hotplug CPUs before reboot |
| |
On Mon, Jan 13, 2020 at 8:46 PM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
Thanks for your comments.
> > +config REBOOT_HOTPLUG_CPU > > + bool "Support for hotplug CPUs before reboot" > > + depends on HOTPLUG_CPU > > + help > > + Say Y to do a full hotplug on secondary CPUs before reboot. > > I'm not sure this should be a configurable option, e.g. in case this is > a good approach in general, why not just use CONFIG_HOTPLUG_CPU in the > code? > In v2 it uses CONFIG_HOTPLUG_CPU, but I think adding another config is more flexible. Maybe there are some architecture that supports HOTPLUG_CPU but doesn't want to do full cpu hotplug before reboot. (Eg. doing cpu hotplug would make reboot process slower.) > > + > > config HAVE_OPROFILE > > bool > > > > diff --git a/include/linux/cpu.h b/include/linux/cpu.h > > index 1ca2baf817ed..3bf5ab289954 100644 > > --- a/include/linux/cpu.h > > +++ b/include/linux/cpu.h > > @@ -118,6 +118,9 @@ extern void cpu_hotplug_disable(void); > > extern void cpu_hotplug_enable(void); > > void clear_tasks_mm_cpumask(int cpu); > > int cpu_down(unsigned int cpu); > > +#if IS_ENABLED(CONFIG_REBOOT_HOTPLUG_CPU) > > +extern void offline_secondary_cpus(int primary); > > +#endif > > > > #else /* CONFIG_HOTPLUG_CPU */ > > > > diff --git a/kernel/cpu.c b/kernel/cpu.c > > index 9c706af713fb..52afc47dd56a 100644 > > --- a/kernel/cpu.c > > +++ b/kernel/cpu.c > > @@ -1057,6 +1057,25 @@ int cpu_down(unsigned int cpu) > > } > > EXPORT_SYMBOL(cpu_down); > > > > +#if IS_ENABLED(CONFIG_REBOOT_HOTPLUG_CPU) > > +void offline_secondary_cpus(int primary) > > +{ > > + int i, err; > > + > > + cpu_maps_update_begin(); > > + > > + for_each_online_cpu(i) { > > + if (i == primary) > > + continue; > > + err = _cpu_down(i, 0, CPUHP_OFFLINE); > > + if (err) > > + pr_warn("Failed to offline cpu %d\n", i); > > + } > > + cpu_hotplug_disabled++; > > + > > + cpu_maps_update_done(); > > +} > > +#endif > > This looks like a simplified version of freeze_secondary_cpus(), can > they be merged? > Comparing to freeze_secondary_cpus(), I think it's not necessary to check pm_wakeup_pending() before _cpu_down() here. Thus it doesn't need to depend on CONFIG_PM_SLEEP_SMP. Also I think it could continue to offline other CPUs even one fails, while freeze_secondary_cpus() would stop once it fails on offlining one CPU. Based on these differences, I didn't use freeze_secondary_cpus() here. As for merging the common part, it might need additional flags to handle the difference, which might lower the readability. >
| |