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

[Srivatsa S. Bhat]

Hi Peter,

On 02/20/2012 06:19 PM, Peter Zijlstra wrote:

> On Fri, 2012-02-17 at 17:45 +0530, Srivatsa S. Bhat wrote:
> 
>>> 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.
> 
> That's really not a problem as long as they're not in the active mask.
> 


Ok..

>>> 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.
> 
> 
> Have you actually tried the simple patch? 
> 


Yes, I tried it out and it fixed the suspend/resume bug. I was just unsure that it
was the right fix, because the scheduler's sched domains wouldn't be up-to-date,
unintentionally.

> Calling partition_sched_domains() like you do doesn't seem right, it
> completely ignores cpusets, it will make certain cpuset configurations
> mis-behave.
> 


Well, why do we care about cpuset configurations when we are in the midst of
suspend/resume? Anyway all tasks are frozen during suspend, and when we resume,
we restore everything (including cpusets and sched domains) to exactly how it
was before suspend.. So that must be fine right?

(I am referring to the patch posted at https://lkml.org/lkml/2012/2/17/125).

Regards,
Srivatsa S. Bhat

--
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/