Messages in this thread | | | Date | Tue, 16 Oct 2012 22:00:53 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: [PATCH v9 05/12] x86, hotplug, suspend: Online CPU0 for suspend or hibernate |
| |
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.yu@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.
Regards, Srivatsa S. Bhat
>>>> +static int __init bsp_pm_check_init(void) >>>> +{ >>>> + /* >>>> + * Set this bsp_pm_callback as lower priority than >>>> + * cpu_hotplug_pm_callback. So cpu_hotplug_pm_callback will be called >>>> + * earlier to disable cpu hotplug before bsp online check. >>>> + */ >>>> + pm_notifier(bsp_pm_callback, -INT_MAX); >>>> + return 0; >>>> +} >>>> + >>>> +core_initcall(bsp_pm_check_init); >
| |