Re: [PATCH] x86: kill arch/x86/kernel/mpparse.c debugging printk.

[Rene Herman <rene.herman@keyaccess.nl>]

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On 11-08-08 14:20, Ingo Molnar wrote:

>> From: Rene Herman <rene.herman@keyaccess.nl>
>> Date: Thu, 7 Aug 2008 01:50:35 +0200
>> Subject: [PATCH] x86: kill arch/x86/kernel/mpparse.c debugging printk.
>>
>> commit 11a62a056093a7f25f1595fbd8bd5f93559572b6 turns some formerly 
>> nopped debugging printks in arch/x86/kernel/mppparse.c into regular 
>> ones. The one at the top of smp_scan_config() in particular also 
>> prints on !CONFIG_SMP/CONFIG_X86_LOCAL_APIC kernels and UP machines 
>> without anything resembling MP tables which makes their lowly UP 
>> owners wonder...
>>
>> given that it was up to this point also not considered valuable 
>> user-level information, let's just kill that one.
> 
> hm, i found it useful in the past in about 2-3 cases.
> 
> How about a patch that makes the printout depend on apic=debug ? That 
> way the message can still be there in case of bugreports that somehow 
> deal with SMP or APIC bugs (without having to recompile the kernel).
> 
> The way to make the printout depend on apic=debug/verbose is to do 
> something like this:
> 
> 	apic_printk(APIC_VERBOSE, "Scan SMP from %p for %ld bytes.\n", bp, length);
> 
> Would you mind to send a patch for that?

I wouldn't. Like this? This turns the printk's that used to be Dprintk's 
into apic_printk's.

I am myself only interested in the one at the top of smp_scan_config() 
(it made me think I had misconfigured something upon all of a sudden 
seeing SMP printk's on my UP machine on 2.6.27-rc) but I guess this is 
the more complete version.

One problem; on 32-bit, "apic=" is a __setup() param and isn't actually 
early enough for us here so this needs it turned into an early_param() 
(followup).

Rene.

[-- Attachment #2: 0001-x86-make-arch-x86-kernel-mpparse.c-debugging-printk.patch --]
[-- Type: text/plain, Size: 3250 bytes --]

>From 3d6ab02d08c3597cd24581968dd0b41f3c264716 Mon Sep 17 00:00:00 2001
From: Rene Herman <rene.herman@gmail.com>
Date: Mon, 11 Aug 2008 17:20:42 +0200
Subject: [PATCH] x86: make arch/x86/kernel/mpparse.c debugging printk's apic_printk's

commit 11a62a056093a7f25f1595fbd8bd5f93559572b6 turns some formerly
nopped debugging printks in arch/x86/kernel/mppparse.c into regular
ones. The one at the top of smp_scan_config() in particular also
prints on !CONFIG_SMP/CONFIG_X86_LOCAL_APIC kernels and UP machines
without anything resembling MP tables which makes their lowly UP
owners wonder...

Turn the former Dprintk()s into apic_printk()s instead meaning that
their printing is dependent on passing the apic=verbose (or =debug)
command line param.

On 32-bit, "apic" is a __setup() param which isn't early enough
for this code and therefore needs a followup changing it into an
early_param(). On 64-bit, it already is.

Signed-off-by: Rene Herman <rene.herman@gmail.com>
---
 arch/x86/kernel/mpparse.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
index 6ae005c..6780905 100644
--- a/arch/x86/kernel/mpparse.c
+++ b/arch/x86/kernel/mpparse.c
@@ -83,7 +83,7 @@ static void __init MP_bus_info(struct mpc_config_bus *m)
 	if (x86_quirks->mpc_oem_bus_info)
 		x86_quirks->mpc_oem_bus_info(m, str);
 	else
-		printk(KERN_INFO "Bus #%d is %s\n", m->mpc_busid, str);
+		apic_printk(APIC_VERBOSE, "Bus #%d is %s\n", m->mpc_busid, str);
 
 #if MAX_MP_BUSSES < 256
 	if (m->mpc_busid >= MAX_MP_BUSSES) {
@@ -154,7 +154,7 @@ static void __init MP_ioapic_info(struct mpc_config_ioapic *m)
 
 static void print_MP_intsrc_info(struct mpc_config_intsrc *m)
 {
-	printk(KERN_CONT "Int: type %d, pol %d, trig %d, bus %02x,"
+	apic_printk(APIC_VERBOSE, "Int: type %d, pol %d, trig %d, bus %02x,"
 		" IRQ %02x, APIC ID %x, APIC INT %02x\n",
 		m->mpc_irqtype, m->mpc_irqflag & 3,
 		(m->mpc_irqflag >> 2) & 3, m->mpc_srcbus,
@@ -163,7 +163,7 @@ static void print_MP_intsrc_info(struct mpc_config_intsrc *m)
 
 static void __init print_mp_irq_info(struct mp_config_intsrc *mp_irq)
 {
-	printk(KERN_CONT "Int: type %d, pol %d, trig %d, bus %02x,"
+	apic_printk(APIC_VERBOSE, "Int: type %d, pol %d, trig %d, bus %02x,"
 		" IRQ %02x, APIC ID %x, APIC INT %02x\n",
 		mp_irq->mp_irqtype, mp_irq->mp_irqflag & 3,
 		(mp_irq->mp_irqflag >> 2) & 3, mp_irq->mp_srcbus,
@@ -235,7 +235,7 @@ static void __init MP_intsrc_info(struct mpc_config_intsrc *m)
 
 static void __init MP_lintsrc_info(struct mpc_config_lintsrc *m)
 {
-	printk(KERN_INFO "Lint: type %d, pol %d, trig %d, bus %02x,"
+	apic_printk(APIC_VERBOSE, "Lint: type %d, pol %d, trig %d, bus %02x,"
 		" IRQ %02x, APIC ID %x, APIC LINT %02x\n",
 		m->mpc_irqtype, m->mpc_irqflag & 3,
 		(m->mpc_irqflag >> 2) & 3, m->mpc_srcbusid,
@@ -695,7 +695,8 @@ static int __init smp_scan_config(unsigned long base, unsigned long length,
 	unsigned int *bp = phys_to_virt(base);
 	struct intel_mp_floating *mpf;
 
-	printk(KERN_DEBUG "Scan SMP from %p for %ld bytes.\n", bp, length);
+	apic_printk(APIC_VERBOSE, "Scan SMP from %p for %ld bytes.\n",
+			bp, length);
 	BUILD_BUG_ON(sizeof(*mpf) != 16);
 
 	while (length > 0) {
-- 
1.5.5