Re: [PATCH] cpu: rid cpu_hotplug_disabled check for cpu_down()

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

On 04/29/2013 08:19 AM, liguang wrote:
> in cpu_down(), _cpu_down() will do
> "
>         if (num_online_cpus() == 1)
>                  return -EBUSY;
> "
> when cpu_hotplug_disabled was set, num_online_cpus
> will return 1 for there's only 1 boot cpu.
> so, it's unnecessary to check cpu_hotplug_disabled
> here.
>

The 2 checks serve very different purposes; they are not the same!

The num_online_cpus() check is to ensure that the user doesn't do
something insane like trying to offline the last online CPU in the
system.

Whereas, the flag 'cpu_hotplug_disabled' is used to prevent user-
triggered CPU hotplug (such as those initiated through sysfs).
This is useful in cases where the system itself wants to initiate CPU
hotplug and it doesn't want annoying races with CPU hotplug going
on in parallel due to other reasons. One such case is suspend/resume.
That's why, if you have noticed, the suspend/resume code invokes the
_cpu_down() version, in order to bypass the flag and get its job done.

So, no, I think the check needs to stay.

Regards,
Srivatsa S. Bhat

> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  kernel/cpu.c |    6 ------
>  1 files changed, 0 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index b5e4ab2..cd166d3 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -330,14 +330,8 @@ int __ref cpu_down(unsigned int cpu)
> 
>  	cpu_maps_update_begin();
> 
> -	if (cpu_hotplug_disabled) {
> -		err = -EBUSY;
> -		goto out;
> -	}
> -
>  	err = _cpu_down(cpu, 0);
> 
> -out:
>  	cpu_maps_update_done();
>  	return err;
>  }
>