Re: [PATCH, v2] x86: Fix printk call in print_local_apic()

From: Yinghai Lu
Date: Thu Jul 02 2009 - 02:40:32 EST


Ingo Molnar wrote:
> * Ingo Molnar <mingo@xxxxxxx> wrote:
>
>>> - printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
>>> + printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n");
>>> for (i = 0; i < 8; i++) {
>>> + char bin[33];
>>> v = apic_read(base + i*0x10);
>>> +
>>> + /* Do we really want to print out LSB first? */
>> We definitely want MSB first - i'll flip around the order.
>
> in fact i dont remember ever having relied on the bitfield nature of
> that printout. Since this is uncommon debug code, printing the plain
> hexa value is more than enough.
>
> In fact we can compact it down to a single line:
>
> 0123456701234567012345670123456701234567012345670123456701234567
>
> instead of 4 lines of bitfields.
>
> So i've done the patch below that looks quite a bit simpler. Mind
> testing it, does it fix all the printout artifacts you've seen?
>
> Ingo
>
> ------------------>
>>From ef611b322dc9a917c71f28f9498dfff0d3949779 Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <mingo@xxxxxxx>
> Date: Thu, 2 Jul 2009 08:26:20 +0200
> Subject: [PATCH] x86: Fix printk call in print_local_apic()
>
> Instead of this:
>
> [ 75.690022] <7>printing local APIC contents on CPU#0/0:
> [ 75.704406] ... APIC ID: 00000000 (0)
> [ 75.707905] ... APIC VERSION: 00060015
> [ 75.722551] ... APIC TASKPRI: 00000000 (00)
> [ 75.725473] ... APIC PROCPRI: 00000000
> [ 75.728592] ... APIC LDR: 00000001
> [ 75.742137] ... APIC SPIV: 000001ff
> [ 75.744101] ... APIC ISR field:
> [ 75.746648] 0123456789abcdef0123456789abcdef
> [ 75.746649] <7>00000000000000000000000000000000
>
> Improve the code to be saner and simpler and just print out
> the bitfield in a single line using hexa values - not as a
> (rather pointless) binary bitfield.
>
> Partially reused Linus's initial fix for this.
>
> Reported-and-Tested-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> LKML-Reference: <4A4C43BC.90506@xxxxxxxxxx>
> Signed-off-by: Ingo Molnar <mingo@xxxxxxx>
> ---
> arch/x86/kernel/apic/io_apic.c | 25 ++++++++++---------------
> 1 files changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 4d0216f..e32e453 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1716,25 +1716,19 @@ __apicdebuginit(void) print_IO_APIC(void)
> return;
> }
>
> -__apicdebuginit(void) print_APIC_bitfield(int base)
> +__apicdebuginit(void) print_APIC_(int base)

?


> {
> - unsigned int v;
> int i, j;
>
> - if (apic_verbosity == APIC_QUIET)
> + if (apic_verbosity != APIC_QUIET)

is this one reversed?

> return;
>
> - printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
> - for (i = 0; i < 8; i++) {
> - v = apic_read(base + i*0x10);
> - for (j = 0; j < 32; j++) {
> - if (v & (1<<j))
> - printk("1");
> - else
> - printk("0");
> - }
> - printk("\n");
> - }
> + printk(KERN_DEBUG);
> +
> + for (i = 0; i < 8; i++)
> + printk(KERN_CONT "%08x", i, apic_read(base + i*0x10));
> +
> + printk(KERN_CONT "\n");
> }
>
> __apicdebuginit(void) print_local_APIC(void *dummy)
> @@ -1745,7 +1739,8 @@ __apicdebuginit(void) print_local_APIC(void *dummy)
> if (apic_verbosity == APIC_QUIET)
> return;
>
> - printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
> + printk(KERN_DEBUG "\n");
> + printk(KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
> smp_processor_id(), hard_smp_processor_id());
> v = apic_read(APIC_ID);
> printk(KERN_INFO "... APIC ID: %08x (%01x)\n", v, read_apic_id());

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/