> On 10/16/2012 09:47 PM, Rafael J. Wysocki wrote: > > On Tuesday 16 of October 2012 11:05:18 Srivatsa S. Bhat wrote: > >> On 10/16/2012 02:20 AM, Rafael J. Wysocki wrote: > >>> On Friday 12 of October 2012 09:09:42 Fenghua Yu wrote: > >>>> From: Fenghua Yu <fenghua...@intel.com> > >>>> > >>>> + > >>>> +/* > >>>> + * When bsp_check() is called in hibernate and suspend, cpu > hotplug > >>>> + * is disabled already. So it's unnessary to handle race > condition between > >>>> + * cpumask query and cpu hotplug. > >>>> + */ > >>>> +static int bsp_check(void) > >>>> +{ > >>>> + if (cpumask_first(cpu_online_mask) != 0) { > >>>> + pr_warn("CPU0 is offline.\n"); > >>>> + return -ENODEV; > >>>> + } > >>>> + > >>>> + return 0; > >>>> +} > >>>> + > >>>> +static int bsp_pm_callback(struct notifier_block *nb, unsigned > long action, > >>>> + void *ptr) > >>>> +{ > >>>> + int ret = 0; > >>>> + > >>>> + switch (action) { > >>>> + case PM_SUSPEND_PREPARE: > >>>> + case PM_HIBERNATION_PREPARE: > >>>> + ret = bsp_check(); > >>>> + break; > >>>> + default: > >>>> + break; > >>>> + } > >>>> + return notifier_from_errno(ret); > >>>> +} > >>>> + > >>> > >>> I wonder if there's anything preventing CPU0 from becoming offline > after you've > >>> done this check and before user space is frozen? > >>> > >> > >> Hi Rafael, > >> > >> bsp_pm_callback runs as a low priority notifier callback, > specifically with lower > >> priority than the cpu_hotplug_pm_callback (as mentioned in the > comment below). > >> And cpu_hotplug_pm_callback disables regular CPU hotplug (till the > suspend/resume > >> sequence is complete).. So there is no chance for CPU0 to become > offline after that. > >> > >> Or, are you thinking of some other scenario where CPU0 can go > offline? > > > > No, that should be fine technically, but designs relying on notifier > priority > > for correctness are kind of fragile. > > > > Hmm.. I agree. > > > Would it be possible to make cpu_hotplug_pm_callback() do the BSP > online check? > > > > Good idea! I think that could be done quite easily. And while doing > that, it would > be good to rethink what to do in patch 12/12 (Debug CPU0 hotplug) to > fix the bug I > pointed out in my other mail.
Why is this design relying on notifier priority fragile? I don't get it. The priority in pm_notifier() is in a well designed API. This code just follows the API to assign lower priority for bsp_pm_callback than cpu_hotplug_pm_callback. Unless your justification is that the API of pm_notifier() is fragile, I think this patch should be ok. It's not a good idea to move bsp_pm_callback() into cpu_hotplug_pm_callback(). There is no architecture specific hook to call x86 bsp specific hotplug in cpu_hotplug_pm_callback(). Moving bsp_pm_callback() into cpu_hotplug_pm_callback() is hacking while this patch follows well defined API and has better coding. Thanks. -Fenghua N�����r��y����b�X��ǧv�^�){.n�+����{����zX����ܨ}���Ơz�&j:+v�������zZ+��+zf���h���~����i���z��w���?�����&�)ߢf��^jǫy�m��@A�a��� 0��h���i