therm_throt: Refactor and improve thermal throttle processing

therm_throt: Refactor and improve thermal throttle processing

[Dmitriy Zavin]

This patch-set factors out the thermal throttle processing code
from i386 and x86_64 into a separate file (therm_throt.c).
This allows consistent reporting of CPU thermal throttle events.
Furthermore, a counter is added to /sys that keeps track of the
number of thermal events, such that the user knows how bad the
thermal problem might be (since the logging to syslog and mcelog
is rate limited).

 arch/i386/kernel/cpu/mcheck/Makefile      |    2
 arch/i386/kernel/cpu/mcheck/p4.c          |   26 +---
 arch/i386/kernel/cpu/mcheck/therm_throt.c |  185 ++++++++++++++++++++++++++++++
 arch/x86_64/kernel/Makefile               |    4
 arch/x86_64/kernel/mce_intel.c            |   29 +---
 include/asm-i386/therm_throt.h            |    9 +
 include/asm-x86_64/therm_throt.h          |    1
 include/linux/jiffies.h                   |   12 +
 8 files changed, 227 insertions(+), 41 deletions(-)

[PATCH 1/4] x86_64/i386 thermal mce: Refactor thermal throttle reporting.

[Dmitriy Zavin]

- Refactor the event processing (syslog/mcelog messaging and rate limiting)
  into separate file therm_throt.c.
- After ACK'ing the interrupt, if the event is current, the user
  (p4.c/mce_intel.c) just has to make a single call to take care of logging
  the event.

Signed-off-by: Dmitriy Zavin <dmitriyz@google.com>
---
 arch/i386/kernel/cpu/mcheck/Makefile      |    2 -
 arch/i386/kernel/cpu/mcheck/p4.c          |   23 ++------
 arch/i386/kernel/cpu/mcheck/therm_throt.c |   79 +++++++++++++++++++++++++++++
 arch/x86_64/kernel/Makefile               |    4 +
 arch/x86_64/kernel/mce_intel.c            |   26 ++--------
 include/asm-i386/therm_throt.h            |    8 +++
 include/asm-x86_64/therm_throt.h          |    1 
 7 files changed, 102 insertions(+), 41 deletions(-)

diff --git a/arch/i386/kernel/cpu/mcheck/Makefile b/arch/i386/kernel/cpu/mcheck/Makefile
index 30808f3..f1ebe1c 100644
--- a/arch/i386/kernel/cpu/mcheck/Makefile
+++ b/arch/i386/kernel/cpu/mcheck/Makefile
@@ -1,2 +1,2 @@
-obj-y	=	mce.o k7.o p4.o p5.o p6.o winchip.o
+obj-y	=	mce.o k7.o p4.o p5.o p6.o winchip.o therm_throt.o
 obj-$(CONFIG_X86_MCE_NONFATAL)	+=	non-fatal.o
diff --git a/arch/i386/kernel/cpu/mcheck/p4.c b/arch/i386/kernel/cpu/mcheck/p4.c
index b95f1b3..5f9896f 100644
--- a/arch/i386/kernel/cpu/mcheck/p4.c
+++ b/arch/i386/kernel/cpu/mcheck/p4.c
@@ -13,6 +13,8 @@ #include <asm/system.h>
 #include <asm/msr.h>
 #include <asm/apic.h>
 
+#include <asm/therm_throt.h>
+
 #include "mce.h"
 
 /* as supported by the P4/Xeon family */
@@ -44,25 +46,12 @@ static void unexpected_thermal_interrupt
 /* P4/Xeon Thermal transition interrupt handler */
 static void intel_thermal_interrupt(struct pt_regs *regs)
 {
-	u32 l, h;
-	unsigned int cpu = smp_processor_id();
-	static unsigned long next[NR_CPUS];
+	__u64 msr_val;
 
 	ack_APIC_irq();
 
-	if (time_after(next[cpu], jiffies))
-		return;
-
-	next[cpu] = jiffies + HZ*5;
-	rdmsr(MSR_IA32_THERM_STATUS, l, h);
-	if (l & 0x1) {
-		printk(KERN_EMERG "CPU%d: Temperature above threshold\n", cpu);
-		printk(KERN_EMERG "CPU%d: Running in modulated clock mode\n",
-				cpu);
-		add_taint(TAINT_MACHINE_CHECK);
-	} else {
-		printk(KERN_INFO "CPU%d: Temperature/speed normal\n", cpu);
-	}
+	rdmsrl(MSR_IA32_THERM_STATUS, msr_val);
+	therm_throt_process(msr_val & 0x1, msr_val);
 }
 
 /* Thermal interrupt handler for this CPU setup */
@@ -122,7 +111,7 @@ static void intel_init_thermal(struct cp
 	
 	rdmsr (MSR_IA32_MISC_ENABLE, l, h);
 	wrmsr (MSR_IA32_MISC_ENABLE, l | (1<<3), h);
-	
+
 	l = apic_read (APIC_LVTTHMR);
 	apic_write_around (APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
 	printk (KERN_INFO "CPU%d: Thermal monitoring enabled\n", cpu);
diff --git a/arch/i386/kernel/cpu/mcheck/therm_throt.c b/arch/i386/kernel/cpu/mcheck/therm_throt.c
new file mode 100644
index 0000000..342750e
--- /dev/null
+++ b/arch/i386/kernel/cpu/mcheck/therm_throt.c
@@ -0,0 +1,79 @@
+/*
+ * linux/arch/i386/kerne/cpu/mcheck/therm_throt.c
+ *
+ * Thermal throttle event support code.
+ *
+ * Author: Dmitriy Zavin (dmitriyz@google.com)
+ *
+ * Credits: Adapted from Zwane Mwaikambo's original code in mce_intel.c.
+ *
+ */
+
+#include <linux/percpu.h>
+#include <linux/cpu.h>
+#include <asm/cpu.h>
+#include <linux/notifier.h>
+#include <asm/therm_throt.h>
+
+#ifdef CONFIG_X86_64
+#include <asm/mce.h>
+#endif /* CONFIG_X86_64 */
+
+/* How long to wait between reporting thermal events */
+#define CHECK_INTERVAL              (300 * HZ)
+
+static DEFINE_PER_CPU(unsigned long, next_check);
+
+#ifdef CONFIG_X86_64
+static void therm_throt_log_mce(unsigned int cpu, __u64 status)
+{
+	struct mce m;
+
+	memset(&m, 0, sizeof(m));
+	m.cpu = cpu;
+	m.bank = MCE_THERMAL_BANK;
+	m.status = status;
+	rdtscll(m.tsc);
+	mce_log(&m);
+}
+#endif /* CONFIG_X86_64 */
+
+/***
+ * therm_throt_process - Process thermal throttling event
+ * @curr: Whether the condition is current or not (boolean), since the
+ *        thermal interrupt normally gets called both when the thermal
+ *        event begins and once the event has ended.
+ * @status: Event status information.
+ *
+ * This function is normally called by the thermal interrupt after the
+ * IRQ has been acknowledged.
+ *
+ * It will take care of rate limiting and printing messages to the syslog.
+ *
+ * The status parameter is used for an mce_log entry (if available), and
+ * historically has been the register value of the MSR_IA32_THERMAL_STATUS
+ * msr.
+ */
+void therm_throt_process(int curr, __u64 status)
+{
+	unsigned int cpu = smp_processor_id();
+
+	if (time_before(jiffies, __get_cpu_var(next_check)))
+		return;
+
+	__get_cpu_var(next_check) = jiffies + CHECK_INTERVAL;
+
+	/* if we just entered the thermal event */
+	if (curr) {
+		printk(KERN_CRIT "CPU%d: Temperature above threshold, "
+		       "cpu clock throttled\n", cpu);
+		add_taint(TAINT_MACHINE_CHECK);
+
+#ifdef CONFIG_X86_64
+		therm_throt_log_mce(cpu, status);
+#endif
+	} else {
+		printk(KERN_CRIT "CPU%d: Temperature/speed normal\n", cpu);
+	}
+
+}
diff --git a/arch/x86_64/kernel/Makefile b/arch/x86_64/kernel/Makefile
index b5aaeaf..d97cec0 100644
--- a/arch/x86_64/kernel/Makefile
+++ b/arch/x86_64/kernel/Makefile
@@ -11,7 +11,7 @@ obj-y	:= process.o signal.o entry.o trap
 		pci-dma.o pci-nommu.o alternative.o
 
 obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
-obj-$(CONFIG_X86_MCE)         += mce.o
+obj-$(CONFIG_X86_MCE)		+= mce.o therm_throt.o
 obj-$(CONFIG_X86_MCE_INTEL)	+= mce_intel.o
 obj-$(CONFIG_X86_MCE_AMD)	+= mce_amd.o
 obj-$(CONFIG_MTRR)		+= ../../i386/kernel/cpu/mtrr/
@@ -45,6 +45,7 @@ obj-y				+= intel_cacheinfo.o
 
 CFLAGS_vsyscall.o		:= $(PROFILING) -g0
 
+therm_throt-y                   += ../../i386/kernel/cpu/mcheck/therm_throt.o
 bootflag-y			+= ../../i386/kernel/bootflag.o
 cpuid-$(subst m,y,$(CONFIG_X86_CPUID))  += ../../i386/kernel/cpuid.o
 topology-y                     += ../../i386/kernel/topology.o
@@ -54,4 +55,3 @@ quirks-y			+= ../../i386/kernel/quirks.o
 i8237-y				+= ../../i386/kernel/i8237.o
 msr-$(subst m,y,$(CONFIG_X86_MSR))  += ../../i386/kernel/msr.o
 alternative-y			+= ../../i386/kernel/alternative.o
-
diff --git a/arch/x86_64/kernel/mce_intel.c b/arch/x86_64/kernel/mce_intel.c
index 8f533d2..a13096e 100644
--- a/arch/x86_64/kernel/mce_intel.c
+++ b/arch/x86_64/kernel/mce_intel.c
@@ -11,36 +11,20 @@ #include <asm/msr.h>
 #include <asm/mce.h>
 #include <asm/hw_irq.h>
 #include <asm/idle.h>
-
-static DEFINE_PER_CPU(unsigned long, next_check);
+#include <asm/therm_throt.h>
 
 asmlinkage void smp_thermal_interrupt(void)
 {
-	struct mce m;
+	__u64 msr_val;
 
 	ack_APIC_irq();
 
 	exit_idle();
 	irq_enter();
-	if (time_before(jiffies, __get_cpu_var(next_check)))
-		goto done;
-
-	__get_cpu_var(next_check) = jiffies + HZ*300;
-	memset(&m, 0, sizeof(m));
-	m.cpu = smp_processor_id();
-	m.bank = MCE_THERMAL_BANK;
-	rdtscll(m.tsc);
-	rdmsrl(MSR_IA32_THERM_STATUS, m.status);
-	if (m.status & 0x1) {
-		printk(KERN_EMERG
-			"CPU%d: Temperature above threshold, cpu clock throttled\n", m.cpu);
-		add_taint(TAINT_MACHINE_CHECK);
-	} else {
-		printk(KERN_EMERG "CPU%d: Temperature/speed normal\n", m.cpu);
-	}
 
-	mce_log(&m);
-done:
+	rdmsrl(MSR_IA32_THERM_STATUS, msr_val);
+	therm_throt_process(msr_val & 1, msr_val);
+
 	irq_exit();
 }
 
diff --git a/include/asm-i386/therm_throt.h b/include/asm-i386/therm_throt.h
new file mode 100644
index 0000000..c6e638b
--- /dev/null
+++ b/include/asm-i386/therm_throt.h
@@ -0,0 +1,8 @@
+#ifndef __ASM_I386_THERM_THROT_H__
+#define __ASM_I386_THERM_THROT_H__ 1
+
+#include <asm/types.h>
+
+void therm_throt_process(int curr, __u64 status);
+
+#endif /* __ASM_I386_THERM_THROT_H__ */
diff --git a/include/asm-x86_64/therm_throt.h b/include/asm-x86_64/therm_throt.h
new file mode 100644
index 0000000..5aac059
--- /dev/null
+++ b/include/asm-x86_64/therm_throt.h
@@ -0,0 +1 @@
+#include <asm-i386/therm_throt.h>
-- 
1.4.2

[PATCH 2/4] jiffies: Add 64bit jiffies compares (needed when long < 64bit).

[Dmitriy Zavin]

Signed-off-by: Dmitriy Zavin <dmitriyz@google.com>
---
 include/linux/jiffies.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 329ebcf..dc74075 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -115,6 +115,18 @@ #define time_after_eq(a,b)	\
 	 ((long)(a) - (long)(b) >= 0))
 #define time_before_eq(a,b)	time_after_eq(b,a)
 
+#define time_after64(a,b)		\
+	(typecheck(__u64, a) && \
+	 typecheck(__u64, b) && \
+	 ((__s64)(b) - (__s64)(a) < 0))
+#define time_before64(a,b)	time_after64(b,a)
+
+#define time_after_eq64(a,b)	\
+	(typecheck(__u64, a) && \
+	 typecheck(__u64, b) && \
+	 ((__s64)(a) - (__s64)(b) >= 0))
+#define time_before_eq64(a,b)	time_after_eq64(b,a)
+
 /*
  * Have the 32 bit jiffies value wrap 5 minutes after boot
  * so jiffies wrap bugs show up earlier.
-- 
1.4.2

[PATCH 3/4] therm_throt: Make the jiffies compares use the 64bit safe macros.

[Dmitriy Zavin]

Signed-off-by: Dmitriy Zavin <dmitriyz@google.com>
---
 arch/i386/kernel/cpu/mcheck/therm_throt.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/i386/kernel/cpu/mcheck/therm_throt.c b/arch/i386/kernel/cpu/mcheck/therm_throt.c
index 342750e..3d6f217 100644
--- a/arch/i386/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/i386/kernel/cpu/mcheck/therm_throt.c
@@ -22,7 +22,7 @@ #endif /* CONFIG_X86_64 */
 /* How long to wait between reporting thermal events */
 #define CHECK_INTERVAL              (300 * HZ)
 
-static DEFINE_PER_CPU(unsigned long, next_check);
+static DEFINE_PER_CPU(__u64, next_check) = INITIAL_JIFFIES;
 
 #ifdef CONFIG_X86_64
 static void therm_throt_log_mce(unsigned int cpu, __u64 status)
@@ -58,10 +58,10 @@ void therm_throt_process(int curr, __u64
 {
 	unsigned int cpu = smp_processor_id();
 
-	if (time_before(jiffies, __get_cpu_var(next_check)))
+	if (time_before64(get_jiffies_64(), __get_cpu_var(next_check)))
 		return;
 
-	__get_cpu_var(next_check) = jiffies + CHECK_INTERVAL;
+	__get_cpu_var(next_check) = get_jiffies_64() + CHECK_INTERVAL;
 
 	/* if we just entered the thermal event */
 	if (curr) {
-- 
1.4.2

[PATCH 4/4] therm_throt: Add a cumulative event counter and export it to /sys.

[Dmitriy Zavin]

Signed-off-by: Dmitriy Zavin <dmitriyz@google.com>
---
 arch/i386/kernel/cpu/mcheck/p4.c          |    3 +
 arch/i386/kernel/cpu/mcheck/therm_throt.c |  114 ++++++++++++++++++++++++++++-
 arch/x86_64/kernel/mce_intel.c            |    3 +
 include/asm-i386/therm_throt.h            |    1 
 4 files changed, 117 insertions(+), 4 deletions(-)

diff --git a/arch/i386/kernel/cpu/mcheck/p4.c b/arch/i386/kernel/cpu/mcheck/p4.c
index 5f9896f..7d10e67 100644
--- a/arch/i386/kernel/cpu/mcheck/p4.c
+++ b/arch/i386/kernel/cpu/mcheck/p4.c
@@ -115,6 +115,9 @@ static void intel_init_thermal(struct cp
 	l = apic_read (APIC_LVTTHMR);
 	apic_write_around (APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
 	printk (KERN_INFO "CPU%d: Thermal monitoring enabled\n", cpu);
+
+	/* enable thermal throttle processing */
+	atomic_set(&therm_throt_en, 1);
 	return;
 }
 #endif /* CONFIG_X86_MCE_P4THERMAL */
diff --git a/arch/i386/kernel/cpu/mcheck/therm_throt.c b/arch/i386/kernel/cpu/mcheck/therm_throt.c
index 3d6f217..c56fffb 100644
--- a/arch/i386/kernel/cpu/mcheck/therm_throt.c
+++ b/arch/i386/kernel/cpu/mcheck/therm_throt.c
@@ -2,14 +2,16 @@
  * linux/arch/i386/kerne/cpu/mcheck/therm_throt.c
  *
  * Thermal throttle event support code.
+ * Maintains a counter of thermal throttle events in /sys.
  *
  * Author: Dmitriy Zavin (dmitriyz@google.com)
  *
  * Credits: Adapted from Zwane Mwaikambo's original code in mce_intel.c.
- *
+ *          Inspired by rossb and alb's counter code.
  */
 
 #include <linux/percpu.h>
+#include <linux/sysdev.h>
 #include <linux/cpu.h>
 #include <asm/cpu.h>
 #include <linux/notifier.h>
@@ -23,6 +25,44 @@ #endif /* CONFIG_X86_64 */
 #define CHECK_INTERVAL              (300 * HZ)
 
 static DEFINE_PER_CPU(__u64, next_check) = INITIAL_JIFFIES;
+static DEFINE_PER_CPU(unsigned long, thermal_throttle_count);
+atomic_t therm_throt_en = ATOMIC_INIT(0);
+
+#ifdef CONFIG_SYSFS
+#define define_therm_throt_sysdev_one_ro(_name)                              \
+        static SYSDEV_ATTR(_name, 0444, therm_throt_sysdev_show_##_name, NULL)
+
+#define define_therm_throt_sysdev_show_func(name)                            \
+static ssize_t therm_throt_sysdev_show_##name(struct sys_device *dev,        \
+                                              char *buf)                     \
+{                                                                            \
+	unsigned int cpu = dev->id;                                          \
+	ssize_t ret;                                                         \
+                                                                             \
+	preempt_disable();              /* CPU hotplug */                    \
+	if (cpu_online(cpu))                                                 \
+		ret = sprintf(buf, "%lu\n",                                  \
+			      per_cpu(thermal_throttle_##name, cpu));        \
+	else                                                                 \
+		ret = 0;                                                     \
+	preempt_enable();                                                    \
+                                                                             \
+	return ret;                                                          \
+}
+
+define_therm_throt_sysdev_show_func(count);
+define_therm_throt_sysdev_one_ro(count);
+
+static struct attribute *thermal_throttle_attrs[] = {
+	&attr_count.attr,
+	NULL
+};
+
+static struct attribute_group thermal_throttle_attr_group = {
+	.attrs = thermal_throttle_attrs,
+	.name = "thermal_throttle"
+};
+#endif /* CONFIG_SYSFS */
 
 #ifdef CONFIG_X86_64
 static void therm_throt_log_mce(unsigned int cpu, __u64 status)
@@ -39,13 +79,13 @@ static void therm_throt_log_mce(unsigned
 #endif /* CONFIG_X86_64 */
 
 /***
- * therm_throt_process - Process thermal throttling event
+ * therm_throt_process - Process thermal throttling event from interrupt
  * @curr: Whether the condition is current or not (boolean), since the
  *        thermal interrupt normally gets called both when the thermal
  *        event begins and once the event has ended.
  * @status: Event status information.
  *
- * This function is normally called by the thermal interrupt after the
+ * This function is called by the thermal interrupt after the
  * IRQ has been acknowledged.
  *
  * It will take care of rate limiting and printing messages to the syslog.
@@ -58,6 +98,9 @@ void therm_throt_process(int curr, __u64
 {
 	unsigned int cpu = smp_processor_id();
 
+	if (curr)
+		__get_cpu_var(thermal_throttle_count)++;
+
 	if (time_before64(get_jiffies_64(), __get_cpu_var(next_check)))
 		return;
 
@@ -66,7 +109,9 @@ void therm_throt_process(int curr, __u64
 	/* if we just entered the thermal event */
 	if (curr) {
 		printk(KERN_CRIT "CPU%d: Temperature above threshold, "
-		       "cpu clock throttled\n", cpu);
+		       "cpu clock throttled (total events = %lu)\n", cpu,
+		       __get_cpu_var(thermal_throttle_count));
+
 		add_taint(TAINT_MACHINE_CHECK);
 
 #ifdef CONFIG_X86_64
@@ -75,5 +120,66 @@ #endif
 	} else {
 		printk(KERN_CRIT "CPU%d: Temperature/speed normal\n", cpu);
 	}
+}
+
+#ifdef CONFIG_SYSFS
+/* Add/Remove thermal_throttle interface for CPU device */
+static __cpuinit int thermal_throttle_add_dev(struct sys_device * sys_dev)
+{
+	sysfs_create_group(&sys_dev->kobj, &thermal_throttle_attr_group);
+	return 0;
+}
+
+#ifdef CONFIG_HOTPLUG_CPU
+static __cpuinit int thermal_throttle_remove_dev(struct sys_device * sys_dev)
+{
+	sysfs_remove_group(&sys_dev->kobj, &thermal_throttle_attr_group);
+	return 0;
+}
+
+/* Get notified when a cpu comes on/off. Be hotplug friendly. */
+static __cpuinit int thermal_throttle_cpu_callback(struct notifier_block *nfb,
+						   unsigned long action,
+						   void *hcpu)
+{
+	unsigned int cpu = (unsigned long)hcpu;
+	struct sys_device *sys_dev;
+
+	sys_dev = get_cpu_sysdev(cpu);
+	switch (action) {
+		case CPU_ONLINE:
+			thermal_throttle_add_dev(sys_dev);
+			break;
+		case CPU_DEAD:
+			thermal_throttle_remove_dev(sys_dev);
+			break;
+	}
+	return NOTIFY_OK;
+}
 
+static struct notifier_block thermal_throttle_cpu_notifier =
+{
+	.notifier_call = thermal_throttle_cpu_callback,
+};
+#endif
+
+static __init int thermal_throttle_init_device(void)
+{
+	unsigned int cpu = 0;
+
+	if (!atomic_read(&therm_throt_en))
+		return 0;
+
+	preempt_disable();         /* CPU hotplug */
+	for_each_online_cpu(cpu) {
+		/* connect live CPUs to sysfs */
+		thermal_throttle_add_dev(get_cpu_sysdev(cpu));
+	}
+	preempt_enable();
+
+	register_hotcpu_notifier(&thermal_throttle_cpu_notifier);
+	return 0;
 }
+
+device_initcall(thermal_throttle_init_device);
+#endif /* CONFIG_SYSFS */
diff --git a/arch/x86_64/kernel/mce_intel.c b/arch/x86_64/kernel/mce_intel.c
index a13096e..7bbcfbb 100644
--- a/arch/x86_64/kernel/mce_intel.c
+++ b/arch/x86_64/kernel/mce_intel.c
@@ -76,6 +76,9 @@ static void __cpuinit intel_init_thermal
 	apic_write(APIC_LVTTHMR, l & ~APIC_LVT_MASKED);
 	printk(KERN_INFO "CPU%d: Thermal monitoring enabled (%s)\n",
 		cpu, tm2 ? "TM2" : "TM1");
+
+	/* enable thermal throttle processing */
+	atomic_set(&therm_throt_en, 1);
 	return;
 }
 
diff --git a/include/asm-i386/therm_throt.h b/include/asm-i386/therm_throt.h
index c6e638b..4cf4f75 100644
--- a/include/asm-i386/therm_throt.h
+++ b/include/asm-i386/therm_throt.h
@@ -3,6 +3,7 @@ #define __ASM_I386_THERM_THROT_H__ 1
 
 #include <asm/types.h>
 
+extern atomic_t therm_throt_en;
 void therm_throt_process(int curr, __u64 status);
 
 #endif /* __ASM_I386_THERM_THROT_H__ */
-- 
1.4.2

Re: [PATCH 1/4] x86_64/i386 thermal mce: Refactor thermal throttle reporting.

[Andi Kleen]

> +
> +#include <linux/percpu.h>
> +#include <linux/cpu.h>
> +#include <asm/cpu.h>
> +#include <linux/notifier.h>
> +#include <asm/therm_throt.h>
> +
> +#ifdef CONFIG_X86_64


Sorry, no such ifdefs allowed. That is what I meant with making the merged code
worse than split code.
 
-Andi

Re: [PATCH 2/4] jiffies: Add 64bit jiffies compares (needed when long < 64bit).

[Andi Kleen]

> +#define time_after64(a,b)		\
> +	(typecheck(__u64, a) && \
> +	 typecheck(__u64, b) && \

Did you double check the typecheck DTRT when someone
passes in both plain long and long long on 64bit?

-Andi

Re: [PATCH 4/4] therm_throt: Add a cumulative event counter and export it to /sys.

[Andi Kleen]

On Wednesday 20 September 2006 04:42, Dmitriy Zavin wrote:


Looks good thanks.

Although a sentence or more changelog would be appreciated.

-Andi

Re: [PATCH 3/4] therm_throt: Make the jiffies compares use the 64bit safe macros.

[Andi Kleen]

On Wednesday 20 September 2006 04:42, Dmitriy Zavin wrote:

Looks good thanks.

(i haven't merged yet though because of the dependencies) 

-Andi

Re: therm_throt: Refactor and improve thermal throttle processing

[Andi Kleen]

On Wednesday 20 September 2006 04:42, Dmitriy Zavin wrote:
> This patch-set factors out the thermal throttle processing code
> from i386 and x86_64 into a separate file (therm_throt.c).
> This allows consistent reporting of CPU thermal throttle events.
> Furthermore, a counter is added to /sys that keeps track of the
> number of thermal events, such that the user knows how bad the
> thermal problem might be (since the logging to syslog and mcelog
> is rate limited).

Can you put this blurb into the description of the main patch that implements
the code please too? 

-Andi

Re: [PATCH 1/4] x86_64/i386 thermal mce: Refactor thermal throttle reporting.

[Dmitriy Zavin]

Ok, will move the mce_log part into mce.c in x86_64 and go back to
returning 0/1 from therm_throt_process. Then only the x86_64 thermal
mce will mcelog, and no #ifdefs :)

On 9/20/06, Andi Kleen <ak@suse.de> wrote:
>
> > +
> > +#include <linux/percpu.h>
> > +#include <linux/cpu.h>
> > +#include <asm/cpu.h>
> > +#include <linux/notifier.h>
> > +#include <asm/therm_throt.h>
> > +
> > +#ifdef CONFIG_X86_64
>
>
> Sorry, no such ifdefs allowed. That is what I meant with making the merged code
> worse than split code.
>
> -Andi
>

Re: [PATCH 2/4] jiffies: Add 64bit jiffies compares (needed when long < 64bit).

[Dmitriy Zavin]

These compares are there to use jiffies_64 safely on 32bit and 64 bit
systems, and jiffies_64 (and thus get_jiffies_64()) is always defined
as u64. Since this is not arch code, but generic for all
architectures, it should use the generic u64/s64 types. If the user
passes in a long and assumes that it's 64 bit, then it is user error.
I'll add a comment that says that these macros must be used with u64
values (as returned by get_jiffies_64()).

Thanks.

--Dima

P.S. This also means that currently, on 64 bit systems,
time_before/time_after is broken (typecheck fails) on values returned
by get_jiffies_64().

On 9/20/06, Andi Kleen <ak@suse.de> wrote:
>
> > +#define time_after64(a,b)            \
> > +     (typecheck(__u64, a) && \
> > +      typecheck(__u64, b) && \
>
> Did you double check the typecheck DTRT when someone
> passes in both plain long and long long on 64bit?
>
> -Andi
>
>