Re: [PATCH -mm][resend] Disable CPU hotplug during suspend

From: Nathan Lynch
Date: Fri Jul 28 2006 - 18:38:57 EST


Rafael J. Wysocki wrote:
> On Friday 28 July 2006 20:20, Nathan Lynch wrote:
> > > +#ifdef CONFIG_SUSPEND_SMP
> > > +static cpumask_t frozen_cpus;
> > > +
> > > +int disable_nonboot_cpus(void)
> > > +{
> > > + int cpu, error = 0;
> > > +
> > > + /* We take all of the non-boot CPUs down in one shot to avoid races
> > > + * with the userspace trying to use the CPU hotplug at the same time
> > > + */
> > > + mutex_lock(&cpu_add_remove_lock);
> > > + cpus_clear(frozen_cpus);
> > > + printk("Disabling non-boot CPUs ...\n");
> > > + for_each_online_cpu(cpu) {
> > > + if (cpu == 0)
> > > + continue;
> >
> > Assuming cpu 0 is online is not okay in generic code.
>
> Absolutely. Thanks for pointing this out.
>
> > This should be something like:
> >
> > int cpu, first_cpu, error = 0;
> >
> > /* We take all of the non-boot CPUs down in one shot to avoid races
> > * with the userspace trying to use the CPU hotplug at the same time
> > */
> > mutex_lock(&cpu_add_remove_lock);
> > cpus_clear(frozen_cpus);
> > first_cpu = first_cpu(cpu_online_mask);
> > printk("Disabling non-boot CPUs ...\n");
> > for_each_online_cpu(cpu) {
> > if (cpu == first_cpu)
> > continue;
>
>
> I'm not quite sure if we can finish with CPU0 offline. Perhaps it's
> better to check if CPU0 is online and bring it up if not and then
> continue or return an error if that fails?

You can't assume that cpu 0 is even present in generic code. :-)

But maybe I'm misunderstanding the motivation for using cpu 0 here. I
had assumed it was because on i386 (and others?) the BSP can't be
offlined. Is there some other reason?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/