linux-kernel - Re: [PATCH RFC v3 1/3] reboot: support hotplug CPUs before reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Jan 2020 23:12:09 +0800
From:   Hsin-Yi Wang <hsinyi@...omium.org>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
Cc:     Josh Poimboeuf <jpoimboe@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Jiri Kosina <jkosina@...e.cz>,
        Pavankumar Kondeti <pkondeti@...eaurora.org>,
        Zhenzhong Duan <zhenzhong.duan@...cle.com>,
        Aaro Koskinen <aaro.koskinen@...ia.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Guenter Roeck <groeck@...omium.org>,
        Stephen Boyd <swboyd@...omium.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>
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@...hat.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.
>