[RESEND] X86 cpuidle cleanups

[RESEND] X86 cpuidle cleanups

[Thomas Renninger]

This is a rebase of some recently posted intel_idle processor_idle
related patches which needed refreshing, due to recent commits.

[PATCH 1/3] Revert "intel_idle: disable auto_demotion for hotplugged
-> because I think it's wrong/unneeded. I commented on the initial
posting.

I tested these patches before I had to rebase them.
Now they are compile tested, but adjusting the code wasn't that hard.

Would be great if these could get picked up for 3.3 inclusion.

Thanks,

   Thomas

[PATCH 1/3] Revert "intel_idle: disable auto_demotion for hotplugged CPUs"

[Thomas Renninger]

This reverts commit e5f03b06dc99348df53118c6ef7b51a8611a7256.

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 drivers/idle/intel_idle.c |   53 +++++++++++++++++----------------------------
 1 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index e56ae0b..91aff18 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -296,42 +296,33 @@ static void __setup_broadcast_timer(void *arg)
 	clockevents_notify(reason, &cpu);
 }
 
-static void auto_demotion_disable(void *dummy)
-{
-	unsigned long long msr_bits;
-
-	rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-	msr_bits &= ~auto_demotion_disable_flags;
-	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
-}
-
-static void __intel_idle_notify_handler(void *arg)
-{
-	if (auto_demotion_disable_flags)
-		auto_demotion_disable(NULL);
-
-	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
-		__setup_broadcast_timer((void *)true);
-}
-
-static int setup_intelidle_cpuhp_notify(struct notifier_block *n,
+static int setup_broadcast_cpuhp_notify(struct notifier_block *n,
 		unsigned long action, void *hcpu)
 {
 	int hotcpu = (unsigned long)hcpu;
 
 	switch (action & 0xf) {
 	case CPU_ONLINE:
-		smp_call_function_single(hotcpu, __intel_idle_notify_handler,
-			NULL, 1);
+		smp_call_function_single(hotcpu, __setup_broadcast_timer,
+			(void *)true, 1);
 		break;
 	}
 	return NOTIFY_OK;
 }
 
-static struct notifier_block setup_intelidle_notifier = {
-	.notifier_call = setup_intelidle_cpuhp_notify,
+static struct notifier_block setup_broadcast_notifier = {
+	.notifier_call = setup_broadcast_cpuhp_notify,
 };
 
+static void auto_demotion_disable(void *dummy)
+{
+	unsigned long long msr_bits;
+
+	rdmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+	msr_bits &= ~auto_demotion_disable_flags;
+	wrmsrl(MSR_NHM_SNB_PKG_CST_CFG_CTL, msr_bits);
+}
+
 /*
  * intel_idle_probe()
  */
@@ -402,8 +393,10 @@ static int intel_idle_probe(void)
 
 	if (boot_cpu_has(X86_FEATURE_ARAT))	/* Always Reliable APIC Timer */
 		lapic_timer_reliable_states = LAPIC_TIMER_ALWAYS_RELIABLE;
-	else
+	else {
 		on_each_cpu(__setup_broadcast_timer, (void *)true, 1);
+		register_cpu_notifier(&setup_broadcast_notifier);
+	}
 
 	pr_debug(PREFIX "v" INTEL_IDLE_VERSION
 		" model 0x%X\n", boot_cpu_data.x86_model);
@@ -566,10 +559,6 @@ static int __init intel_idle_init(void)
 		return retval;
 	}
 
-	if (auto_demotion_disable_flags || lapic_timer_reliable_states !=
-	    LAPIC_TIMER_ALWAYS_RELIABLE)
-		register_cpu_notifier(&setup_intelidle_notifier);
-
 	return 0;
 }
 
@@ -578,12 +567,10 @@ static void __exit intel_idle_exit(void)
 	intel_idle_cpuidle_devices_uninit();
 	cpuidle_unregister_driver(&intel_idle_driver);
 
-	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
+	if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE) {
 		on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
-
-	if (auto_demotion_disable_flags || lapic_timer_reliable_states !=
-	    LAPIC_TIMER_ALWAYS_RELIABLE)
-		unregister_cpu_notifier(&setup_intelidle_notifier);
+		unregister_cpu_notifier(&setup_broadcast_notifier);
+	}
 
 	return;
 }
-- 
1.7.6.1

[PATCH 2/3] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Thomas Renninger]

This is a very small part taken from patches which afaik
are coming from Yunhong Jiang for Xen.
Xen CPU hotplug things not existing in Linus kernel yet were
removed.

Cleanup only: no functional change.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: Jiang, Yunhong <yunhong.jiang@intel.com>
---
 drivers/acpi/processor_driver.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 90719d1..2291aa4 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -82,7 +82,7 @@ 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);
 
 
@@ -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->handle)))
 			return -ENODEV;
-		}
 	}
 	/*
 	 * On some boxes several processors use the same processor bus id.
@@ -721,18 +719,19 @@ 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)
 {
+	acpi_handle handle = pr->handle;
 
 	if (!is_processor_present(handle)) {
 		return AE_ERROR;
 	}
 
-	if (acpi_map_lsapic(handle, p_cpu))
+	if (acpi_map_lsapic(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;
 	}
 
@@ -749,7 +748,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;
 }
-- 
1.7.6.1

Re: [PATCH 2/3] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Thomas Renninger]

On Tuesday, January 17, 2012 05:20:29 PM Thomas Renninger wrote:
...
> +	if (acpi_map_lsapic(handle, &pr-id))
oops, blindly used guilt import.
I resend.
Sorry,

   Thomas

Re: [PATCH 2/3] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Konrad Rzeszutek Wilk]

On Tue, Jan 17, 2012 at 05:20:29PM +0100, Thomas Renninger wrote:
> This is a very small part taken from patches which afaik
> are coming from Yunhong Jiang for Xen.
> Xen CPU hotplug things not existing in Linus kernel yet were
> removed.

Could you point out the git commit please?
I did this:
 git log --grep=Yunhong drivers/acpi/processor_driver.c

And could not find it?
> 
> Cleanup only: no functional change.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-acpi@vger.kernel.org
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: Jiang, Yunhong <yunhong.jiang@intel.com>
> ---
>  drivers/acpi/processor_driver.c |   17 ++++++++---------
>  1 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 90719d1..2291aa4 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -82,7 +82,7 @@ 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);
>  
>  
> @@ -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->handle)))
>  			return -ENODEV;
> -		}
>  	}
>  	/*
>  	 * On some boxes several processors use the same processor bus id.
> @@ -721,18 +719,19 @@ 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)
>  {
> +	acpi_handle handle = pr->handle;
>  
>  	if (!is_processor_present(handle)) {
>  		return AE_ERROR;
>  	}
>  
> -	if (acpi_map_lsapic(handle, p_cpu))
> +	if (acpi_map_lsapic(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;
>  	}
>  
> @@ -749,7 +748,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;
>  }
> -- 
> 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

Re: [PATCH 2/3] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Thomas Renninger]

On Tuesday, January 17, 2012 05:29:51 PM Konrad Rzeszutek Wilk wrote:
> On Tue, Jan 17, 2012 at 05:20:29PM +0100, Thomas Renninger wrote:
> > This is a very small part taken from patches which afaik
> > are coming from Yunhong Jiang for Xen.
> > Xen CPU hotplug things not existing in Linus kernel yet were
> > removed.
> 
> Could you point out the git commit please?
> I did this:
>  git log --grep=Yunhong drivers/acpi/processor_driver.c
> 
> And could not find it?
Jiang may know in which Xen Linux kernel repo this went in.
I grepped the commit from our distri where Jiang was
mentioned as author.

Anyway, it's an improvement (one param less)
and makes things easier when dealing with ACPI CPU hotplug.

   Thomas

Re: [PATCH 2/3] ACPI processor: Remove unneeded variable passed by acpi_processor_hotadd_init

[Konrad Rzeszutek Wilk]

On Tue, Jan 17, 2012 at 05:47:53PM +0100, Thomas Renninger wrote:
> On Tuesday, January 17, 2012 05:29:51 PM Konrad Rzeszutek Wilk wrote:
> > On Tue, Jan 17, 2012 at 05:20:29PM +0100, Thomas Renninger wrote:
> > > This is a very small part taken from patches which afaik
> > > are coming from Yunhong Jiang for Xen.
> > > Xen CPU hotplug things not existing in Linus kernel yet were
> > > removed.
> > 
> > Could you point out the git commit please?
> > I did this:
> >  git log --grep=Yunhong drivers/acpi/processor_driver.c
> > 
> > And could not find it?
> Jiang may know in which Xen Linux kernel repo this went in.
> I grepped the commit from our distri where Jiang was
> mentioned as author.
> 
> Anyway, it's an improvement (one param less)
> and makes things easier when dealing with ACPI CPU hotplug.

Then please provide that description in the git commit.

How does it make it "easier" ?

[PATCH 3/3] intel_idle: Split up and provide per CPU initialization func

[Thomas Renninger]

Function split up, should have no functional change.

Provides entry point for physically hotplugged CPUs
to initialize and activate cpuidle.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-acpi@vger.kernel.org
CC: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
CC: Shaohua Li <shaohua.li@intel.com>
CC: Andrew Morton <akpm@linux-foundation.org>
---
 drivers/idle/intel_idle.c |   84 +++++++++++++++++++++++----------------------
 include/linux/cpuidle.h   |    7 ++++
 2 files changed, 50 insertions(+), 41 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 91aff18..be96715 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -478,64 +478,60 @@ static int intel_idle_cpuidle_driver_init(void)
 
 
 /*
- * 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;
-
-	for_each_online_cpu(i) {
-		dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
-
-		dev->state_count = 1;
+	dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu);
 
-		for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) {
-			int num_substates;
+	dev->state_count = 1;
 
-			if (cstate > max_cstate) {
-				printk(PREFIX "max_cstate %d reached\n",
-					max_cstate);
-				break;
-			}
+	for (cstate = 1; cstate < MWAIT_MAX_NUM_CSTATES; ++cstate) {
+		int num_substates;
 
-			/* 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) {
-				continue;
-			}
+		if (cstate > max_cstate) {
+			printk(PREFIX "max_cstate %d reached\n",
+			       max_cstate);
+			break;
+		}
 
-			dev->states_usage[dev->state_count].driver_data =
-				(void *)get_driver_data(cstate);
+		/* 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) {
+			continue;
+		}
+		dev->states_usage[dev->state_count].driver_data =
+			(void *)get_driver_data(cstate);
 
 			dev->state_count += 1;
 		}
+	dev->cpu = cpu;
 
-		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;
-		}
+	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_single(cpu, auto_demotion_disable, NULL, 1);
+
 	return 0;
 }
 
 
 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)
@@ -553,10 +549,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 7408af8..93df66e 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -188,7 +188,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