Fix passive zone thermal throttling

Fix passive zone thermal throttling

[Andi Kleen]

I have a laptop with a broken fan and it would constantly shut down
under load. In principle passive thermal zone throttling was supposed
to prevent that by slowing down the CPU, but this didn't work.

This patchkit makes it work on my system.

It turned out to be a combination of problems: the EC driver 
talking to the embedded controller on my system would frequently
get out of step, which resulted in missing temperature events.
While not completely cured it's much better now with some
replies.

The other more serious problem was that the throttling code
never really worked on multi-core systems: it only lowers
the frequency of a single CPU, that gets the event, but the
other threads/cores still keep the frequency up. Also worse
throttling would never be invoked before the lowest frequency
is reached.

The first patch is merely a cleanup, the other patches fix
these problems.

Now that the kernel is fixed I can send the laptop in for repair.

-Andi

[PATCH 1/4] ACPI: ec: Do request_region outside WARN()

[Andi Kleen]

WARN() is not supposed to have side effects, so move the request_regions
outside.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/acpi/ec.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b19a18d..3268dcf 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -812,10 +812,10 @@ static int acpi_ec_add(struct acpi_device *device)
 		first_ec = ec;
 	device->driver_data = ec;
 
-	WARN(!request_region(ec->data_addr, 1, "EC data"),
-	     "Could not request EC data io port 0x%lx", ec->data_addr);
-	WARN(!request_region(ec->command_addr, 1, "EC cmd"),
-	     "Could not request EC cmd io port 0x%lx", ec->command_addr);
+	ret = !!request_region(ec->data_addr, 1, "EC data");
+	WARN(!ret, "Could not request EC data io port 0x%lx", ec->data_addr);
+	ret = !!request_region(ec->command_addr, 1, "EC cmd");
+	WARN(!ret, "Could not request EC cmd io port 0x%lx", ec->command_addr);
 
 	pr_info(PREFIX "GPE = 0x%lx, I/O: command/status = 0x%lx, data = 0x%lx\n",
 			  ec->gpe, ec->command_addr, ec->data_addr);
-- 
1.7.7

[PATCH 2/4] ACPI: Make ACPI interrupt threaded

[Andi Kleen]

Some ACPI interrupt actions may need to wait, and it's easiest to
have a thread context for this. So turn the ACPI interrupt
into a threaded interrupt.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/acpi/osl.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f31c5c5..3503434 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -564,7 +564,8 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
 
 	acpi_irq_handler = handler;
 	acpi_irq_context = context;
-	if (request_irq(irq, acpi_irq, IRQF_SHARED, "acpi", acpi_irq)) {
+	if (request_threaded_irq(irq, NULL, acpi_irq, IRQF_SHARED, "acpi", 
+				 acpi_irq)) {
 		printk(KERN_ERR PREFIX "SCI (IRQ%d) allocation failed\n", irq);
 		acpi_irq_handler = NULL;
 		return AE_NOT_ACQUIRED;
-- 
1.7.7

[PATCH 4/4] ACPI: Do cpufreq clamping for throttling per package instead of per CPU

[Andi Kleen]

On Intel CPUs the processor typically uses the highest frequency
set by any logical CPU. When the system overheats
Linux first forces the frequency to the lowest available one
to lower the temperature.

However this was done only per logical CPU, which means all
logical CPUs in a package would need to go through this before
the frequency is actually lowered.

Worse this delay actually prevents real throttling, because
the real throttle code only proceeds when the lowest frequency
is already reached.

So when a throttle event happens force the lowest frequency
for all CPUs in the package where it happened. The per CPU
state is now kept per package, not per logical CPU. An alternative
would be to do it per cpufreq unit, but since we want to bring
down the temperature of the complete chip it's better
to do it for all.

In principle it may even make sense to do it for all CPUs,
but I kept it on the package for now.

With this change the frequency is actually lowered, which
in terms also allows real throttling to proceed.

I also removed an unnecessary per cpu variable initialization.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 drivers/acpi/processor_thermal.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 870550d..d5dd141 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -58,6 +58,9 @@ ACPI_MODULE_NAME("processor_thermal");
 static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
 static unsigned int acpi_thermal_cpufreq_is_init = 0;
 
+#define reduction_pctg(cpu) \
+	per_cpu(cpufreq_thermal_reduction_pctg, topology_physical_package_id(cpu))
+
 static int cpu_has_cpufreq(unsigned int cpu)
 {
 	struct cpufreq_policy policy;
@@ -72,12 +75,15 @@ static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
 	struct cpufreq_policy *policy = data;
 	unsigned long max_freq = 0;
 
+	if (reduction_pctg(policy->cpu))
+		printk("cpufreq notifier %ld\n", event);
+
 	if (event != CPUFREQ_ADJUST)
 		goto out;
 
 	max_freq = (
 	    policy->cpuinfo.max_freq *
-	    (100 - per_cpu(cpufreq_thermal_reduction_pctg, policy->cpu) * 20)
+	    (100 - reduction_pctg(policy->cpu) * 20)
 	) / 100;
 
 	cpufreq_verify_within_limits(policy, 0, max_freq);
@@ -103,16 +109,26 @@ static int cpufreq_get_cur_state(unsigned int cpu)
 	if (!cpu_has_cpufreq(cpu))
 		return 0;
 
-	return per_cpu(cpufreq_thermal_reduction_pctg, cpu);
+	return reduction_pctg(cpu);
 }
 
 static int cpufreq_set_cur_state(unsigned int cpu, int state)
 {
+	int i;
+
 	if (!cpu_has_cpufreq(cpu))
 		return 0;
 
-	per_cpu(cpufreq_thermal_reduction_pctg, cpu) = state;
-	cpufreq_update_policy(cpu);
+	reduction_pctg(cpu) = state;
+
+	/* Update all the CPUs in the same package because they all
+	 * contribute to the temperature.
+	 */
+	for_each_online_cpu (i) {
+		if (topology_physical_package_id(i) == 
+		    topology_physical_package_id(cpu))
+			cpufreq_update_policy(i);
+	}
 	return 0;
 }
 
@@ -120,10 +136,6 @@ void acpi_thermal_cpufreq_init(void)
 {
 	int i;
 
-	for (i = 0; i < nr_cpu_ids; i++)
-		if (cpu_present(i))
-			per_cpu(cpufreq_thermal_reduction_pctg, i) = 0;
-
 	i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
 				      CPUFREQ_POLICY_NOTIFIER);
 	if (!i)
-- 
1.7.7

Re: [PATCH 4/4] ACPI: Do cpufreq clamping for throttling per package instead of per CPU

[Matthew Garrett]

On Mon, Dec 26, 2011 at 10:34:47PM -0800, Andi Kleen wrote:

> +	if (reduction_pctg(policy->cpu))
> +		printk("cpufreq notifier %ld\n", event);
> +

Needed?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 4/4] ACPI: Do cpufreq clamping for throttling per package instead of per CPU

[Andi Kleen]

On Tue, Dec 27, 2011 at 04:43:24PM +0000, Matthew Garrett wrote:
> On Mon, Dec 26, 2011 at 10:34:47PM -0800, Andi Kleen wrote:
> 
> > +	if (reduction_pctg(policy->cpu))
> > +		printk("cpufreq notifier %ld\n", event);
> > +
> 
> Needed?

No, i'll drop it.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 4/4] ACPI: Do cpufreq clamping for throttling per package instead of per CPU

[Matthew Garrett]

On Tue, Dec 27, 2011 at 10:21:26PM +0100, Andi Kleen wrote:
> On Tue, Dec 27, 2011 at 04:43:24PM +0000, Matthew Garrett wrote:
> > On Mon, Dec 26, 2011 at 10:34:47PM -0800, Andi Kleen wrote:
> > 
> > > +	if (reduction_pctg(policy->cpu))
> > > +		printk("cpufreq notifier %ld\n", event);
> > > +
> > 
> > Needed?
> 
> No, i'll drop it.

Thanks. Also, 3/4 didn't seem to make it to -kernel or -acpi.

-- 
Matthew Garrett | mjg59@srcf.ucam.org