* Make ACPI physical CPU hotadd cpuidle robust
@ 2011-09-11 23:42 Thomas Renninger
2011-09-11 23:42 ` [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal Thomas Renninger
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Thomas Renninger @ 2011-09-11 23:42 UTC (permalink / raw)
To: lenb, linux-acpi
The patches are against (because I had problems with kernel.org):
git clone git://kernel.opensuse.org/kernel.git
which should be equal to a latest 3.2-rc5 kernel and these parts
should not have been changed by SUSE commits.
Would be great to see them queued in the acpi test branch for 3.2
inclusion.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal 2011-09-11 23:42 Make ACPI physical CPU hotadd cpuidle robust Thomas Renninger @ 2011-09-11 23:42 ` Thomas Renninger 2011-09-12 17:14 ` Bjorn Helgaas 2011-09-11 23:42 ` [PATCH 2/5] intel_idle: Split up and provide per CPU initialization func Thomas Renninger ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Thomas Renninger @ 2011-09-11 23:42 UTC (permalink / raw) To: lenb, linux-acpi; +Cc: Thomas Renninger Only unregister acpi_idle driver if acpi_idle driver got registered. On latest Nehalem/Sandybridge this is not the case, but the intel_idle driver is used instead. Signed-off-by: Thomas Renninger <trenn@suse.de> CC: Len Brown <lenb@kernel.org> CC: linux-acpi@vger.kernel.org --- drivers/acpi/processor_driver.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index a4e0f1b..9247e67 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -823,7 +823,8 @@ static int __init acpi_processor_init(void) return 0; out_cpuidle: - cpuidle_unregister_driver(&acpi_idle_driver); + if (cpuidle_get_driver() == &acpi_idle_driver) + cpuidle_unregister_driver(&acpi_idle_driver); return result; } @@ -841,7 +842,8 @@ static void __exit acpi_processor_exit(void) acpi_bus_unregister_driver(&acpi_processor_driver); - cpuidle_unregister_driver(&acpi_idle_driver); + if (cpuidle_get_driver() == &acpi_idle_driver) + cpuidle_unregister_driver(&acpi_idle_driver); return; } -- 1.7.6.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal 2011-09-11 23:42 ` [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal Thomas Renninger @ 2011-09-12 17:14 ` Bjorn Helgaas 0 siblings, 0 replies; 10+ messages in thread From: Bjorn Helgaas @ 2011-09-12 17:14 UTC (permalink / raw) To: Thomas Renninger; +Cc: lenb, linux-acpi On Sun, Sep 11, 2011 at 4:42 PM, Thomas Renninger <trenn@suse.de> wrote: > Only unregister acpi_idle driver if acpi_idle driver got > registered. On latest Nehalem/Sandybridge this is not the case, > but the intel_idle driver is used instead. > > Signed-off-by: Thomas Renninger <trenn@suse.de> > CC: Len Brown <lenb@kernel.org> > CC: linux-acpi@vger.kernel.org > --- > drivers/acpi/processor_driver.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c > index a4e0f1b..9247e67 100644 > --- a/drivers/acpi/processor_driver.c > +++ b/drivers/acpi/processor_driver.c > @@ -823,7 +823,8 @@ static int __init acpi_processor_init(void) > return 0; > > out_cpuidle: > - cpuidle_unregister_driver(&acpi_idle_driver); > + if (cpuidle_get_driver() == &acpi_idle_driver) > + cpuidle_unregister_driver(&acpi_idle_driver); I think this is ugly. I know we already have a similar check for acpi_idle_driver in acpi_processor_add(), and I think that's ugly, too :) The standard register/unregister_driver() model does not require looking at what's currently registered, and I don't think this should be any different. I don't know the mechanism for choosing either acpi_idle_driver or intel_idle_driver, but I think you can accomplish what you want here by just adding a static "acpi_idle_registered" variable to remember whether the cpuidle_register_driver() was successful, and only do the unregister if it was. > > return result; > } > @@ -841,7 +842,8 @@ static void __exit acpi_processor_exit(void) > > acpi_bus_unregister_driver(&acpi_processor_driver); > > - cpuidle_unregister_driver(&acpi_idle_driver); > + if (cpuidle_get_driver() == &acpi_idle_driver) > + cpuidle_unregister_driver(&acpi_idle_driver); > > return; > } > -- > 1.7.6.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/5] intel_idle: Split up and provide per CPU initialization func 2011-09-11 23:42 Make ACPI physical CPU hotadd cpuidle robust Thomas Renninger 2011-09-11 23:42 ` [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal Thomas Renninger @ 2011-09-11 23:42 ` Thomas Renninger 2011-09-11 23:42 ` [PATCH 3/5] ACPI processor: Fix error path, also remove sysdev link Thomas Renninger ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Thomas Renninger @ 2011-09-11 23:42 UTC (permalink / raw) To: lenb, linux-acpi; +Cc: Thomas Renninger Function split up, should have no functional change Signed-off-by: Thomas Renninger <trenn@suse.de> CC: Len Brown <lenb@kernel.org> CC: linux-acpi@vger.kernel.org --- drivers/idle/intel_idle.c | 108 +++++++++++++++++++++----------------------- include/linux/cpuidle.h | 7 +++ 2 files changed, 59 insertions(+), 56 deletions(-) diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 5b0e456..1c0d132 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -398,77 +398,67 @@ static void intel_idle_cpuidle_devices_uninit(void) return; } /* - * intel_idle_cpuidle_devices_init() + * intel_idle_cpu_init() * allocate, initialize, register cpuidle_devices + * @cpu: cpu/core to initialize */ -static int intel_idle_cpuidle_devices_init(void) +int intel_idle_cpu_init(int cpu) { - int i, cstate; + int cstate; struct cpuidle_device *dev; - intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device); - if (intel_idle_cpuidle_devices == NULL) - return -ENOMEM; + dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu); - for_each_online_cpu(i) { - dev = per_cpu_ptr(intel_idle_cpuidle_devices, i); + dev->state_count = 1; - dev->state_count = 1; - - for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) { - int num_substates; - - if (cstate > max_cstate) { - printk(PREFIX "max_cstate %d reached\n", - max_cstate); - break; - } - - /* does the state exist in CPUID.MWAIT? */ - num_substates = (mwait_substates >> ((cstate) * 4)) - & MWAIT_SUBSTATE_MASK; - if (num_substates == 0) - continue; - /* is the state not enabled? */ - if (cpuidle_state_table[cstate].enter == NULL) { - /* does the driver not know about the state? */ - if (*cpuidle_state_table[cstate].name == '\0') - pr_debug(PREFIX "unaware of model 0x%x" - " MWAIT %d please" - " contact lenb@kernel.org", - boot_cpu_data.x86_model, cstate); - continue; - } - - if ((cstate > 2) && - !boot_cpu_has(X86_FEATURE_NONSTOP_TSC)) - mark_tsc_unstable("TSC halts in idle" - " states deeper than C2"); - - dev->states[dev->state_count] = /* structure copy */ - cpuidle_state_table[cstate]; - - dev->state_count += 1; + for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) { + int num_substates; + + if (cstate > max_cstate) { + printk(PREFIX "max_cstate %d reached\n", max_cstate); + break; } - dev->cpu = i; - if (cpuidle_register_device(dev)) { - pr_debug(PREFIX "cpuidle_register_device %d failed!\n", - i); - intel_idle_cpuidle_devices_uninit(); - return -EIO; + /* does the state exist in CPUID.MWAIT? */ + num_substates = (mwait_substates >> ((cstate) * 4)) + & MWAIT_SUBSTATE_MASK; + if (num_substates == 0) + continue; + /* is the state not enabled? */ + if (cpuidle_state_table[cstate].enter == NULL) { + /* does the driver not know about the state? */ + if (*cpuidle_state_table[cstate].name == '\0') + pr_debug(PREFIX "unaware of model 0x%x MWAIT " + "%d please contact lenb@kernel.org", + boot_cpu_data.x86_model, cstate); + continue; } + + if ((cstate > 2) && !boot_cpu_has(X86_FEATURE_NONSTOP_TSC)) + mark_tsc_unstable("TSC halts in idle" + " states deeper than C2"); + + dev->states[dev->state_count] = /* structure copy */ + cpuidle_state_table[cstate]; + dev->state_count += 1; + } + dev->cpu = cpu; + if (cpuidle_register_device(dev)) { + pr_debug(PREFIX "cpuidle_register_device %d failed!\n", cpu); + intel_idle_cpuidle_devices_uninit(); + return -EIO; } if (auto_demotion_disable_flags) - smp_call_function(auto_demotion_disable, NULL, 1); + smp_call_function_single(cpu, auto_demotion_disable, NULL, 1); return 0; } +EXPORT_SYMBOL(intel_idle_cpu_init); static int __init intel_idle_init(void) { - int retval; + int retval, i; /* Do not load intel_idle at all for now if idle= is passed */ if (boot_option_idle_override != IDLE_NO_OVERRIDE) @@ -485,10 +475,16 @@ static int __init intel_idle_init(void) return retval; } - retval = intel_idle_cpuidle_devices_init(); - if (retval) { - cpuidle_unregister_driver(&intel_idle_driver); - return retval; + intel_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device); + if (intel_idle_cpuidle_devices == NULL) + return -ENOMEM; + + for_each_online_cpu(i) { + retval = intel_idle_cpu_init(i); + if (retval) { + cpuidle_unregister_driver(&intel_idle_driver); + return retval; + } } return 0; diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index b51629e..15bd1bd 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -179,7 +179,14 @@ struct cpuidle_governor { extern int cpuidle_register_governor(struct cpuidle_governor *gov); extern void cpuidle_unregister_governor(struct cpuidle_governor *gov); +#ifdef CONFIG_INTEL_IDLE +extern int intel_idle_cpu_init(int cpu); #else +static inline int intel_idle_cpu_init(int cpu) { return -1; } +#endif + +#else +static inline int intel_idle_cpu_init(int cpu) { return -1; } static inline int cpuidle_register_governor(struct cpuidle_governor *gov) {return 0;} -- 1.7.6.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] ACPI processor: Fix error path, also remove sysdev link 2011-09-11 23:42 Make ACPI physical CPU hotadd cpuidle robust Thomas Renninger 2011-09-11 23:42 ` [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal Thomas Renninger 2011-09-11 23:42 ` [PATCH 2/5] intel_idle: Split up and provide per CPU initialization func Thomas Renninger @ 2011-09-11 23:42 ` Thomas Renninger 2011-09-12 17:46 ` Bjorn Helgaas 2011-09-11 23:42 ` [PATCH 4/5] ACPI processor: Split up acpi_processor_add() Thomas Renninger 2011-09-11 23:42 ` [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online Thomas Renninger 4 siblings, 1 reply; 10+ messages in thread From: Thomas Renninger @ 2011-09-11 23:42 UTC (permalink / raw) To: lenb, linux-acpi; +Cc: Thomas Renninger Signed-off-by: Thomas Renninger <trenn@suse.de> CC: Len Brown <lenb@kernel.org> CC: linux-acpi@vger.kernel.org --- drivers/acpi/processor_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index 9247e67..7bc141a 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -540,6 +540,7 @@ err_thermal_unregister: thermal_cooling_device_unregister(pr->cdev); err_power_exit: acpi_processor_power_exit(pr, device); + sysfs_remove_link(&device->dev.kobj, "sysdev"); err_free_cpumask: free_cpumask_var(pr->throttling.shared_cpu_map); -- 1.7.6.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] ACPI processor: Fix error path, also remove sysdev link 2011-09-11 23:42 ` [PATCH 3/5] ACPI processor: Fix error path, also remove sysdev link Thomas Renninger @ 2011-09-12 17:46 ` Bjorn Helgaas 0 siblings, 0 replies; 10+ messages in thread From: Bjorn Helgaas @ 2011-09-12 17:46 UTC (permalink / raw) To: Thomas Renninger; +Cc: lenb, linux-acpi On Sun, Sep 11, 2011 at 4:42 PM, Thomas Renninger <trenn@suse.de> wrote: > Signed-off-by: Thomas Renninger <trenn@suse.de> > CC: Len Brown <lenb@kernel.org> > CC: linux-acpi@vger.kernel.org > --- > drivers/acpi/processor_driver.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c > index 9247e67..7bc141a 100644 > --- a/drivers/acpi/processor_driver.c > +++ b/drivers/acpi/processor_driver.c > @@ -540,6 +540,7 @@ err_thermal_unregister: > thermal_cooling_device_unregister(pr->cdev); > err_power_exit: > acpi_processor_power_exit(pr, device); > + sysfs_remove_link(&device->dev.kobj, "sysdev"); The sysfs_remove_link("sysdev") looks right to me. acpi_processor_power_exit() looks like it is undoing whatever acpi_processor_power_init() did earlier. But we only called _power_init() if we're using acpi_idle_driver, and we call _power_exit() always. That seems wrong. It seems like you only want to call _power_exit() if you successfully called _power_init(). Oh, and _power_init() can return failure, but we didn't check for it. Seems like we ought to do *something*. While you're looking at it, I'd say the "if (!pr) return -EINVAL" in _power_init() should just be removed. It's impossible to call with pr==NULL, and even if we could, I'd rather take the null pointer oops so we could find the problem. I know these issues aren't your fault, but maybe you could fix them since you're in the area? Bjorn > err_free_cpumask: > free_cpumask_var(pr->throttling.shared_cpu_map); > > -- > 1.7.6.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] ACPI processor: Split up acpi_processor_add() 2011-09-11 23:42 Make ACPI physical CPU hotadd cpuidle robust Thomas Renninger ` (2 preceding siblings ...) 2011-09-11 23:42 ` [PATCH 3/5] ACPI processor: Fix error path, also remove sysdev link Thomas Renninger @ 2011-09-11 23:42 ` Thomas Renninger 2011-09-11 23:42 ` [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online Thomas Renninger 4 siblings, 0 replies; 10+ messages in thread From: Thomas Renninger @ 2011-09-11 23:42 UTC (permalink / raw) To: lenb, linux-acpi; +Cc: Thomas Renninger Should have no functional change, beside a tiny fix in error case: also remove a previously created link. Signed-off-by: Thomas Renninger <trenn@suse.de> CC: Len Brown <lenb@kernel.org> CC: linux-acpi@vger.kernel.org --- drivers/acpi/processor_driver.c | 93 ++++++++++++++++++++++---------------- 1 files changed, 54 insertions(+), 39 deletions(-) diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index 7bc141a..546951a 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -442,6 +442,58 @@ static struct notifier_block acpi_cpu_notifier = .notifier_call = acpi_cpu_soft_notify, }; +static int __cpuinit acpi_processor_start(struct acpi_processor *pr) +{ + struct acpi_device *device = per_cpu(processor_device_array, pr->id); + int result = 0; + +#ifdef CONFIG_CPU_FREQ + acpi_processor_ppc_has_changed(pr, 0); +#endif + acpi_processor_get_throttling_info(pr); + acpi_processor_get_limit_info(pr); + + + if (cpuidle_get_driver() == &acpi_idle_driver) + acpi_processor_power_init(pr, device); + + pr->cdev = thermal_cooling_device_register("Processor", device, + &processor_cooling_ops); + if (IS_ERR(pr->cdev)) { + result = PTR_ERR(pr->cdev); + goto err_power_exit; + } + + dev_dbg(&device->dev, "registered as cooling_device%d\n", + pr->cdev->id); + + result = sysfs_create_link(&device->dev.kobj, + &pr->cdev->device.kobj, + "thermal_cooling"); + if (result) { + printk(KERN_ERR PREFIX "Create sysfs link\n"); + goto err_thermal_unregister; + } + result = sysfs_create_link(&pr->cdev->device.kobj, + &device->dev.kobj, + "device"); + if (result) { + printk(KERN_ERR PREFIX "Create sysfs link\n"); + goto err_remove_sysfs_thermal; + } + return 0; + +err_remove_sysfs_thermal: + sysfs_remove_link(&device->dev.kobj, "thermal_cooling"); +err_thermal_unregister: + thermal_cooling_device_unregister(pr->cdev); +err_power_exit: + acpi_processor_power_exit(pr, device); + + return result; +} + + static int __cpuinit acpi_processor_add(struct acpi_device *device) { struct acpi_processor *pr = NULL; @@ -496,50 +548,13 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device) result = -EFAULT; goto err_free_cpumask; } - -#ifdef CONFIG_CPU_FREQ - acpi_processor_ppc_has_changed(pr, 0); -#endif - acpi_processor_get_throttling_info(pr); - acpi_processor_get_limit_info(pr); - - - if (cpuidle_get_driver() == &acpi_idle_driver) - acpi_processor_power_init(pr, device); - - pr->cdev = thermal_cooling_device_register("Processor", device, - &processor_cooling_ops); - if (IS_ERR(pr->cdev)) { - result = PTR_ERR(pr->cdev); - goto err_power_exit; - } - - dev_dbg(&device->dev, "registered as cooling_device%d\n", - pr->cdev->id); - - result = sysfs_create_link(&device->dev.kobj, - &pr->cdev->device.kobj, - "thermal_cooling"); - if (result) { - printk(KERN_ERR PREFIX "Create sysfs link\n"); - goto err_thermal_unregister; - } - result = sysfs_create_link(&pr->cdev->device.kobj, - &device->dev.kobj, - "device"); - if (result) { - printk(KERN_ERR PREFIX "Create sysfs link\n"); + result = acpi_processor_start(pr); + if (result) goto err_remove_sysfs; - } return 0; err_remove_sysfs: - sysfs_remove_link(&device->dev.kobj, "thermal_cooling"); -err_thermal_unregister: - thermal_cooling_device_unregister(pr->cdev); -err_power_exit: - acpi_processor_power_exit(pr, device); sysfs_remove_link(&device->dev.kobj, "sysdev"); err_free_cpumask: free_cpumask_var(pr->throttling.shared_cpu_map); -- 1.7.6.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online 2011-09-11 23:42 Make ACPI physical CPU hotadd cpuidle robust Thomas Renninger ` (3 preceding siblings ...) 2011-09-11 23:42 ` [PATCH 4/5] ACPI processor: Split up acpi_processor_add() Thomas Renninger @ 2011-09-11 23:42 ` Thomas Renninger 2011-09-12 23:16 ` Bjorn Helgaas 4 siblings, 1 reply; 10+ messages in thread From: Thomas Renninger @ 2011-09-11 23:42 UTC (permalink / raw) To: lenb, linux-acpi; +Cc: Thomas Renninger When the CPU hotplug notification comes in via ACPI notify() cpu_data of the core is not yet initialized (the core did never get booted and set up). This leads to errors in throttling and (if acpi_idle is used as cpuidle driver) to not set up cpu idle subsystem. -> Only bring up ACPI sysfs at that time -> Initialize the rest (throttling, cpuidle,...) when a hotplugged core is onlined the first time Sidenote: When starting on this, Xen patches were applied on the code base I worked on. I later realized that changing: static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu); to only passing the processor object: static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr); came from Xen patches, but is convenient to have for physical cpu hotplugging and took that over. Signed-off-by: Thomas Renninger <trenn@suse.de> CC: Len Brown <lenb@kernel.org> CC: linux-acpi@vger.kernel.org --- drivers/acpi/processor_driver.c | 75 +++++++++++++++++++++++++++++--------- include/acpi/processor.h | 1 + 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index 546951a..16f26b8 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -82,9 +82,9 @@ MODULE_LICENSE("GPL"); static int acpi_processor_add(struct acpi_device *device); static int acpi_processor_remove(struct acpi_device *device, int type); static void acpi_processor_notify(struct acpi_device *device, u32 event); -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu); +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr); static int acpi_processor_handle_eject(struct acpi_processor *pr); - +static int acpi_processor_start(struct acpi_processor *pr); static const struct acpi_device_id processor_device_ids[] = { {ACPI_PROCESSOR_OBJECT_HID, 0}, @@ -324,10 +324,8 @@ static int acpi_processor_get_info(struct acpi_device *device) * they are physically not present. */ if (pr->id == -1) { - if (ACPI_FAILURE - (acpi_processor_hotadd_init(pr->handle, &pr->id))) { + if (ACPI_FAILURE(acpi_processor_hotadd_init(pr))) return -ENODEV; - } } /* * On some boxes several processors use the same processor bus id. @@ -425,10 +423,28 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb, struct acpi_processor *pr = per_cpu(processors, cpu); if (action == CPU_ONLINE && pr) { - acpi_processor_ppc_has_changed(pr, 0); - acpi_processor_cst_has_changed(pr); - acpi_processor_reevaluate_tstate(pr, action); - acpi_processor_tstate_has_changed(pr); + /* CPU got hotplugged and onlined the first time + * Initialize missing things + */ + if (pr->flags.need_hotplug_init) { + struct cpuidle_driver *idle_driver = + cpuidle_get_driver(); + + printk(KERN_INFO "Will online and init hotplugged " + "CPU: %d\n", pr->id); + WARN(acpi_processor_start(pr), "Failed to start CPU:" + " %d\n", pr->id); + pr->flags.need_hotplug_init = 0; + if (idle_driver && !strcmp(idle_driver->name, + "intel_idle")) { + intel_idle_cpu_init(pr->id); + } + } else { + acpi_processor_ppc_has_changed(pr, 0); + acpi_processor_cst_has_changed(pr); + acpi_processor_reevaluate_tstate(pr, action); + acpi_processor_tstate_has_changed(pr); + } } if (action == CPU_DEAD && pr) { /* invalidate the flag.throttling after one CPU is offline */ @@ -442,7 +458,15 @@ static struct notifier_block acpi_cpu_notifier = .notifier_call = acpi_cpu_soft_notify, }; -static int __cpuinit acpi_processor_start(struct acpi_processor *pr) +/* + * acpi_processor_start() is called by the cpu_hotplug_notifier func: + * acpi_cpu_soft_notify(). Getting it __cpuinit{data} is difficult, the + * root cause seem to be that acpi_processor_uninstall_hotplug_notify() + * is in the module_exit (__exit) func. Allowing acpi_processor_start() + * to not be in __cpuinit section, but being called from __cpuinit funcs + * via __ref looks like the right thing to do here. + */ +static __ref int acpi_processor_start(struct acpi_processor *pr) { struct acpi_device *device = per_cpu(processor_device_array, pr->id); int result = 0; @@ -548,6 +572,13 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device) result = -EFAULT; goto err_free_cpumask; } + /* + * Do not start hotplugged CPUs now, but when they + * are onlined the first time + */ + if (pr->flags.need_hotplug_init) + return 0; + result = acpi_processor_start(pr); if (result) goto err_remove_sysfs; @@ -737,20 +768,28 @@ processor_walk_namespace_cb(acpi_handle handle, return (AE_OK); } -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu) +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr) { - - if (!is_processor_present(handle)) { + if (!is_processor_present(pr->handle)) return AE_ERROR; - } - if (acpi_map_lsapic(handle, p_cpu)) + if (acpi_map_lsapic(pr->handle, &pr->id)) return AE_ERROR; - if (arch_register_cpu(*p_cpu)) { - acpi_unmap_lsapic(*p_cpu); + if (arch_register_cpu(pr->id)) { + acpi_unmap_lsapic(pr->id); return AE_ERROR; } + /* CPU got hot-plugged, but cpu_data is not initialized yet + * Set flag to delay cpu_idle/throttling initialization + * in: + * acpi_processor_add() + * acpi_processor_get_info() + * and do it when the CPU gets online the first time + * TBD: Cleanup above functions and try to do this more elegant. + */ + printk(KERN_INFO "CPU %d got hotplugged\n", pr->id); + pr->flags.need_hotplug_init = 1; return AE_OK; } @@ -765,7 +804,7 @@ static int acpi_processor_handle_eject(struct acpi_processor *pr) return (0); } #else -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu) +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr) { return AE_ERROR; } diff --git a/include/acpi/processor.h b/include/acpi/processor.h index 67055f1..2a2d3ab 100644 --- a/include/acpi/processor.h +++ b/include/acpi/processor.h @@ -195,6 +195,7 @@ struct acpi_processor_flags { u8 has_cst:1; u8 power_setup_done:1; u8 bm_rld_set:1; + u8 need_hotplug_init:1; }; struct acpi_processor { -- 1.7.6.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online 2011-09-11 23:42 ` [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online Thomas Renninger @ 2011-09-12 23:16 ` Bjorn Helgaas 2011-09-13 12:40 ` Thomas Renninger 0 siblings, 1 reply; 10+ messages in thread From: Bjorn Helgaas @ 2011-09-12 23:16 UTC (permalink / raw) To: Thomas Renninger; +Cc: lenb, linux-acpi On Sun, Sep 11, 2011 at 4:42 PM, Thomas Renninger <trenn@suse.de> wrote: > When the CPU hotplug notification comes in via ACPI notify() > cpu_data of the core is not yet initialized (the core did never > get booted and set up). I guess that when we handle the ACPI CPU add notification, we don't automatically put the CPU online. When *does* the CPU become online? Does the user have to do something separate? Is there a udev rule or something that puts it online? Why don't we automatically online it? If we hot-add a PCI device, I don't think there's a separate step to make it online, so I'm curious about why CPUs should be different. I know I'm asking naive questions; I just don't know the expected flow here. The start() function was originally separate, and I combined it with add() in 970b04929a6 because I think that will make it easier to move the hotplug handling out of drivers and into the ACPI core. ACPI is unusual in that the driver ops have both .add() and .start() ops, and in general I don't think they are both strictly necessary. These current patches reintroduce a start() function, but they do not reintroduce use of the .start() driver entry point, so I'm not worried about that. I'm just trying to understand the difference between the hot-add and online operations. > This leads to errors in throttling and (if acpi_idle is used > as cpuidle driver) to not set up cpu idle subsystem. > -> Only bring up ACPI sysfs at that time > -> Initialize the rest (throttling, cpuidle,...) when a hotplugged > core is onlined the first time > > Sidenote: > When starting on this, Xen patches were applied on the code base > I worked on. I later realized that changing: > static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu); > to only passing the processor object: > static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr); > came from Xen patches, but is convenient to have for physical cpu hotplugging > and took that over. I think it'd be nice to have this acpi_processor_hotadd_init() interface change as a separate patch, especially since it overlaps a Xen change. > Signed-off-by: Thomas Renninger <trenn@suse.de> > CC: Len Brown <lenb@kernel.org> > CC: linux-acpi@vger.kernel.org > --- > drivers/acpi/processor_driver.c | 75 +++++++++++++++++++++++++++++--------- > include/acpi/processor.h | 1 + > 2 files changed, 58 insertions(+), 18 deletions(-) > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c > index 546951a..16f26b8 100644 > --- a/drivers/acpi/processor_driver.c > +++ b/drivers/acpi/processor_driver.c > @@ -82,9 +82,9 @@ MODULE_LICENSE("GPL"); > static int acpi_processor_add(struct acpi_device *device); > static int acpi_processor_remove(struct acpi_device *device, int type); > static void acpi_processor_notify(struct acpi_device *device, u32 event); > -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu); > +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr); > static int acpi_processor_handle_eject(struct acpi_processor *pr); > - > +static int acpi_processor_start(struct acpi_processor *pr); > > static const struct acpi_device_id processor_device_ids[] = { > {ACPI_PROCESSOR_OBJECT_HID, 0}, > @@ -324,10 +324,8 @@ static int acpi_processor_get_info(struct acpi_device *device) > * they are physically not present. > */ > if (pr->id == -1) { > - if (ACPI_FAILURE > - (acpi_processor_hotadd_init(pr->handle, &pr->id))) { > + if (ACPI_FAILURE(acpi_processor_hotadd_init(pr))) > return -ENODEV; > - } > } > /* > * On some boxes several processors use the same processor bus id. > @@ -425,10 +423,28 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb, > struct acpi_processor *pr = per_cpu(processors, cpu); > > if (action == CPU_ONLINE && pr) { > - acpi_processor_ppc_has_changed(pr, 0); > - acpi_processor_cst_has_changed(pr); > - acpi_processor_reevaluate_tstate(pr, action); > - acpi_processor_tstate_has_changed(pr); I'm trying to figure out when this stuff (acpi_processor_ppc_has changed(), _cst_has changed(), etc) now happens. It used to happen when a hot-added CPU came online. Now it happens when a hot-added CPU comes online *and* need_hotplug_init == 0. But need_hotplug_init is set to 1 in the acpi_processor_add() -> acpi_processor_get_info() -> acpi_processor_hotadd_init() path. And I don't understand this weird interaction of "idle driver is intel_idle" with all of this. If acpi_processor_ppc_has_changed(), et al. are mutually exclusive with intel_idle, maybe we need a new idle driver entry point or something. As it is, this is kind of a mess. > + /* CPU got hotplugged and onlined the first time > + * Initialize missing things > + */ > + if (pr->flags.need_hotplug_init) { > + struct cpuidle_driver *idle_driver = > + cpuidle_get_driver(); > + > + printk(KERN_INFO "Will online and init hotplugged " > + "CPU: %d\n", pr->id); > + WARN(acpi_processor_start(pr), "Failed to start CPU:" > + " %d\n", pr->id); Maybe I'm the only one, but I think the acpi_processor_start() call is easy to miss when it's inside the WARN(). I'd rather see something like: ret = acpi_processor_start(pr); if (ret) dev_warn(...); > + pr->flags.need_hotplug_init = 0; > + if (idle_driver && !strcmp(idle_driver->name, > + "intel_idle")) { > + intel_idle_cpu_init(pr->id); Checking the driver name here seems pretty ugly, and I think intel_idle_cpu_init() is conditionally compiled, so it looks like this will break if CONFIG_INTEL_IDLE=n. > + } > + } else { > + acpi_processor_ppc_has_changed(pr, 0); > + acpi_processor_cst_has_changed(pr); > + acpi_processor_reevaluate_tstate(pr, action); > + acpi_processor_tstate_has_changed(pr); > + } > } > if (action == CPU_DEAD && pr) { > /* invalidate the flag.throttling after one CPU is offline */ > @@ -442,7 +458,15 @@ static struct notifier_block acpi_cpu_notifier = > .notifier_call = acpi_cpu_soft_notify, > }; > > -static int __cpuinit acpi_processor_start(struct acpi_processor *pr) > +/* > + * acpi_processor_start() is called by the cpu_hotplug_notifier func: > + * acpi_cpu_soft_notify(). Getting it __cpuinit{data} is difficult, the > + * root cause seem to be that acpi_processor_uninstall_hotplug_notify() > + * is in the module_exit (__exit) func. Allowing acpi_processor_start() > + * to not be in __cpuinit section, but being called from __cpuinit funcs > + * via __ref looks like the right thing to do here. > + */ > +static __ref int acpi_processor_start(struct acpi_processor *pr) This comment seems hand-wavey, and hence is a red flag :) As I understand it, __ref is for use when non-init code references init data. What initdata does acpi_processor_start() reference, and how do we know that's always safe? (Maybe you just need to remove the __cpuinit/__ref annotation completely?) > { > struct acpi_device *device = per_cpu(processor_device_array, pr->id); > int result = 0; > @@ -548,6 +572,13 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device) > result = -EFAULT; > goto err_free_cpumask; > } > + /* > + * Do not start hotplugged CPUs now, but when they > + * are onlined the first time > + */ > + if (pr->flags.need_hotplug_init) > + return 0; > + > result = acpi_processor_start(pr); > if (result) > goto err_remove_sysfs; > @@ -737,20 +768,28 @@ processor_walk_namespace_cb(acpi_handle handle, > return (AE_OK); > } > > -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu) > +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr) > { > - > - if (!is_processor_present(handle)) { > + if (!is_processor_present(pr->handle)) > return AE_ERROR; > - } > > - if (acpi_map_lsapic(handle, p_cpu)) > + if (acpi_map_lsapic(pr->handle, &pr->id)) > return AE_ERROR; > > - if (arch_register_cpu(*p_cpu)) { > - acpi_unmap_lsapic(*p_cpu); > + if (arch_register_cpu(pr->id)) { > + acpi_unmap_lsapic(pr->id); > return AE_ERROR; > } > + /* CPU got hot-plugged, but cpu_data is not initialized yet > + * Set flag to delay cpu_idle/throttling initialization > + * in: > + * acpi_processor_add() > + * acpi_processor_get_info() > + * and do it when the CPU gets online the first time > + * TBD: Cleanup above functions and try to do this more elegant. > + */ > + printk(KERN_INFO "CPU %d got hotplugged\n", pr->id); > + pr->flags.need_hotplug_init = 1; > > return AE_OK; > } > @@ -765,7 +804,7 @@ static int acpi_processor_handle_eject(struct acpi_processor *pr) > return (0); > } > #else > -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu) > +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr) > { > return AE_ERROR; > } > diff --git a/include/acpi/processor.h b/include/acpi/processor.h > index 67055f1..2a2d3ab 100644 > --- a/include/acpi/processor.h > +++ b/include/acpi/processor.h > @@ -195,6 +195,7 @@ struct acpi_processor_flags { > u8 has_cst:1; > u8 power_setup_done:1; > u8 bm_rld_set:1; > + u8 need_hotplug_init:1; > }; > > struct acpi_processor { > -- > 1.7.6.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online 2011-09-12 23:16 ` Bjorn Helgaas @ 2011-09-13 12:40 ` Thomas Renninger 0 siblings, 0 replies; 10+ messages in thread From: Thomas Renninger @ 2011-09-13 12:40 UTC (permalink / raw) To: Bjorn Helgaas; +Cc: lenb, linux-acpi On Tuesday 13 September 2011 01:16:27 Bjorn Helgaas wrote: > On Sun, Sep 11, 2011 at 4:42 PM, Thomas Renninger <trenn@suse.de> wrote: > > When the CPU hotplug notification comes in via ACPI notify() > > cpu_data of the core is not yet initialized (the core did never > > get booted and set up). > > I guess that when we handle the ACPI CPU add notification, we don't > automatically put the CPU online. When *does* the CPU become online? echo 1 >/sys/devices/system/cpu/cpu[0-9]*/online arch_register_cpu(*p_cpu) only creates above sysfs entry. The CPU is booted and set up when it gets online the first time. > Does the user have to do something separate? Is there a udev rule or > something that puts it online? Yes, I have a rule working, but there still seem to be a lock/race issue , maybe related to microcode loading if the CPUs are onlined immediately. I currently see about 1 minute delays between each core gets onlined (only in udev "immediately online" case). > Why don't we automatically online it? > If we hot-add a PCI device, I don't think there's a separate step to > make it online, so I'm curious about why CPUs should be different. Looks like a systemwide (not x86) design for CPUs and memory as well. Hm. One solution could be to add: arch_prepare_cpu() to the generic kernel layer which boots up and initializes the CPU in x86 case , but still keep it (soft) offline. Some code how this could be done pasted in the end. Not sure whether the critical part of booting the core should get directly connected to the hotplug event. Makes it at least more difficult to debug if things go wrong. > I know I'm asking naive questions; I just don't know the expected flow > here. Sure. It cost me some time to realize that the hotadd event (and the acpi bus register) call could come from the container driver as well. I first wanted to do it differently. > The start() function was originally separate, and I combined it > with add() in 970b04929a6 because I think that will make it easier to > move the hotplug handling out of drivers and into the ACPI core. ACPI > is unusual in that the driver ops have both .add() and .start() ops, > and in general I don't think they are both strictly necessary. These > current patches reintroduce a start() function, but they do not > reintroduce use of the .start() driver entry point, so I'm not worried > about that. I'm just trying to understand the difference between the > hot-add and online operations. > > > This leads to errors in throttling and (if acpi_idle is used > > as cpuidle driver) to not set up cpu idle subsystem. > > -> Only bring up ACPI sysfs at that time > > -> Initialize the rest (throttling, cpuidle,...) when a hotplugged > > core is onlined the first time > > > > Sidenote: > > When starting on this, Xen patches were applied on the code base > > I worked on. I later realized that changing: > > static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu); > > to only passing the processor object: > > static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr); > > came from Xen patches, but is convenient to have for physical cpu hotplugging > > and took that over. > > I think it'd be nice to have this acpi_processor_hotadd_init() > interface change as a separate patch, especially since it overlaps a > Xen change. Makes sense. > > Signed-off-by: Thomas Renninger <trenn@suse.de> > > CC: Len Brown <lenb@kernel.org> > > CC: linux-acpi@vger.kernel.org > > --- > > drivers/acpi/processor_driver.c | 75 +++++++++++++++++++++++++++++--------- > > include/acpi/processor.h | 1 + > > 2 files changed, 58 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c > > index 546951a..16f26b8 100644 > > --- a/drivers/acpi/processor_driver.c > > +++ b/drivers/acpi/processor_driver.c > > @@ -82,9 +82,9 @@ MODULE_LICENSE("GPL"); > > static int acpi_processor_add(struct acpi_device *device); > > static int acpi_processor_remove(struct acpi_device *device, int type); > > static void acpi_processor_notify(struct acpi_device *device, u32 event); > > -static acpi_status acpi_processor_hotadd_init(acpi_handle handle, int *p_cpu); > > +static acpi_status acpi_processor_hotadd_init(struct acpi_processor *pr); > > static int acpi_processor_handle_eject(struct acpi_processor *pr); > > - > > +static int acpi_processor_start(struct acpi_processor *pr); > > > > static const struct acpi_device_id processor_device_ids[] = { > > {ACPI_PROCESSOR_OBJECT_HID, 0}, > > @@ -324,10 +324,8 @@ static int acpi_processor_get_info(struct acpi_device *device) > > * they are physically not present. > > */ > > if (pr->id == -1) { > > - if (ACPI_FAILURE > > - (acpi_processor_hotadd_init(pr->handle, &pr->id))) { > > + if (ACPI_FAILURE(acpi_processor_hotadd_init(pr))) > > return -ENODEV; > > - } > > } > > /* > > * On some boxes several processors use the same processor bus id. > > @@ -425,10 +423,28 @@ static int acpi_cpu_soft_notify(struct notifier_block *nfb, > > struct acpi_processor *pr = per_cpu(processors, cpu); > > > > if (action == CPU_ONLINE && pr) { > > - acpi_processor_ppc_has_changed(pr, 0); > > - acpi_processor_cst_has_changed(pr); > > - acpi_processor_reevaluate_tstate(pr, action); > > - acpi_processor_tstate_has_changed(pr); > > I'm trying to figure out when this stuff (acpi_processor_ppc_has > changed(), _cst_has changed(), etc) now happens. It used to happen > when a hot-added CPU came online. Now it happens when a hot-added CPU > comes online *and* need_hotplug_init == 0. But need_hotplug_init is > set to 1 in the acpi_processor_add() -> acpi_processor_get_info() -> > acpi_processor_hotadd_init() path. If a core is (soft) onlined: If need_hotplug_init ==1, you have to initialize everything from scratch -> call .start() -> No need to check *_changed() states If need_hotplug_init == 0 the core was already active/booted, just check for possibly changed state info as before. > And I don't understand this weird interaction of "idle driver is > intel_idle" with all of this. If acpi_processor_ppc_has_changed(), et > al. are mutually exclusive with intel_idle, maybe we need a new idle > driver entry point or something. As it is, this is kind of a mess. I agree that this could need some cleanup. I'll wait a bit for further comments. Hm, I agree that this patch needs more discussion. Possibly the rest could get added already and I could come up with a new version of this one and an additional cleanup patch which addresses the already existing (intel_idle,..) things. This would make things easier for me... > > + /* CPU got hotplugged and onlined the first time > > + * Initialize missing things > > + */ > > + if (pr->flags.need_hotplug_init) { > > + struct cpuidle_driver *idle_driver = > > + cpuidle_get_driver(); > > + > > + printk(KERN_INFO "Will online and init hotplugged " > > + "CPU: %d\n", pr->id); > > + WARN(acpi_processor_start(pr), "Failed to start CPU:" > > + " %d\n", pr->id); > > Maybe I'm the only one, but I think the acpi_processor_start() call is > easy to miss when it's inside the WARN(). I'd rather see something > like: > > ret = acpi_processor_start(pr); > if (ret) > dev_warn(...); Ok. > > + pr->flags.need_hotplug_init = 0; > > + if (idle_driver && !strcmp(idle_driver->name, > > + "intel_idle")) { > > + intel_idle_cpu_init(pr->id); > > Checking the driver name here seems pretty ugly, and I think > intel_idle_cpu_init() is conditionally compiled, so it looks like this > will break if CONFIG_INTEL_IDLE=n. Should work, compare with patch 3/5: +#ifdef CONFIG_INTEL_IDLE +extern int intel_idle_cpu_init(int cpu); #else +static inline int intel_idle_cpu_init(int cpu) { return -1; } +#endif + +#else +static inline int intel_idle_cpu_init(int cpu) { return -1; } > > > + } > > + } else { > > + acpi_processor_ppc_has_changed(pr, 0); > > + acpi_processor_cst_has_changed(pr); > > + acpi_processor_reevaluate_tstate(pr, action); > > + acpi_processor_tstate_has_changed(pr); > > + } > > } > > if (action == CPU_DEAD && pr) { > > /* invalidate the flag.throttling after one CPU is offline */ > > @@ -442,7 +458,15 @@ static struct notifier_block acpi_cpu_notifier = > > .notifier_call = acpi_cpu_soft_notify, > > }; > > > > -static int __cpuinit acpi_processor_start(struct acpi_processor *pr) > > +/* > > + * acpi_processor_start() is called by the cpu_hotplug_notifier func: > > + * acpi_cpu_soft_notify(). Getting it __cpuinit{data} is difficult, the > > + * root cause seem to be that acpi_processor_uninstall_hotplug_notify() > > + * is in the module_exit (__exit) func. Allowing acpi_processor_start() > > + * to not be in __cpuinit section, but being called from __cpuinit funcs > > + * via __ref looks like the right thing to do here. > > + */ > > +static __ref int acpi_processor_start(struct acpi_processor *pr) > > This comment seems hand-wavey, and hence is a red flag :) As I > understand it, __ref is for use when non-init code references init > data. What initdata does acpi_processor_start() reference, and how do > we know that's always safe? (Maybe you just need to remove the > __cpuinit/__ref annotation completely?) Yep. I fear I have to remove quite some __cpuinit then. I wasn't sure what the best way to handle this is, therefore the somewhat longer comment. But calling a non __init function from __init context should be safe? That's what happens here and produces a warning if __ref is missing. Thomas --- The flags should get unsigned at the end...: drivers/acpi/processor_driver.c | 2 ++ drivers/base/cpu.c | 2 ++ include/linux/cpu.h | 3 ++- 3 files changed, 6 insertions(+), 1 deletion(-) Index: linux-3.0-SLE11-SP2-3.0/drivers/acpi/processor_driver.c =================================================================== --- linux-3.0-SLE11-SP2-3.0.orig/drivers/acpi/processor_driver.c +++ linux-3.0-SLE11-SP2-3.0/drivers/acpi/processor_driver.c @@ -811,6 +811,8 @@ static acpi_status acpi_processor_hotadd if (acpi_map_lsapic(handle, p_cpu)) return AE_ERROR; + per_cpu(cpu_devices, *p_cpu).cpu.hotplugged = 1; + if (arch_register_cpu(*p_cpu)) { acpi_unmap_lsapic(*p_cpu); return AE_ERROR; Index: linux-3.0-SLE11-SP2-3.0/drivers/base/cpu.c =================================================================== --- linux-3.0-SLE11-SP2-3.0.orig/drivers/base/cpu.c +++ linux-3.0-SLE11-SP2-3.0/drivers/base/cpu.c @@ -224,6 +224,8 @@ int __cpuinit register_cpu(struct cpu *c error = sysdev_register(&cpu->sysdev); + if (!error && cpu->hotplugged) + { /* arch_prepare_cpu() -> boot up and init CPU if necessary */ } if (!error && cpu->hotpluggable) register_cpu_control(cpu); if (!error) Index: linux-3.0-SLE11-SP2-3.0/include/linux/cpu.h =================================================================== --- linux-3.0-SLE11-SP2-3.0.orig/include/linux/cpu.h +++ linux-3.0-SLE11-SP2-3.0/include/linux/cpu.h @@ -21,7 +21,8 @@ struct cpu { int node_id; /* The node which contains the CPU */ - int hotpluggable; /* creates sysfs control file if hotpluggable */ + int hotpluggable:1; /* creates sysfs control file if hotpluggable */ + int hotplugged:1; /* will boot the cpu if registered */ struct sys_device sysdev; }; ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-09-13 12:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-11 23:42 Make ACPI physical CPU hotadd cpuidle robust Thomas Renninger 2011-09-11 23:42 ` [PATCH 1/5] ACPI processor: Avoid WARN message on processor driver removal Thomas Renninger 2011-09-12 17:14 ` Bjorn Helgaas 2011-09-11 23:42 ` [PATCH 2/5] intel_idle: Split up and provide per CPU initialization func Thomas Renninger 2011-09-11 23:42 ` [PATCH 3/5] ACPI processor: Fix error path, also remove sysdev link Thomas Renninger 2011-09-12 17:46 ` Bjorn Helgaas 2011-09-11 23:42 ` [PATCH 4/5] ACPI processor: Split up acpi_processor_add() Thomas Renninger 2011-09-11 23:42 ` [PATCH 5/5] ACPI processor hotplug: Delay most initialization to cpu online Thomas Renninger 2011-09-12 23:16 ` Bjorn Helgaas 2011-09-13 12:40 ` Thomas Renninger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox