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
| ||
|
Date: Tue, 16 Oct 2012 22:12:27 +0000 From: "Yu, Fenghua" <fenghua.yu@...el.com> To: "Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>, "Rafael J. Wysocki" <rjw@...k.pl> CC: Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>, H Peter Anvin <hpa@...or.com>, Linus Torvalds <torvalds@...ux-foundation.org>, Andrew Morton <akpm@...ux-foundation.org>, "Mallick, Asit K" <asit.k.mallick@...el.com>, "Luck, Tony" <tony.luck@...el.com>, Arjan Dan De Ven <arjan@...ux.intel.com>, "Siddha, Suresh B" <suresh.b.siddha@...el.com>, "Brown, Len" <len.brown@...el.com>, Randy Dunlap <rdunlap@...otime.net>, Chen Gong <gong.chen@...ux.intel.com>, linux-kernel <linux-kernel@...r.kernel.org>, linux-pm <linux-pm@...r.kernel.org>, x86 <x86@...nel.org> 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@...el.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
Powered by blists - more mailing lists