Re: [PATCH 0/4] CPU hotplug, cpusets: Fix CPU online handling related to cpusets

["Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>]

On 02/11/2012 09:56 AM, Srivatsa S. Bhat wrote:

> On 02/11/2012 07:37 AM, Peter Zijlstra wrote:
> 
>> On Fri, 2012-02-10 at 23:39 +0100, Rafael J. Wysocki wrote:
>>>
>>>> I don't see why not.  Presumably no CPUs will be added or removed while 
>>>> the system is asleep.
>>>
>>> ACPI explicitly forbids that level of hardware reconfiguration in a sleep
>>> state (even in S4), AFAICS.  Still, people may try to do that ... 
>>
>> I'm ok with breaking that :-) If its really really important to someone
>> we (him most likely) could fix it by detecting the topology changed over
>> the suspend and do a fixup. Assuming it actually gets that far.
>>
>> Srivatsa, wanna give this (the proposal to not modify cpusets on
>> CPU_TASKS_FROZEN) a try?
>>
> 
> 
> Sure! After you pointed out that CPU Hotplug is destructive in general and
> hence it is not a good idea to put back online CPUs to cpusets, the next
> thing I thought of trying was a special case handling for suspend/resume
> alone, IOW, not calling the cpuset update upon CPU_TASKS_FROZEN.
> 
> So, yes, I'll write up a patch for that and post it soon :-)
> 


Trivially removing CPU_TASKS_FROZEN as shown below doesn't look right to me:

---

 kernel/sched/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5255c9d..43a166e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6729,7 +6729,7 @@ int __init sched_create_sysfs_power_savings_entries(struct device *dev)
 static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
 			     void *hcpu)
 {
-	switch (action & ~CPU_TASKS_FROZEN) {
+	switch (action) {
 	case CPU_ONLINE:
 	case CPU_DOWN_FAILED:
 		cpuset_update_active_cpus();
@@ -6742,7 +6742,7 @@ static int cpuset_cpu_active(struct notifier_block *nfb, unsigned long action,
 static int cpuset_cpu_inactive(struct notifier_block *nfb, unsigned long action,
 			       void *hcpu)
 {
-	switch (action & ~CPU_TASKS_FROZEN) {
+	switch (action) {
 	case CPU_DOWN_PREPARE:
 		cpuset_update_active_cpus();
 		return NOTIFY_OK;


IMO, irrespective of whether we keep cpusets unaware of all CPU Hotplug or
only unaware of the CPU hotplug in the suspend/resume path, I feel the
scheduler should always know the true state of the system, ie., offline CPUs
must not be part of any sched domain, at any point in time.

At the moment, I am exploring several ways to achieve this (I can think of 2
ways at the moment, will see which one is better). But in case this approach
itself seems wrong for any reason, please let me know.

Regards,
Srivatsa S. Bhat