[PATCH 0/3] fix debug message of LOCAL APIC calibration

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] fix debug message of LOCAL APIC calibration
@ 2009-01-27  8:13 Yasuaki Ishimatsu
  2009-01-27  8:21 ` [PATCH 1/3] fix debug message of CPU clock speed Yasuaki Ishimatsu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Yasuaki Ishimatsu @ 2009-01-27  8:13 UTC (permalink / raw)
  To: linux-kernel; +Cc: mingo

Hi,

There are some minor fix patches for LOCAL APIC calibration.

	[PATCH 1/3] fix debug message of CPU clock speed
	[PATCH 2/3] unify PM-Timer messages
	[PATCH 3/3] fix typo "ACPI_PM_OVRRUN" -> "ACPI_PM_OVERRUN"

For more details, please refer to the header of each patch.

Thanks,
Yasuaki Ishimatsu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] fix debug message of CPU clock speed
  2009-01-27  8:13 [PATCH 0/3] fix debug message of LOCAL APIC calibration Yasuaki Ishimatsu
@ 2009-01-27  8:21 ` Yasuaki Ishimatsu
  2009-01-27 12:53   ` Ingo Molnar
  2009-01-27  8:22 ` [PATCH 2/3] unify PM-Timer messages Yasuaki Ishimatsu
  2009-01-27  8:24 ` [PATCH 3/3] fix typo "ACPI_PM_OVRRUN" -> "ACPI_PM_OVERRUN" Yasuaki Ishimatsu
  2 siblings, 1 reply; 6+ messages in thread
From: Yasuaki Ishimatsu @ 2009-01-27  8:21 UTC (permalink / raw)
  To: linux-kernel; +Cc: mingo

LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC is calibrated.
In this case, LOCAL APIC debug message(Boot with apic=debug) is displayed correctly,
however, CPU clock speed debug message is displayed wrongly .

When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is displayed
3622.0205 MHz as follow.

	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
	Using local APIC timer interrupts.
	calibrating APIC timer ...
	... lapic delta = 3773130
	... PM timer delta = 812434
	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
	APIC delta adjusted to PM-Timer: 1662420 (3773130)
	..... delta 1662420
	..... mult: 71411249
	..... calibration result: 265987
	..... CPU clock speed is 3622.0205 MHz.  =====>  here
	..... host bus clock speed is 265.0987 MHz.

This patch fixes to displaying CPU clock speed correctly as follow.

	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
	Using local APIC timer interrupts.
	calibrating APIC timer ...
	... lapic delta = 3773131
	... PM timer delta = 812434
	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
	APIC delta adjusted to PM-Timer: 1662420 (3773131)
	TSC delta adjusted to PM-Timer: 159592409 (362220564)
	..... delta 1662420
	..... mult: 71411249
	..... calibration result: 265987
	..... CPU clock speed is 1595.0924 MHz.
	..... host bus clock speed is 265.0987 MHz.

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

---
 arch/x86/kernel/apic.c |   23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c	2009-01-27 11:04:40.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/apic.c	2009-01-27 12:29:03.000000000 +0900
@@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str
 	}
 }

-static int __init calibrate_by_pmtimer(long deltapm, long *delta)
+static int __init calibrate_by_pmtimer(long deltapm, long *delta,
+						long *deltatsc)
 {
 	const long pm_100ms = PMTMR_TICKS_PER_SEC / 10;
 	const long pm_thresh = pm_100ms / 100;
@@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l
 		pr_info("APIC delta adjusted to PM-Timer: "
 			"%lu (%ld)\n", (unsigned long)res, *delta);
 		*delta = (long)res;
+
+		if (cpu_has_tsc) {
+			res = (((u64)(*deltatsc)) * pm_100ms);
+			do_div(res, deltapm);
+			apic_printk(APIC_VERBOSE, "TSC delta adjusted to "
+					"PM-Timer: %lu (%ld)\n",
+					(unsigned long)res, *deltatsc);
+			*deltatsc = (long)res;
+		}
 	}

 	return 0;
@@ -579,7 +589,7 @@ static int __init calibrate_APIC_clock(v
 	struct clock_event_device *levt = &__get_cpu_var(lapic_events);
 	void (*real_handler)(struct clock_event_device *dev);
 	unsigned long deltaj;
-	long delta;
+	long delta, deltatsc;
 	int pm_referenced = 0;

 	local_irq_disable();
@@ -609,9 +619,11 @@ static int __init calibrate_APIC_clock(v
 	delta = lapic_cal_t1 - lapic_cal_t2;
 	apic_printk(APIC_VERBOSE, "... lapic delta = %ld\n", delta);

+	deltatsc = (long)(lapic_cal_tsc2 - lapic_cal_tsc1);
+
 	/* we trust the PM based calibration if possible */
 	pm_referenced = !calibrate_by_pmtimer(lapic_cal_pm2 - lapic_cal_pm1,
-					&delta);
+					&delta, &deltatsc);

 	/* Calculate the scaled math multiplication factor */
 	lapic_clockevent.mult = div_sc(delta, TICK_NSEC * LAPIC_CAL_LOOPS,
@@ -629,11 +641,10 @@ static int __init calibrate_APIC_clock(v
 		    calibration_result);

 	if (cpu_has_tsc) {
-		delta = (long)(lapic_cal_tsc2 - lapic_cal_tsc1);
 		apic_printk(APIC_VERBOSE, "..... CPU clock speed is "
 			    "%ld.%04ld MHz.\n",
-			    (delta / LAPIC_CAL_LOOPS) / (1000000 / HZ),
-			    (delta / LAPIC_CAL_LOOPS) % (1000000 / HZ));
+			    (deltatsc / LAPIC_CAL_LOOPS) / (1000000 / HZ),
+			    (deltatsc / LAPIC_CAL_LOOPS) % (1000000 / HZ));
 	}

 	apic_printk(APIC_VERBOSE, "..... host bus clock speed is "



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] fix debug message of CPU clock speed
  2009-01-27  8:21 ` [PATCH 1/3] fix debug message of CPU clock speed Yasuaki Ishimatsu
@ 2009-01-27 12:53   ` Ingo Molnar
  2009-01-28  2:33     ` Yasuaki Ishimatsu
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2009-01-27 12:53 UTC (permalink / raw)
  To: Yasuaki Ishimatsu; +Cc: linux-kernel, mingo


* Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> wrote:

> LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC 
> is calibrated. In this case, LOCAL APIC debug message(Boot with 
> apic=debug) is displayed correctly, however, CPU clock speed debug 
> message is displayed wrongly .

your fix looks good, but there are a few minor style details that need to 
be addressed:

> When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is 
> displayed 3622.0205 MHz as follow.
> 
> 	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
> 	Using local APIC timer interrupts.
> 	calibrating APIC timer ...
> 	... lapic delta = 3773130
> 	... PM timer delta = 812434
> 	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
> 	APIC delta adjusted to PM-Timer: 1662420 (3773130)
> 	..... delta 1662420
> 	..... mult: 71411249
> 	..... calibration result: 265987
> 	..... CPU clock speed is 3622.0205 MHz.  =====>  here
> 	..... host bus clock speed is 265.0987 MHz.
> 
> This patch fixes to displaying CPU clock speed correctly as follow.
> 
> 	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
> 	Using local APIC timer interrupts.
> 	calibrating APIC timer ...
> 	... lapic delta = 3773131
> 	... PM timer delta = 812434
> 	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
> 	APIC delta adjusted to PM-Timer: 1662420 (3773131)
> 	TSC delta adjusted to PM-Timer: 159592409 (362220564)
> 	..... delta 1662420
> 	..... mult: 71411249
> 	..... calibration result: 265987
> 	..... CPU clock speed is 1595.0924 MHz.
> 	..... host bus clock speed is 265.0987 MHz.
> 
> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> 
> ---
>  arch/x86/kernel/apic.c |   23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
> ===================================================================
> --- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c	2009-01-27 11:04:40.000000000 +0900
> +++ linux-2.6.29-rc2/arch/x86/kernel/apic.c	2009-01-27 12:29:03.000000000 +0900
> @@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str
>  	}
>  }
> 
> -static int __init calibrate_by_pmtimer(long deltapm, long *delta)
> +static int __init calibrate_by_pmtimer(long deltapm, long *delta,
> +						long *deltatsc)

The cleaner prototype line-width break is:

  + static int __init
  + calibrate_by_pmtimer(long deltapm, long *delta, long *deltatsc)


this function:

> @@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l
>  		pr_info("APIC delta adjusted to PM-Timer: "
>  			"%lu (%ld)\n", (unsigned long)res, *delta);
>  		*delta = (long)res;
> +
> +		if (cpu_has_tsc) {
> +			res = (((u64)(*deltatsc)) * pm_100ms);
> +			do_div(res, deltapm);
> +			apic_printk(APIC_VERBOSE, "TSC delta adjusted to "
> +					"PM-Timer: %lu (%ld)\n",
> +					(unsigned long)res, *deltatsc);
> +			*deltatsc = (long)res;
> +		}

has too deep nesting in the form of:

	if (A) {
		...
	} else {
		...
		if (B) {
			...
		}
	}

	return 0;


Could you please restructure it to a cleaner control flow, in the form of:

	if (A) {
		...
		return 0;
	}
	...
	if (B) {
		...
  	}

	return 0;

?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] fix debug message of CPU clock speed
  2009-01-27 12:53   ` Ingo Molnar
@ 2009-01-28  2:33     ` Yasuaki Ishimatsu
  0 siblings, 0 replies; 6+ messages in thread
From: Yasuaki Ishimatsu @ 2009-01-28  2:33 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, mingo

Hi, Ingo

Thank you for your comments.
I apply your comments to the patch ASAP.

Regard,
Yasuaki Ishimatsu

Ingo Molnar wrote:
> * Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com> wrote:
> 
>> LOCAL APIC is corrected by PM-Timer, when SMI occurred while LOCAL APIC 
>> is calibrated. In this case, LOCAL APIC debug message(Boot with 
>> apic=debug) is displayed correctly, however, CPU clock speed debug 
>> message is displayed wrongly .
> 
> your fix looks good, but there are a few minor style details that need to 
> be addressed:
> 
>> When SMI occured on my machine, which has 1.6GHz CPU, CPU clock speed is 
>> displayed 3622.0205 MHz as follow.
>>
>> 	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
>> 	Using local APIC timer interrupts.
>> 	calibrating APIC timer ...
>> 	... lapic delta = 3773130
>> 	... PM timer delta = 812434
>> 	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
>> 	APIC delta adjusted to PM-Timer: 1662420 (3773130)
>> 	..... delta 1662420
>> 	..... mult: 71411249
>> 	..... calibration result: 265987
>> 	..... CPU clock speed is 3622.0205 MHz.  =====>  here
>> 	..... host bus clock speed is 265.0987 MHz.
>>
>> This patch fixes to displaying CPU clock speed correctly as follow.
>>
>> 	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
>> 	Using local APIC timer interrupts.
>> 	calibrating APIC timer ...
>> 	... lapic delta = 3773131
>> 	... PM timer delta = 812434
>> 	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
>> 	APIC delta adjusted to PM-Timer: 1662420 (3773131)
>> 	TSC delta adjusted to PM-Timer: 159592409 (362220564)
>> 	..... delta 1662420
>> 	..... mult: 71411249
>> 	..... calibration result: 265987
>> 	..... CPU clock speed is 1595.0924 MHz.
>> 	..... host bus clock speed is 265.0987 MHz.
>>
>> Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
>>
>> ---
>>  arch/x86/kernel/apic.c |   23 +++++++++++++++++------
>>  1 file changed, 17 insertions(+), 6 deletions(-)
>>
>> Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
>> ===================================================================
>> --- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c	2009-01-27 11:04:40.000000000 +0900
>> +++ linux-2.6.29-rc2/arch/x86/kernel/apic.c	2009-01-27 12:29:03.000000000 +0900
>> @@ -535,7 +535,8 @@ static void __init lapic_cal_handler(str
>>  	}
>>  }
>>
>> -static int __init calibrate_by_pmtimer(long deltapm, long *delta)
>> +static int __init calibrate_by_pmtimer(long deltapm, long *delta,
>> +						long *deltatsc)
> 
> The cleaner prototype line-width break is:
> 
>   + static int __init
>   + calibrate_by_pmtimer(long deltapm, long *delta, long *deltatsc)
> 
> 
> this function:
> 
>> @@ -569,6 +570,15 @@ static int __init calibrate_by_pmtimer(l
>>  		pr_info("APIC delta adjusted to PM-Timer: "
>>  			"%lu (%ld)\n", (unsigned long)res, *delta);
>>  		*delta = (long)res;
>> +
>> +		if (cpu_has_tsc) {
>> +			res = (((u64)(*deltatsc)) * pm_100ms);
>> +			do_div(res, deltapm);
>> +			apic_printk(APIC_VERBOSE, "TSC delta adjusted to "
>> +					"PM-Timer: %lu (%ld)\n",
>> +					(unsigned long)res, *deltatsc);
>> +			*deltatsc = (long)res;
>> +		}
> 
> has too deep nesting in the form of:
> 
> 	if (A) {
> 		...
> 	} else {
> 		...
> 		if (B) {
> 			...
> 		}
> 	}
> 
> 	return 0;
> 
> 
> Could you please restructure it to a cleaner control flow, in the form of:
> 
> 	if (A) {
> 		...
> 		return 0;
> 	}
> 	...
> 	if (B) {
> 		...
>   	}
> 
> 	return 0;
> 
> ?
> 
> Thanks,
> 
> 	Ingo
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] unify PM-Timer messages
  2009-01-27  8:13 [PATCH 0/3] fix debug message of LOCAL APIC calibration Yasuaki Ishimatsu
  2009-01-27  8:21 ` [PATCH 1/3] fix debug message of CPU clock speed Yasuaki Ishimatsu
@ 2009-01-27  8:22 ` Yasuaki Ishimatsu
  2009-01-27  8:24 ` [PATCH 3/3] fix typo "ACPI_PM_OVRRUN" -> "ACPI_PM_OVERRUN" Yasuaki Ishimatsu
  2 siblings, 0 replies; 6+ messages in thread
From: Yasuaki Ishimatsu @ 2009-01-27  8:22 UTC (permalink / raw)
  To: linux-kernel; +Cc: mingo

When LOCAL APIC was calibrated, the debug message is displayed as follows.

	CPU0: Intel(R) Xeon(R) CPU            5110  @ 1.60GHz stepping 06
	Using local APIC timer interrupts.
	calibrating APIC timer ...
	... lapic delta = 3773131
	... PM timer delta = 812434
	APIC calibration not consistent with PM Timer: 226ms instead of 100ms
	APIC delta adjusted to PM-Timer: 1662420 (3773131)
	TSC delta adjusted to PM-Timer: 159592409 (362220564)
	..... delta 1662420
	..... mult: 71411249
	..... calibration result: 265987
	..... CPU clock speed is 1595.0924 MHz.
	..... host bus clock speed is 265.0987 MHz.

The PM-timer is written in several ways (PM-Timer, PM Timer, and PM timer)
in this message. This patch unifies those messages.

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

---
 arch/x86/kernel/apic.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c	2009-01-27 12:21:24.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/apic.c	2009-01-27 12:24:04.000000000 +0900
@@ -547,7 +547,7 @@ static int __init calibrate_by_pmtimer(l
 	return -1;
 #endif

-	apic_printk(APIC_VERBOSE, "... PM timer delta = %ld\n", deltapm);
+	apic_printk(APIC_VERBOSE, "... PM-Timer delta = %ld\n", deltapm);

 	/* Check, if the PM timer is available */
 	if (!deltapm)
@@ -557,12 +557,12 @@ static int __init calibrate_by_pmtimer(l

 	if (deltapm > (pm_100ms - pm_thresh) &&
 	    deltapm < (pm_100ms + pm_thresh)) {
-		apic_printk(APIC_VERBOSE, "... PM timer result ok\n");
+		apic_printk(APIC_VERBOSE, "... PM-Timer result ok\n");
 	} else {
 		res = (((u64)deltapm) *  mult) >> 22;
 		do_div(res, 1000000);
 		pr_warning("APIC calibration not consistent "
-			"with PM Timer: %ldms instead of 100ms\n",
+			"with PM-Timer: %ldms instead of 100ms\n",
 			(long)res);
 		/* Correct the lapic counter value */
 		res = (((u64)(*delta)) * pm_100ms);


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] fix typo "ACPI_PM_OVRRUN" -> "ACPI_PM_OVERRUN"
  2009-01-27  8:13 [PATCH 0/3] fix debug message of LOCAL APIC calibration Yasuaki Ishimatsu
  2009-01-27  8:21 ` [PATCH 1/3] fix debug message of CPU clock speed Yasuaki Ishimatsu
  2009-01-27  8:22 ` [PATCH 2/3] unify PM-Timer messages Yasuaki Ishimatsu
@ 2009-01-27  8:24 ` Yasuaki Ishimatsu
  2 siblings, 0 replies; 6+ messages in thread
From: Yasuaki Ishimatsu @ 2009-01-27  8:24 UTC (permalink / raw)
  To: linux-kernel; +Cc: mingo

This patch fixes typo.

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

---
 arch/x86/kernel/apic.c     |    2 +-
 arch/x86/kernel/tsc.c      |    2 +-
 include/linux/acpi_pmtmr.h |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6.29-rc2/arch/x86/kernel/apic.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/apic.c	2009-01-27 12:24:04.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/apic.c	2009-01-27 12:24:18.000000000 +0900
@@ -528,7 +528,7 @@ static void __init lapic_cal_handler(str
 		lapic_cal_t2 = tapic;
 		lapic_cal_tsc2 = tsc;
 		if (pm < lapic_cal_pm1)
-			pm += ACPI_PM_OVRRUN;
+			pm += ACPI_PM_OVERRUN;
 		lapic_cal_pm2 = pm;
 		lapic_cal_j2 = jiffies;
 		break;
Index: linux-2.6.29-rc2/arch/x86/kernel/tsc.c
===================================================================
--- linux-2.6.29-rc2.orig/arch/x86/kernel/tsc.c	2009-01-27 12:21:20.000000000 +0900
+++ linux-2.6.29-rc2/arch/x86/kernel/tsc.c	2009-01-27 12:24:18.000000000 +0900
@@ -161,7 +161,7 @@ static unsigned long calc_pmtimer_ref(u6
 		return ULONG_MAX;

 	if (pm2 < pm1)
-		pm2 += (u64)ACPI_PM_OVRRUN;
+		pm2 += (u64)ACPI_PM_OVERRUN;
 	pm2 -= pm1;
 	tmp = pm2 * 1000000000LL;
 	do_div(tmp, PMTMR_TICKS_PER_SEC);
Index: linux-2.6.29-rc2/include/linux/acpi_pmtmr.h
===================================================================
--- linux-2.6.29-rc2.orig/include/linux/acpi_pmtmr.h	2009-01-27 12:21:20.000000000 +0900
+++ linux-2.6.29-rc2/include/linux/acpi_pmtmr.h	2009-01-27 12:24:18.000000000 +0900
@@ -10,7 +10,7 @@
 #define ACPI_PM_MASK CLOCKSOURCE_MASK(24)

 /* Overrun value */
-#define ACPI_PM_OVRRUN	(1<<24)
+#define ACPI_PM_OVERRUN	(1<<24)

 #ifdef CONFIG_X86_PM_TIMER


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-01-28  2:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-27  8:13 [PATCH 0/3] fix debug message of LOCAL APIC calibration Yasuaki Ishimatsu
2009-01-27  8:21 ` [PATCH 1/3] fix debug message of CPU clock speed Yasuaki Ishimatsu
2009-01-27 12:53   ` Ingo Molnar
2009-01-28  2:33     ` Yasuaki Ishimatsu
2009-01-27  8:22 ` [PATCH 2/3] unify PM-Timer messages Yasuaki Ishimatsu
2009-01-27  8:24 ` [PATCH 3/3] fix typo "ACPI_PM_OVRRUN" -> "ACPI_PM_OVERRUN" Yasuaki Ishimatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox