[PATCH][trivial] line up 'ESR value before/after enabling vector' messages

[PATCH][trivial] line up 'ESR value before/after enabling vector' messages

[Jesper Juhl]

Here's a very trivial patch to make the values in these messages line up 
nicely :

ESR value before enabling vector: 00000000
ESR value after enabling vector: 00000000

Much nicer to read when they line up like so : 

ESR value before enabling vector: 00000000
ESR value after enabling vector:  00000000

We got the "CPU: After ... caps:" messages nicely lined up recently, now I 
figured it was time for these ones as well..

Patches for both i386 and x86_64 below. Both diffs where created against 
2.6.8-rc3-mm1

Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>

diff -up linux-2.6.8-rc3-mm1-orig/arch/i386/kernel/apic.c linux-2.6.8-rc3-mm1/arch/i386/kernel/apic.c
--- linux-2.6.8-rc3-mm1-orig/arch/i386/kernel/apic.c	2004-08-05 21:48:50.000000000 +0200
+++ linux-2.6.8-rc3-mm1/arch/i386/kernel/apic.c	2004-08-11 01:40:56.000000000 +0200
@@ -473,7 +473,7 @@ void __init setup_local_APIC (void)
 			apic_write(APIC_ESR, 0);
 		value = apic_read(APIC_ESR);
 		apic_printk(APIC_VERBOSE, "ESR value after enabling vector:"
-				" %08lx\n", value);
+				"  %08lx\n", value);
 	} else {
 		if (esr_disable)	
 			/* 

diff -up linux-2.6.8-rc3-mm1-orig/arch/x86_64/kernel/apic.c linux-2.6.8-rc3-mm1/arch/x86_64/kernel/apic.c
--- linux-2.6.8-rc3-mm1-orig/arch/x86_64/kernel/apic.c	2004-08-05 21:48:52.000000000 +0200
+++ linux-2.6.8-rc3-mm1/arch/x86_64/kernel/apic.c	2004-08-11 01:41:52.000000000 +0200
@@ -421,7 +421,7 @@ void __init setup_local_APIC (void)
 		if (maxlvt > 3)
 			apic_write(APIC_ESR, 0);
 		value = apic_read(APIC_ESR);
-		Dprintk("ESR value after enabling vector: %08x\n", value);
+		Dprintk("ESR value after enabling vector:  %08x\n", value);
 	} else {
 		if (esr_disable)	
 			/* 


--
Jesper Juhl

Re: [PATCH][trivial] line up 'ESR value before/after enabling vector' messages

[Ingo Molnar]

* Jesper Juhl <juhl-lkml@dif.dk> wrote:

> -				" %08lx\n", value);
> +				"  %08lx\n", value);

> -		Dprintk("ESR value after enabling vector: %08x\n", value);
> +		Dprintk("ESR value after enabling vector:  %08x\n", value);

Signed-off-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH][trivial] line up 'ESR value before/after enabling vector' messages

[Dave Jones]

On Wed, Aug 11, 2004 at 08:23:14AM +0200, Ingo Molnar wrote:
 > 
 > * Jesper Juhl <juhl-lkml@dif.dk> wrote:
 > 
 > > -				" %08lx\n", value);
 > > +				"  %08lx\n", value);
 > 
 > > -		Dprintk("ESR value after enabling vector: %08x\n", value);
 > > +		Dprintk("ESR value after enabling vector:  %08x\n", value);
 > 
 > Signed-off-by: Ingo Molnar <mingo@elte.hu>

Has this printk actually been useful ? ever ?
I notice a majority of the time, it never changes too.
If it is useful, how about changing it so it prints something
if the value changed ?  (Something like patch below maybe?)
Or is possible for the APIC to lock up between the two printk's ?

		Dave


Only print out the ESR value if it changes after enabling vector.

Signed-off-by: Dave Jones <davej@redhat.com>

--- 1/arch/i386/kernel/apic.c~	2004-08-11 11:43:32.022485536 +0100
+++ 2/arch/i386/kernel/apic.c	2004-08-11 11:45:10.824465352 +0100
@@ -335,7 +335,7 @@
 
 void __init setup_local_APIC (void)
 {
-	unsigned long value, ver, maxlvt;
+	unsigned long oldvalue, value, ver, maxlvt;
 
 	/* Pound the ESR really hard over the head with a big hammer - mbligh */
 	if (esr_disable) {
@@ -449,9 +449,7 @@
 		maxlvt = get_maxlvt();
 		if (maxlvt > 3)		/* Due to the Pentium erratum 3AP. */
 			apic_write(APIC_ESR, 0);
-		value = apic_read(APIC_ESR);
-		printk("ESR value before enabling vector: %08lx\n", value);
-
+		oldvalue = apic_read(APIC_ESR);
 		value = ERROR_APIC_VECTOR;      // enables sending errors
 		apic_write_around(APIC_LVTERR, value);
 		/*
@@ -460,7 +458,9 @@
 		if (maxlvt > 3)
 			apic_write(APIC_ESR, 0);
 		value = apic_read(APIC_ESR);
-		printk("ESR value after enabling vector: %08lx\n", value);
+		if (value != oldvalue)
+			printk("ESR value before enabling vector: %08lx  after: %08lx\n",
+							oldvalue, value);
 	} else {
 		if (esr_disable)	
 			/* 

Re: [PATCH][trivial] line up 'ESR value before/after enabling vector' messages

[Ingo Molnar]

* Dave Jones <davej@redhat.com> wrote:

> Has this printk actually been useful ? ever ? I notice a majority of
> the time, it never changes too. If it is useful, how about changing it
> so it prints something if the value changed ?  (Something like patch
> below maybe?) Or is possible for the APIC to lock up between the two
> printk's ?

it was used to debug secondary-CPU startup problems many years ago. It
is also useful to see whether it's a real APIC we got there. I'd suggest
to remove the message altogether.

	Ingo