Re: [PATCH v9 12/12] x86, topology: Debug CPU00 hotplug

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

On 10/12/2012 09:39 PM, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>

> +config DEBUG_HOTPLUG_CPU0
> +	def_bool n
> +	prompt "Debug CPU0 hotplug"
> +	depends on HOTPLUG_CPU && EXPERIMENTAL
> +	---help---
> +	  Enabling this option offlines CPU0 (if CPU0 can be offlined) as
> +	  soon as possible and boots up userspace with CPU0 offlined. User
> +	  can online CPU0 back after boot time.
> +
> +	  To debug CPU0 hotplug, you need to enable CPU0 offline/online
> +	  feature by either turning on CONFIG_BOOTPARAM_HOTPLUG_CPU0 during
> +	  compilation or giving cpu0_hotplug kernel parameter at boot.
> +
> +	  If unuser, say N.

s/unuser/unsure

> +#ifdef CONFIG_DEBUG_HOTPLUG_CPU0
> +/*
> + * This function offlines a CPU as early as possible and allows userspace to
> + * boot up without the CPU. The CPU can be onlined back by user after boot.
> + *
> + * This is only called for debugging CPU offline/online feature.
> + */
> +int __ref _debug_hotplug_cpu(int cpu, int action)
> +{
> +	struct device *dev = get_cpu_device(cpu);
> +	int ret;
> +
> +	if (!cpu_is_hotpluggable(cpu))
> +		return -EINVAL;
> +
> +	cpu_hotplug_driver_lock();
> +
> +	switch (action) {
> +	case 0:
> +		ret = cpu_down(cpu);
> +		if (!ret) {
> +			pr_info("CPU %u is now offline\n", cpu);
> +			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
> +		} else
> +			pr_debug("Can't offline CPU%d.\n", cpu);
> +		break;
> +	case 1:
> +		ret = cpu_up(cpu);
> +		if (!ret)
> +			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
> +		else
> +			pr_debug("Can't online CPU%d.\n", cpu);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	cpu_hotplug_driver_unlock();
> +
> +	return ret;
> +}
> +
> +static int __init debug_hotplug_cpu(void)
> +{
> +	_debug_hotplug_cpu(0, 0);
> +	return 0;
> +}
> +
> +late_initcall_sync(debug_hotplug_cpu);
> +#endif /* CONFIG_DEBUG_HOTPLUG_CPU0 */
> +
>  int __ref arch_register_cpu(int num)
>  {
>  	struct cpuinfo_x86 *c = &cpu_data(num);
> diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
> index adde775..120cee1 100644
> --- a/arch/x86/power/cpu.c
> +++ b/arch/x86/power/cpu.c
> @@ -21,6 +21,7 @@
>  #include <asm/suspend.h>
>  #include <asm/debugreg.h>
>  #include <asm/fpu-internal.h> /* pcntxt_mask */
> +#include <asm/cpu.h>
> 
>  #ifdef CONFIG_X86_32
>  static struct saved_context saved_context;
> @@ -263,6 +264,43 @@ static int bsp_pm_callback(struct notifier_block *nb, unsigned long action,
>  	case PM_HIBERNATION_PREPARE:
>  		ret = bsp_check();
>  		break;
> +#ifdef CONFIG_DEBUG_HOTPLUG_CPU0
> +	case PM_RESTORE_PREPARE:
> +		/*
> +		 * When system resumes from hibernation, online CPU0 because
> +		 * 1. it's required for resume and
> +		 * 2. the CPU was online before hibernation
> +		 */
> +		if (!cpu_online(0))
> +			_debug_hotplug_cpu(0, 1);

This won't work. CPU hotplug is disabled by cpu_hotplug_pm_callback() during
PM_HIBERNATION_PREPARE and is enabled back only during PM_POST_HIBERNATION.

So calls to cpu_up() or cpu_down() will fail in the meantime.

Regards,
Srivatsa S. Bhat

> +		break;
> +	case PM_POST_RESTORE:
> +		/*
> +		 * When a resume really happens, this code won't be called.
> +		 *
> +		 * This code is called only when user space hibernation software
> +		 * prepares for snapshot device during boot time. So we just
> +		 * call _debug_hotplug_cpu() to restore to CPU0's state prior to
> +		 * preparing the snapshot device.
> +		 *
> +		 * This works for normal boot case in our CPU0 hotplug debug
> +		 * mode, i.e. CPU0 is offline and user mode hibernation
> +		 * software initializes during boot time.
> +		 *
> +		 * If CPU0 is online and user application accesses snapshot
> +		 * device after boot time, this will offline CPU0 and user may
> +		 * see different CPU0 state before and after accessing
> +		 * the snapshot device. But hopefully this is not a case when
> +		 * user debugging CPU0 hotplug. Even if users hit this case,
> +		 * they can easily online CPU0 back.
> +		 *
> +		 * To simplify this debug code, we only consider normal boot
> +		 * case. Otherwise we need to remember CPU0's state and restore
> +		 * to that state and resolve racy conditions etc.
> +		 */
> +		_debug_hotplug_cpu(0, 0);
> +		break;
> +#endif
>  	default:
>  		break;
>  	}
>