[Kernel-janitors] [PATCH] IO-APIC debug message reduction at

[Kernel-janitors] [PATCH] IO-APIC debug message reduction at

[Mark Broadbent]

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

Hi All,

Below is a patch that reduces the amount of debug information given out
by the IO-APIC initialisation.

Comments welcome, please be gentle as this is my first patch.

Thanks
Mark



[-- Attachment #2: ioapic-output-reduction.patch --]
[-- Type: text/x-patch, Size: 7873 bytes --]

--- linux-2.6.7/arch/i386/kernel/io_apic.c	2004-07-06 21:07:29.000000000 +0100
+++ linux/arch/i386/kernel/io_apic.c	2004-07-06 21:15:26.000000000 +0100
@@ -84,6 +84,17 @@
 #define vector_to_irq(vector)	(vector)
 #endif
 
+#define IOAPIC_QUIET   (0)
+#define IOAPIC_VERBOSE (1)
+#define IOAPIC_DEBUG   (2)
+
+/*
+ * Define the default level of output to be very little
+ * This can be turned up by using apic=verbose for more
+ * information and apic=debug for _lots_ of information.
+ */
+static int ioapic_verbosity = 0;
+
 /*
  * The common case is 1:1 IRQ<->pin mappings. Sometimes there are
  * shared ISA-space IRQs, so we have to support them. We are super
@@ -734,6 +745,26 @@
 
 __setup("noapic", ioapic_setup);
 
+static int __init ioapic_set_verbosity(char *str)
+{
+	if (strcmp("debug", str) == 0)
+	{
+		ioapic_verbosity = IOAPIC_DEBUG;
+	}
+	else if (strcmp("verbose", str) == 0)
+	{
+		ioapic_verbosity = IOAPIC_VERBOSE;
+	}
+	else
+	{
+		printk(KERN_WARNING "IO-APIC Verbosity level %s not recognised, use ioapic=verbose or ioapic=debug", str);
+	}
+
+	return 0;
+}
+
+__setup("ioapic=", ioapic_set_verbosity);
+
 static int __init ioapic_pirq_setup(char *str)
 {
 	int i, max;
@@ -745,13 +776,16 @@
 		pirq_entries[i] = -1;
 
 	pirqs_enabled = 1;
-	printk(KERN_INFO "PIRQ redirection, working around broken MP-BIOS.\n");
+
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "PIRQ redirection, working around broken MP-BIOS.\n");
 	max = MAX_PIRQS;
 	if (ints[0] < MAX_PIRQS)
 		max = ints[0];
 
 	for (i = 0; i < max; i++) {
-		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
+		if (ioapic_verbosity > IOAPIC_QUIET)
+			printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
 		/*
 		 * PIRQs are mapped upside down, usually.
 		 */
@@ -1122,12 +1156,13 @@
 	 */
 	if ((pin >= 16) && (pin <= 23)) {
 		if (pirq_entries[pin-16] != -1) {
-			if (!pirq_entries[pin-16]) {
+			if (!pirq_entries[pin-16] && ioapic_verbosity > IOAPIC_QUIET) {
 				printk(KERN_DEBUG "disabling PIRQ%d\n", pin-16);
 			} else {
 				irq = pirq_entries[pin-16];
-				printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
-						pin-16, irq);
+				if (ioapic_verbosity > IOAPIC_QUIET)
+					printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
+						   pin-16, irq);
 			}
 		}
 	}
@@ -1216,7 +1251,8 @@
 	int apic, pin, idx, irq, first_notcon = 1, vector;
 	unsigned long flags;
 
-	printk(KERN_DEBUG "init IO_APIC IRQs\n");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_DEBUG "init IO_APIC IRQs\n");
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
 	for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
@@ -1235,9 +1271,10 @@
 		idx = find_irq_entry(apic,pin,mp_INT);
 		if (idx == -1) {
 			if (first_notcon) {
-				printk(KERN_DEBUG " IO-APIC (apicid-pin) %d-%d", mp_ioapics[apic].mpc_apicid, pin);
+				if (ioapic_verbosity > IOAPIC_QUIET)
+					printk(KERN_DEBUG " IO-APIC (apicid-pin) %d-%d", mp_ioapics[apic].mpc_apicid, pin);
 				first_notcon = 0;
-			} else
+			} else if (ioapic_verbosity > IOAPIC_QUIET)
 				printk(", %d-%d", mp_ioapics[apic].mpc_apicid, pin);
 			continue;
 		}
@@ -1278,7 +1315,7 @@
 	}
 	}
 
-	if (!first_notcon)
+	if (!first_notcon && ioapic_verbosity > IOAPIC_QUIET)
 		printk(" not connected.\n");
 }
 
@@ -1339,6 +1376,9 @@
 	union IO_APIC_reg_03 reg_03;
 	unsigned long flags;
 
+ 	if (ioapic_verbosity == IOAPIC_QUIET)
+ 		return;
+ 	
  	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
 	for (i = 0; i < nr_ioapics; i++)
 		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1476,6 +1516,9 @@
 	unsigned int v;
 	int i, j;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+	
 	printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
 	for (i = 0; i < 8; i++) {
 		v = apic_read(base + i*0x10);
@@ -1493,6 +1536,9 @@
 {
 	unsigned int v, ver, maxlvt;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+	
 	printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
 		smp_processor_id(), hard_smp_processor_id());
 	v = apic_read(APIC_ID);
@@ -1580,6 +1626,9 @@
 	unsigned int v;
 	unsigned long flags;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+	
 	printk(KERN_DEBUG "\nprinting PIC contents\n");
 
 	spin_lock_irqsave(&i8259A_lock, flags);
@@ -2085,11 +2134,13 @@
 	 * is from Maciej W. Rozycki - so we do not have to EOI from
 	 * the NMI handler or the timer interrupt.
 	 */ 
-	printk(KERN_INFO "activating NMI Watchdog ...");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "activating NMI Watchdog ...");
 
 	on_each_cpu(enable_NMI_through_LVT0, NULL, 1, 1);
 
-	printk(" done.\n");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(" done.\n");
 }
 
 /*
@@ -2187,7 +2238,8 @@
 	pin1 = find_isa_irq_pin(0, mp_INT);
 	pin2 = find_isa_irq_pin(0, mp_ExtINT);
 
-	printk(KERN_INFO "..TIMER: vector=0x%02X pin1=%d pin2=%d\n", vector, pin1, pin2);
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "..TIMER: vector=0x%02X pin1=%d pin2=%d\n", vector, pin1, pin2);
 
 	if (pin1 != -1) {
 		/*
@@ -2207,15 +2259,19 @@
 		printk(KERN_ERR "..MP-BIOS bug: 8254 timer not connected to IO-APIC\n");
 	}
 
-	printk(KERN_INFO "...trying to set up timer (IRQ0) through the 8259A ... ");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "...trying to set up timer (IRQ0) through the 8259A ... ");
+	
 	if (pin2 != -1) {
-		printk("\n..... (found pin %d) ...", pin2);
+		if (ioapic_verbosity > IOAPIC_QUIET)
+			printk("\n..... (found pin %d) ...", pin2);
 		/*
 		 * legacy devices should be connected to IO APIC #0
 		 */
 		setup_ExtINT_IRQ0_pin(pin2, vector);
 		if (timer_irq_works()) {
-			printk("works.\n");
+			if (ioapic_verbosity > IOAPIC_QUIET)
+				printk("works.\n");
 			if (pin1 != -1)
 				replace_pin_at_irq(0, 0, pin1, 0, pin2);
 			else
@@ -2231,14 +2287,16 @@
 		 */
 		clear_IO_APIC_pin(0, pin2);
 	}
-	printk(" failed.\n");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(" failed.\n");
 
 	if (nmi_watchdog == NMI_IO_APIC) {
 		printk(KERN_WARNING "timer doesn't work through the IO-APIC - disabling NMI Watchdog!\n");
 		nmi_watchdog = 0;
 	}
 
-	printk(KERN_INFO "...trying to set up timer as Virtual Wire IRQ...");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "...trying to set up timer as Virtual Wire IRQ...");
 
 	disable_8259A_irq(0);
 	irq_desc[0].handler = &lapic_irq_type;
@@ -2246,13 +2304,16 @@
 	enable_8259A_irq(0);
 
 	if (timer_irq_works()) {
-		printk(" works.\n");
+		if (ioapic_verbosity > IOAPIC_QUIET)
+			printk(" works.\n");
 		return;
 	}
 	apic_write_around(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_FIXED | vector);
-	printk(" failed.\n");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(" failed.\n");
 
-	printk(KERN_INFO "...trying to set up timer as ExtINT IRQ...");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "...trying to set up timer as ExtINT IRQ...");
 
 	timer_ack = 0;
 	init_8259A(0);
@@ -2262,11 +2323,13 @@
 	unlock_ExtINT_logic();
 
 	if (timer_irq_works()) {
-		printk(" works.\n");
+		if (ioapic_verbosity > IOAPIC_QUIET)
+			printk(" works.\n");
 		return;
 	}
-	printk(" failed :(.\n");
-	panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(" failed :(.\n");
+	panic("IO-APIC + timer doesn't work! Boot with ioapic=debug and pester mingo@redhat.com");
 }
 
 /*
@@ -2287,7 +2350,8 @@
 	else
 		io_apic_irqs = ~PIC_IRQS;
 
-	printk("ENABLING IO-APIC IRQs\n");
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk("ENABLING IO-APIC IRQs\n");
 
 	/*
 	 * Set up IO-APIC IRQ routing.
@@ -2390,7 +2454,8 @@
 			panic("IOAPIC[%d]: Unable change apic_id!\n", ioapic);
 	}
 
-	printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
+	if (ioapic_verbosity > IOAPIC_QUIET)
+		printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
 
 	return apic_id;
 }

[-- Attachment #3: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reduction at

[Arnaldo Carvalho de Melo]

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

Em Tue, Jul 06, 2004 at 09:27:08PM +0100, Mark Broadbent escreveu:
> Hi All,
> 
> Below is a patch that reduces the amount of debug information given out
> by the IO-APIC initialisation.
> 
> Comments welcome, please be gentle as this is my first patch.
> 
> Thanks
> Mark
 
comments: 

> --- linux-2.6.7/arch/i386/kernel/io_apic.c	2004-07-06 21:07:29.000000000 +0100
> +++ linux/arch/i386/kernel/io_apic.c	2004-07-06 21:15:26.000000000 +0100
> @@ -84,6 +84,17 @@
>  #define vector_to_irq(vector)	(vector)
>  #endif
>  
> +#define IOAPIC_QUIET   (0)
> +#define IOAPIC_VERBOSE (1)
> +#define IOAPIC_DEBUG   (2)

What is the point of those parens?

> +
> +/*
> + * Define the default level of output to be very little
> + * This can be turned up by using apic=verbose for more
> + * information and apic=debug for _lots_ of information.
> + */
> +static int ioapic_verbosity = 0;
> +

no need to init static variables to zero, not doing so has the advantage of
moving this variable to the .bss kernel section.

>  /*
>   * The common case is 1:1 IRQ<->pin mappings. Sometimes there are
>   * shared ISA-space IRQs, so we have to support them. We are super
> @@ -734,6 +745,26 @@
>  
>  __setup("noapic", ioapic_setup);
>  
> +static int __init ioapic_set_verbosity(char *str)
> +{
> +	if (strcmp("debug", str) == 0)
> +	{
> +		ioapic_verbosity = IOAPIC_DEBUG;
> +	}

follow kernel coding style:

if (condition) {
	j--;
	bleh();
} else if (condition) {
	i++;
	blah();
}

Also why the braces?

> +	else if (strcmp("verbose", str) == 0)
> +	{
> +		ioapic_verbosity = IOAPIC_VERBOSE;
> +	}
> +	else
> +	{
> +		printk(KERN_WARNING "IO-APIC Verbosity level %s not recognised, use ioapic=verbose or ioapic=debug", str);
> +	}
> +
> +	return 0;
> +}
> +
> +__setup("ioapic=", ioapic_set_verbosity);
> +
>  static int __init ioapic_pirq_setup(char *str)
>  {
>  	int i, max;
> @@ -745,13 +776,16 @@
>  		pirq_entries[i] = -1;
>  
>  	pirqs_enabled = 1;
> -	printk(KERN_INFO "PIRQ redirection, working around broken MP-BIOS.\n");
> +
> +	if (ioapic_verbosity > IOAPIC_QUIET)
> +		printk(KERN_INFO "PIRQ redirection, working around broken MP-BIOS.\n");

Humm, why not having something like ioapic_printk(), just like all the
dprintk in lots of places instead of testing ioapic_verbosity all the time,
polluting kernel sources? Sure, there are some places where directly
testing against ioapic_verbosity is better, like on a function that only
prints information... wait, this is the definition of ioapic_printk() :-)

Stopping here.

Regards,

- Arnaldo

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Mark Broadbent]

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

Hi,

> > +static int ioapic_verbosity = 0;
> > +
>
> no need to init static variables to zero, not doing so has the 
> advantage of moving this variable to the .bss kernel section.

Does this mean it will get set to zero automagically or must it
be set explicitly in the code?

As for the other comments. I have removed unneeded parens around the if
statements and brackets around the defined constants.  I have used a
ioapic_printk define to replace the original printk except for the
functions labelled print_* that break out at function start, this is
done to reduce the size of the resulting object code (saves ~1.5kB).

Thanks for the comments, a revised patch is given below:


Index: linux-2.6.7/arch/i386/kernel/io_apic.c
===================================================================
--- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
+++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
@@ -84,7 +84,23 @@
 #define vector_to_irq(vector)	(vector)
 #endif
 
+#define IOAPIC_QUIET   0
+#define IOAPIC_VERBOSE 1
+#define IOAPIC_DEBUG   2
+
 /*
+ * Define the default level of output to be very little
+ * This can be turned up by using apic=verbose for more
+ * information and apic=debug for _lots_ of information.
+ */
+static int ioapic_verbosity;
+
+#define ioapic_printk(v, s, a...) do {       \
+		if ((v) >= ioapic_verbosity) \
+			printk(s, ##a);      \
+	} while (0)
+
+/*
  * The common case is 1:1 IRQ<->pin mappings. Sometimes there are
  * shared ISA-space IRQs, so we have to support them. We are super
  * fast in the common case, and fast for shared ISA-space IRQs.
@@ -745,13 +761,15 @@
 		pirq_entries[i] = -1;
 
 	pirqs_enabled = 1;
-	printk(KERN_INFO "PIRQ redirection, working around broken
MP-BIOS.\n");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"PIRQ redirection, working around broken MP-BIOS.\n");
 	max = MAX_PIRQS;
 	if (ints[0] < MAX_PIRQS)
 		max = ints[0];
 
 	for (i = 0; i < max; i++) {
-		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
+		ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
+				"... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
 		/*
 		 * PIRQs are mapped upside down, usually.
 		 */
@@ -762,6 +780,21 @@
 
 __setup("pirq=", ioapic_pirq_setup);
 
+static int __init ioapic_set_verbosity(char *str)
+{
+	if (strcmp("debug", str) == 0)
+		ioapic_verbosity = IOAPIC_DEBUG;
+	else if (strcmp("verbose", str) == 0)
+		ioapic_verbosity = IOAPIC_VERBOSE;
+	else
+		printk(KERN_WARNING "IO-APIC Verbosity level %s not recognised"
+				" use ioapic=verbose or ioapic=debug", str);
+
+	return 0;
+}
+
+__setup("ioapic=", ioapic_set_verbosity);
+
 /*
  * Find the IRQ entry number of a certain pin.
  */
@@ -881,7 +914,8 @@
 		unsigned int port = 0x4d0 + (irq >> 3);
 		return (inb(port) >> (irq & 7)) & 1;
 	}
-	printk(KERN_INFO "Broken MPtable reports ISA irq %d\n", irq);
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"Broken MPtable reports ISA irq %d\n", irq);
 	return 0;
 }
 
@@ -1123,10 +1157,12 @@
 	if ((pin >= 16) && (pin <= 23)) {
 		if (pirq_entries[pin-16] != -1) {
 			if (!pirq_entries[pin-16]) {
-				printk(KERN_DEBUG "disabling PIRQ%d\n", pin-16);
+				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG +						"disabling
PIRQ%d\n", pin-16);
 			} else {
 				irq = pirq_entries[pin-16];
-				printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
+				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG +						"using PIRQ%d ->
IRQ %d\n",
 						pin-16, irq);
 			}
 		}
@@ -1216,7 +1252,7 @@
 	int apic, pin, idx, irq, first_notcon = 1, vector;
 	unsigned long flags;
 
-	printk(KERN_DEBUG "init IO_APIC IRQs\n");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
 	for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
@@ -1235,10 +1271,14 @@
 		idx = find_irq_entry(apic,pin,mp_INT);
 		if (idx == -1) {
 			if (first_notcon) {
-				printk(KERN_DEBUG " IO-APIC (apicid-pin) %d-%d",
mp_ioapics[apic].mpc_apicid, pin);
+				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG
+						" IO-APIC (apicid-pin) %d-%d", 
+						mp_ioapics[apic].mpc_apicid,
+						pin);
 				first_notcon = 0;
 			} else
-				printk(", %d-%d", mp_ioapics[apic].mpc_apicid, pin);
+				ioapic_printk(IOAPIC_VERBOSE, ", %d-%d",
+					mp_ioapics[apic].mpc_apicid, pin);
 			continue;
 		}
 
@@ -1279,7 +1319,7 @@
 	}
 
 	if (!first_notcon)
-		printk(" not connected.\n");
+		ioapic_printk(IOAPIC_VERBOSE, " not connected.\n");
 }
 
 /*
@@ -1339,6 +1379,9 @@
 	union IO_APIC_reg_03 reg_03;
 	unsigned long flags;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
  	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
 	for (i = 0; i < nr_ioapics; i++)
 		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1476,6 +1519,9 @@
 	unsigned int v;
 	int i, j;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
 	for (i = 0; i < 8; i++) {
 		v = apic_read(base + i*0x10);
@@ -1493,6 +1539,9 @@
 {
 	unsigned int v, ver, maxlvt;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
 	printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
 		smp_processor_id(), hard_smp_processor_id());
 	v = apic_read(APIC_ID);
@@ -1580,6 +1629,9 @@
 	unsigned int v;
 	unsigned long flags;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "\nprinting PIC contents\n");
 
 	spin_lock_irqsave(&i8259A_lock, flags);
@@ -1734,8 +1786,9 @@
 		 * Read the right value from the MPC table and
 		 * write it into the ID register.
 	 	 */
-		printk(KERN_INFO "...changing IO-APIC physical APIC ID to %d ...",
-					mp_ioapics[apic].mpc_apicid);
+		ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"...changing IO-APIC physical APIC ID to %d ...",
+			mp_ioapics[apic].mpc_apicid);
 
 		reg_00.bits.ID = mp_ioapics[apic].mpc_apicid;
 		spin_lock_irqsave(&ioapic_lock, flags);
@@ -1751,7 +1804,7 @@
 		if (reg_00.bits.ID != mp_ioapics[apic].mpc_apicid)
 			panic("could not set ID!\n");
 		else
-			printk(" ok.\n");
+			ioapic_printk(IOAPIC_VERBOSE, " ok.\n");
 	}
 }
 #else
@@ -2085,11 +2138,11 @@
 	 * is from Maciej W. Rozycki - so we do not have to EOI from
 	 * the NMI handler or the timer interrupt.
 	 */ 
-	printk(KERN_INFO "activating NMI Watchdog ...");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO "activating NMI Watchdog
...");
 
 	on_each_cpu(enable_NMI_through_LVT0, NULL, 1, 1);
 
-	printk(" done.\n");
+	ioapic_printk(IOAPIC_VERBOSE, " done.\n");
 }
 
 /*
@@ -2187,7 +2240,8 @@
 	pin1 = find_isa_irq_pin(0, mp_INT);
 	pin2 = find_isa_irq_pin(0, mp_ExtINT);
 
-	printk(KERN_INFO "..TIMER: vector=0x%02X pin1=%d pin2=%d\n", vector,
pin1, pin2);
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+		"..TIMER: vector=0x%02X pin1=%d pin2=%d\n", vector, pin1, pin2);
 
 	if (pin1 != -1) {
 		/*
@@ -2207,9 +2261,11 @@
 		printk(KERN_ERR "..MP-BIOS bug: 8254 timer not connected to
IO-APIC\n");
 	}
 
-	printk(KERN_INFO "...trying to set up timer (IRQ0) through the 8259A
... ");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+		"...trying to set up timer (IRQ0) through the 8259A ... ");
 	if (pin2 != -1) {
-		printk("\n..... (found pin %d) ...", pin2);
+		ioapic_printk(IOAPIC_VERBOSE, "\n..... (found pin %d) ...", 
+				pin2);
 		/*
 		 * legacy devices should be connected to IO APIC #0
 		 */
@@ -2238,7 +2294,8 @@
 		nmi_watchdog = 0;
 	}
 
-	printk(KERN_INFO "...trying to set up timer as Virtual Wire IRQ...");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"...trying to set up timer as Virtual Wire IRQ...");
 
 	disable_8259A_irq(0);
 	irq_desc[0].handler = &lapic_irq_type;
@@ -2246,13 +2303,14 @@
 	enable_8259A_irq(0);
 
 	if (timer_irq_works()) {
-		printk(" works.\n");
+		ioapic_printk(IOAPIC_VERBOSE, " works.\n");
 		return;
 	}
 	apic_write_around(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_FIXED |
vector);
-	printk(" failed.\n");
+	ioapic_printk(IOAPIC_VERBOSE, " failed.\n");
 
-	printk(KERN_INFO "...trying to set up timer as ExtINT IRQ...");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"...trying to set up timer as ExtINT IRQ...");
 
 	timer_ack = 0;
 	init_8259A(0);
@@ -2262,11 +2320,12 @@
 	unlock_ExtINT_logic();
 
 	if (timer_irq_works()) {
-		printk(" works.\n");
+		ioapic_printk(IOAPIC_VERBOSE, " works.\n");
 		return;
 	}
-	printk(" failed :(.\n");
-	panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
+	ioapic_printk(IOAPIC_VERBOSE, " failed :(.\n");
+	panic("IO-APIC + timer doesn't work! ensure you boot with "
+		"ioapic=debug and pester mingo@redhat.com");
 }
 
 /*
@@ -2390,7 +2449,8 @@
 			panic("IOAPIC[%d]: Unable change apic_id!\n", ioapic);
 	}
 
-	printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic,
apic_id);
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
 
 	return apic_id;
 }



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Randy.Dunlap]

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

On Wed, 07 Jul 2004 21:29:36 +0100 Mark Broadbent wrote:

| Hi,
| 
| > > +static int ioapic_verbosity = 0;
| > > +
| >
| > no need to init static variables to zero, not doing so has the 
| > advantage of moving this variable to the .bss kernel section.
| 
| Does this mean it will get set to zero automagically or must it
| be set explicitly in the code?

The BSS section of data is cleared to zero by kernel init...
(somewhere :)

| As for the other comments. I have removed unneeded parens around the if
| statements and brackets around the defined constants.  I have used a
| ioapic_printk define to replace the original printk except for the
| functions labelled print_* that break out at function start, this is
| done to reduce the size of the resulting object code (saves ~1.5kB).
| 
| Thanks for the comments, a revised patch is given below:
| 
| 
| Index: linux-2.6.7/arch/i386/kernel/io_apic.c
| ===================================================================
| --- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
| +++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
| @@ -84,7 +84,23 @@
|  #define vector_to_irq(vector)	(vector)
|  #endif
|  
| +#define IOAPIC_QUIET   0
| +#define IOAPIC_VERBOSE 1
| +#define IOAPIC_DEBUG   2
| +
|  /*
| + * Define the default level of output to be very little
| + * This can be turned up by using apic=verbose for more
| + * information and apic=debug for _lots_ of information.
| + */
| +static int ioapic_verbosity;
| +
| +#define ioapic_printk(v, s, a...) do {       \
| +		if ((v) >= ioapic_verbosity) \
| +			printk(s, ##a);      \
| +	} while (0)
| +
| +/*
|   * The common case is 1:1 IRQ<->pin mappings. Sometimes there are
|   * shared ISA-space IRQs, so we have to support them. We are super
|   * fast in the common case, and fast for shared ISA-space IRQs.
| @@ -745,13 +761,15 @@
|  		pirq_entries[i] = -1;
|  
|  	pirqs_enabled = 1;
| -	printk(KERN_INFO "PIRQ redirection, working around broken
| MP-BIOS.\n");
| +	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
| +			"PIRQ redirection, working around broken MP-BIOS.\n");
|  	max = MAX_PIRQS;
|  	if (ints[0] < MAX_PIRQS)
|  		max = ints[0];
|  
|  	for (i = 0; i < max; i++) {
| -		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
| +		ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
| +				"... PIRQ%d -> IRQ %d\n", i, ints[i+1]);

I'm a little confused about having both a verbosity level and a
printk message level here.  Did you make sure that your uses of
them are consistent?  Can the KERN_ level be removed or automatically
inserted in ioapic_printk() ?
Or maybe it's OK as is...

|  		/*
|  		 * PIRQs are mapped upside down, usually.
|  		 */
| @@ -1123,10 +1157,12 @@
|  	if ((pin >= 16) && (pin <= 23)) {
|  		if (pirq_entries[pin-16] != -1) {
|  			if (!pirq_entries[pin-16]) {
| -				printk(KERN_DEBUG "disabling PIRQ%d\n", pin-16);
| +				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG +						"disabling

Patch is mangled here....

| PIRQ%d\n", pin-16);
|  			} else {
|  				irq = pirq_entries[pin-16];
| -				printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
| +				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG +						"using PIRQ%d ->
| IRQ %d\n",

and here...

|  						pin-16, irq);
|  			}
|  		}
| @@ -2246,13 +2303,14 @@
|  	enable_8259A_irq(0);
|  
|  	if (timer_irq_works()) {
| -		printk(" works.\n");
| +		ioapic_printk(IOAPIC_VERBOSE, " works.\n");
|  		return;
|  	}
|  	apic_write_around(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_FIXED |
| vector);
| -	printk(" failed.\n");
| +	ioapic_printk(IOAPIC_VERBOSE, " failed.\n");

Print always ?

| -	printk(KERN_INFO "...trying to set up timer as ExtINT IRQ...");
| +	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
| +			"...trying to set up timer as ExtINT IRQ...");
|  
|  	timer_ack = 0;
|  	init_8259A(0);
| @@ -2262,11 +2320,12 @@
|  	unlock_ExtINT_logic();
|  
|  	if (timer_irq_works()) {
| -		printk(" works.\n");
| +		ioapic_printk(IOAPIC_VERBOSE, " works.\n");
|  		return;
|  	}
| -	printk(" failed :(.\n");
| -	panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
| +	ioapic_printk(IOAPIC_VERBOSE, " failed :(.\n");

I'd prefer to see this print even if booted with ioapic=quiet.

| +	panic("IO-APIC + timer doesn't work! ensure you boot with "
| +		"ioapic=debug and pester mingo@redhat.com");
|  }
|  
|  /*


--
~Randy

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Luiz Fernando N. Capitulino]

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

Em Wed, Jul 07, 2004 at 02:03:13PM -0700, Randy.Dunlap escreveu:

| | > > +static int ioapic_verbosity = 0;
| | > > +
| | >
| | > no need to init static variables to zero, not doing so has the 
| | > advantage of moving this variable to the .bss kernel section.
| | 
| | Does this mean it will get set to zero automagically or must it
| | be set explicitly in the code?
| 
| The BSS section of data is cleared to zero by kernel init...

 also, static and global variables are set to zero by the compiler,
right ?

-- 
Luiz Fernando N. Capitulino
<http://www.telecentros.sp.gov.br>

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Mark Broadbent]

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

All,

On Wed, 2004-07-07 at 22:03, Randy.Dunlap wrote:
> On Wed, 07 Jul 2004 21:29:36 +0100 Mark Broadbent wrote:
> 
> The BSS section of data is cleared to zero by kernel init...
> (somewhere :)

Found a reference to it (for my peace of mind) it's done in
arch/i386/kernel/head.S:70.  It has the following comment...

...
 * Clear BSS first so that there are no surprises...
...

> |     for (i = 0; i < max; i++) {
> | -           printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i,
ints[i+1]);
> | +           ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
> | +                           "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
> 
> I'm a little confused about having both a verbosity level and a
> printk message level here.  Did you make sure that your uses of
> them are consistent?  Can the KERN_ level be removed or automatically
> inserted in ioapic_printk() ?

This is done deliberately as the apic code uses a lot of constructs
like:

printk(KERN_DEBUG "Pin map...");
for (...) {
        printk("%d->%d ", pina, pinb);
printk("...\n"); 

and I've checked the usage is consistent.

> | -   printk(" failed.\n");
> | +   ioapic_printk(IOAPIC_VERBOSE, " failed.\n");
> 
> Print always ?

OK, changed.

> | -   printk(" failed :(.\n");
> | -   panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
> | +   ioapic_printk(IOAPIC_VERBOSE, " failed :(.\n");
> 
> I'd prefer to see this print even if booted with ioapic=quiet.

OK, changed.

New patch below

Thanks
Mark

Index: linux-2.6.7/arch/i386/kernel/io_apic.c
===================================================================
--- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
+++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
@@ -84,7 +84,23 @@
 #define vector_to_irq(vector)	(vector)
 #endif
 
+#define IOAPIC_QUIET   0
+#define IOAPIC_VERBOSE 1
+#define IOAPIC_DEBUG   2
+
 /*
+ * Define the default level of output to be very little
+ * This can be turned up by using apic=verbose for more
+ * information and apic=debug for _lots_ of information.
+ */
+static int ioapic_verbosity;
+
+#define ioapic_printk(v, s, a...) do {       \
+		if ((v) >= ioapic_verbosity) \
+			printk(s, ##a);      \
+	} while (0)
+
+/*
  * The common case is 1:1 IRQ<->pin mappings. Sometimes there are
  * shared ISA-space IRQs, so we have to support them. We are super
  * fast in the common case, and fast for shared ISA-space IRQs.
@@ -745,13 +761,15 @@
 		pirq_entries[i] = -1;
 
 	pirqs_enabled = 1;
-	printk(KERN_INFO "PIRQ redirection, working around broken
MP-BIOS.\n");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"PIRQ redirection, working around broken MP-BIOS.\n");
 	max = MAX_PIRQS;
 	if (ints[0] < MAX_PIRQS)
 		max = ints[0];
 
 	for (i = 0; i < max; i++) {
-		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
+		ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
+				"... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
 		/*
 		 * PIRQs are mapped upside down, usually.
 		 */
@@ -762,6 +780,21 @@
 
 __setup("pirq=", ioapic_pirq_setup);
 
+static int __init ioapic_set_verbosity(char *str)
+{
+	if (strcmp("debug", str) == 0)
+		ioapic_verbosity = IOAPIC_DEBUG;
+	else if (strcmp("verbose", str) == 0)
+		ioapic_verbosity = IOAPIC_VERBOSE;
+	else
+		printk(KERN_WARNING "IO-APIC Verbosity level %s not recognised"
+				" use ioapic=verbose or ioapic=debug", str);
+
+	return 0;
+}
+
+__setup("ioapic=", ioapic_set_verbosity);
+
 /*
  * Find the IRQ entry number of a certain pin.
  */
@@ -881,7 +914,8 @@
 		unsigned int port = 0x4d0 + (irq >> 3);
 		return (inb(port) >> (irq & 7)) & 1;
 	}
-	printk(KERN_INFO "Broken MPtable reports ISA irq %d\n", irq);
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"Broken MPtable reports ISA irq %d\n", irq);
 	return 0;
 }
 
@@ -1123,10 +1157,12 @@
 	if ((pin >= 16) && (pin <= 23)) {
 		if (pirq_entries[pin-16] != -1) {
 			if (!pirq_entries[pin-16]) {
-				printk(KERN_DEBUG "disabling PIRQ%d\n", pin-16);
+				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
+						"disabling PIRQ%d\n", pin-16);
 			} else {
 				irq = pirq_entries[pin-16];
-				printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
+				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
+						"using PIRQ%d -> IRQ %d\n",
 						pin-16, irq);
 			}
 		}
@@ -1216,7 +1252,7 @@
 	int apic, pin, idx, irq, first_notcon = 1, vector;
 	unsigned long flags;
 
-	printk(KERN_DEBUG "init IO_APIC IRQs\n");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
 	for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
@@ -1235,10 +1271,14 @@
 		idx = find_irq_entry(apic,pin,mp_INT);
 		if (idx == -1) {
 			if (first_notcon) {
-				printk(KERN_DEBUG " IO-APIC (apicid-pin) %d-%d",
mp_ioapics[apic].mpc_apicid, pin);
+				ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
+						" IO-APIC (apicid-pin) %d-%d", 
+						mp_ioapics[apic].mpc_apicid, 
+						pin);
 				first_notcon = 0;
 			} else
-				printk(", %d-%d", mp_ioapics[apic].mpc_apicid, pin);
+				ioapic_printk(IOAPIC_VERBOSE, ", %d-%d", 
+					mp_ioapics[apic].mpc_apicid, pin);
 			continue;
 		}
 
@@ -1279,7 +1319,7 @@
 	}
 
 	if (!first_notcon)
-		printk(" not connected.\n");
+		ioapic_printk(IOAPIC_VERBOSE, " not connected.\n");
 }
 
 /*
@@ -1339,6 +1379,9 @@
 	union IO_APIC_reg_03 reg_03;
 	unsigned long flags;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
  	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
 	for (i = 0; i < nr_ioapics; i++)
 		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1476,6 +1519,9 @@
 	unsigned int v;
 	int i, j;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
 	for (i = 0; i < 8; i++) {
 		v = apic_read(base + i*0x10);
@@ -1493,6 +1539,9 @@
 {
 	unsigned int v, ver, maxlvt;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
 	printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
 		smp_processor_id(), hard_smp_processor_id());
 	v = apic_read(APIC_ID);
@@ -1580,6 +1629,9 @@
 	unsigned int v;
 	unsigned long flags;
 
+	if (ioapic_verbosity == IOAPIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "\nprinting PIC contents\n");
 
 	spin_lock_irqsave(&i8259A_lock, flags);
@@ -1734,8 +1786,9 @@
 		 * Read the right value from the MPC table and
 		 * write it into the ID register.
 	 	 */
-		printk(KERN_INFO "...changing IO-APIC physical APIC ID to %d ...",
-					mp_ioapics[apic].mpc_apicid);
+		ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"...changing IO-APIC physical APIC ID to %d ...",
+			mp_ioapics[apic].mpc_apicid);
 
 		reg_00.bits.ID = mp_ioapics[apic].mpc_apicid;
 		spin_lock_irqsave(&ioapic_lock, flags);
@@ -1751,7 +1804,7 @@
 		if (reg_00.bits.ID != mp_ioapics[apic].mpc_apicid)
 			panic("could not set ID!\n");
 		else
-			printk(" ok.\n");
+			ioapic_printk(IOAPIC_VERBOSE, " ok.\n");
 	}
 }
 #else
@@ -2085,11 +2138,11 @@
 	 * is from Maciej W. Rozycki - so we do not have to EOI from
 	 * the NMI handler or the timer interrupt.
 	 */ 
-	printk(KERN_INFO "activating NMI Watchdog ...");
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO "activating NMI Watchdog
...");
 
 	on_each_cpu(enable_NMI_through_LVT0, NULL, 1, 1);
 
-	printk(" done.\n");
+	ioapic_printk(IOAPIC_VERBOSE, " done.\n");
 }
 
 /*
@@ -2266,7 +2319,8 @@
 		return;
 	}
 	printk(" failed :(.\n");
-	panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
+	panic("IO-APIC + timer doesn't work! ensure you boot with "
+		"ioapic=debug and pester mingo@redhat.com");
 }
 
 /*
@@ -2390,7 +2444,8 @@
 			panic("IOAPIC[%d]: Unable change apic_id!\n", ioapic);
 	}
 
-	printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic,
apic_id);
+	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
+			"IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
 
 	return apic_id;
 }



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Jeff Garzik]

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

Mark Broadbent wrote:
> All,
> 
> On Wed, 2004-07-07 at 22:03, Randy.Dunlap wrote:
> 
>>On Wed, 07 Jul 2004 21:29:36 +0100 Mark Broadbent wrote:
>>
>>The BSS section of data is cleared to zero by kernel init...
>>(somewhere :)
> 
> 
> Found a reference to it (for my peace of mind) it's done in
> arch/i386/kernel/head.S:70.  It has the following comment...
> 
> ...
>  * Clear BSS first so that there are no surprises...
> ...
> 
> 
>>|     for (i = 0; i < max; i++) {
>>| -           printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i,
> 
> ints[i+1]);
> 
>>| +           ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
>>| +                           "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
>>
>>I'm a little confused about having both a verbosity level and a
>>printk message level here.  Did you make sure that your uses of
>>them are consistent?  Can the KERN_ level be removed or automatically
>>inserted in ioapic_printk() ?
> 
> 
> This is done deliberately as the apic code uses a lot of constructs
> like:
> 
> printk(KERN_DEBUG "Pin map...");
> for (...) {
>         printk("%d->%d ", pina, pinb);
> printk("...\n"); 
> 
> and I've checked the usage is consistent.
> 
> 
>>| -   printk(" failed.\n");
>>| +   ioapic_printk(IOAPIC_VERBOSE, " failed.\n");
>>
>>Print always ?
> 
> 
> OK, changed.
> 
> 
>>| -   printk(" failed :(.\n");
>>| -   panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
>>| +   ioapic_printk(IOAPIC_VERBOSE, " failed :(.\n");
>>
>>I'd prefer to see this print even if booted with ioapic=quiet.
> 
> 
> OK, changed.
> 
> New patch below
> 
> Thanks
> Mark
> 
> Index: linux-2.6.7/arch/i386/kernel/io_apic.c
> ===================================================================
> --- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
> +++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
> @@ -84,7 +84,23 @@
>  #define vector_to_irq(vector)	(vector)
>  #endif
>  
> +#define IOAPIC_QUIET   0
> +#define IOAPIC_VERBOSE 1
> +#define IOAPIC_DEBUG   2
> +
>  /*
> + * Define the default level of output to be very little
> + * This can be turned up by using apic=verbose for more
> + * information and apic=debug for _lots_ of information.
> + */
> +static int ioapic_verbosity;
> +
> +#define ioapic_printk(v, s, a...) do {       \
> +		if ((v) >= ioapic_verbosity) \
> +			printk(s, ##a);      \
> +	} while (0)
> +
> +/*
>   * The common case is 1:1 IRQ<->pin mappings. Sometimes there are
>   * shared ISA-space IRQs, so we have to support them. We are super
>   * fast in the common case, and fast for shared ISA-space IRQs.
> @@ -745,13 +761,15 @@
>  		pirq_entries[i] = -1;
>  
>  	pirqs_enabled = 1;
> -	printk(KERN_INFO "PIRQ redirection, working around broken
> MP-BIOS.\n");
> +	ioapic_printk(IOAPIC_VERBOSE, KERN_INFO 
> +			"PIRQ redirection, working around broken MP-BIOS.\n");
>  	max = MAX_PIRQS;
>  	if (ints[0] < MAX_PIRQS)
>  		max = ints[0];
>  
>  	for (i = 0; i < max; i++) {
> -		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
> +		ioapic_printk(IOAPIC_VERBOSE, KERN_DEBUG 
> +				"... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
>  		/*
>  		 * PIRQs are mapped upside down, usually.
>  		 */
> @@ -762,6 +780,21 @@
>  
>  __setup("pirq=", ioapic_pirq_setup);
>  
> +static int __init ioapic_set_verbosity(char *str)
> +{
> +	if (strcmp("debug", str) == 0)
> +		ioapic_verbosity = IOAPIC_DEBUG;


IOAPIC_DEBUG should also activate the Dprintk() statements normally 
activated via editing include/asm-i386/apic.h:

#define APIC_DEBUG 0

#if APIC_DEBUG
#define Dprintk(x...) printk(x)
#else
#define Dprintk(x...)
#endif


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Mark Broadbent]

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

Hi,

> IOAPIC_DEBUG should also activate the Dprintk() statements normally
> activated via editing include/asm-i386/apic.h:
> 
> #define APIC_DEBUG 0

Expanded the output reduction to covert the Dprintk's in apic.c to
apic_printk.  Rediffed and attached.

Thanks
Mark

Index: linux-2.6.7/include/asm-i386/apic.h
===================================================================
--- linux-2.6.7/include/asm-i386/apic.h	(revision 1)
+++ linux-2.6.7/include/asm-i386/apic.h	(working copy)
@@ -7,14 +7,27 @@
 #include <asm/apicdef.h>
 #include <asm/system.h>
 
-#define APIC_DEBUG 0
-
-#if APIC_DEBUG
-#define Dprintk(x...) printk(x)
-#else
 #define Dprintk(x...)
-#endif
 
+/* 
+ * Debugging macros
+ */
+#define APIC_QUIET   0
+#define APIC_VERBOSE 1
+#define APIC_DEBUG   2
+
+/*
+ * Define the default level of output to be very little
+ * This can be turned up by using apic=verbose for more
+ * information and apic=debug for _lots_ of information.
+ * apic_verbosity is defined in apic.c
+ */
+#define apic_printk(v, s, a...) do {       \
+		if ((v) >= apic_verbosity) \
+			printk(s, ##a);    \
+	} while (0)
+
+
 #ifdef CONFIG_X86_LOCAL_APIC
 
 /*
Index: linux-2.6.7/arch/i386/kernel/io_apic.c
===================================================================
--- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
+++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
@@ -58,6 +58,9 @@
  */
 int nr_ioapic_registers[MAX_IO_APICS];
 
+/* APIC verbosity support */
+int apic_verbosity;
+
 /*
  * Rough estimation of how many shared IRQs there are, can
  * be changed anytime.
@@ -745,13 +748,15 @@
 		pirq_entries[i] = -1;
 
 	pirqs_enabled = 1;
-	printk(KERN_INFO "PIRQ redirection, working around broken
MP-BIOS.\n");
+	apic_printk(APIC_VERBOSE, KERN_INFO 
+			"PIRQ redirection, working around broken MP-BIOS.\n");
 	max = MAX_PIRQS;
 	if (ints[0] < MAX_PIRQS)
 		max = ints[0];
 
 	for (i = 0; i < max; i++) {
-		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
+		apic_printk(APIC_VERBOSE, KERN_DEBUG 
+				"... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
 		/*
 		 * PIRQs are mapped upside down, usually.
 		 */
@@ -812,8 +817,8 @@
 {
 	int apic, i, best_guess = -1;
 
-	Dprintk("querying PCI -> IRQ mapping bus:%d, slot:%d, pin:%d.\n",
-		bus, slot, pin);
+	apic_printk(APIC_DEBUG, "querying PCI -> IRQ mapping bus:%d, "
+		"slot:%d, pin:%d.\n", bus, slot, pin);
 	if (mp_bus_id_to_pci_bus[bus] == -1) {
 		printk(KERN_WARNING "PCI BIOS passed nonexistent PCI bus %d!\n",
bus);
 		return -1;
@@ -881,7 +886,8 @@
 		unsigned int port = 0x4d0 + (irq >> 3);
 		return (inb(port) >> (irq & 7)) & 1;
 	}
-	printk(KERN_INFO "Broken MPtable reports ISA irq %d\n", irq);
+	apic_printk(APIC_VERBOSE, KERN_INFO 
+			"Broken MPtable reports ISA irq %d\n", irq);
 	return 0;
 }
 
@@ -1123,10 +1129,12 @@
 	if ((pin >= 16) && (pin <= 23)) {
 		if (pirq_entries[pin-16] != -1) {
 			if (!pirq_entries[pin-16]) {
-				printk(KERN_DEBUG "disabling PIRQ%d\n", pin-16);
+				apic_printk(APIC_VERBOSE, KERN_DEBUG 
+						"disabling PIRQ%d\n", pin-16);
 			} else {
 				irq = pirq_entries[pin-16];
-				printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
+				apic_printk(APIC_VERBOSE, KERN_DEBUG 
+						"using PIRQ%d -> IRQ %d\n",
 						pin-16, irq);
 			}
 		}
@@ -1216,7 +1224,7 @@
 	int apic, pin, idx, irq, first_notcon = 1, vector;
 	unsigned long flags;
 
-	printk(KERN_DEBUG "init IO_APIC IRQs\n");
+	apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
 	for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
@@ -1235,10 +1243,14 @@
 		idx = find_irq_entry(apic,pin,mp_INT);
 		if (idx == -1) {
 			if (first_notcon) {
-				printk(KERN_DEBUG " IO-APIC (apicid-pin) %d-%d",
mp_ioapics[apic].mpc_apicid, pin);
+				apic_printk(APIC_VERBOSE, KERN_DEBUG 
+						" IO-APIC (apicid-pin) %d-%d", 
+						mp_ioapics[apic].mpc_apicid, 
+						pin);
 				first_notcon = 0;
 			} else
-				printk(", %d-%d", mp_ioapics[apic].mpc_apicid, pin);
+				apic_printk(APIC_VERBOSE, ", %d-%d", 
+					mp_ioapics[apic].mpc_apicid, pin);
 			continue;
 		}
 
@@ -1279,7 +1291,7 @@
 	}
 
 	if (!first_notcon)
-		printk(" not connected.\n");
+		apic_printk(APIC_VERBOSE, " not connected.\n");
 }
 
 /*
@@ -1339,6 +1351,9 @@
 	union IO_APIC_reg_03 reg_03;
 	unsigned long flags;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
  	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
 	for (i = 0; i < nr_ioapics; i++)
 		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1476,6 +1491,9 @@
 	unsigned int v;
 	int i, j;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
 	for (i = 0; i < 8; i++) {
 		v = apic_read(base + i*0x10);
@@ -1493,6 +1511,9 @@
 {
 	unsigned int v, ver, maxlvt;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
 	printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
 		smp_processor_id(), hard_smp_processor_id());
 	v = apic_read(APIC_ID);
@@ -1580,6 +1601,9 @@
 	unsigned int v;
 	unsigned long flags;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "\nprinting PIC contents\n");
 
 	spin_lock_irqsave(&i8259A_lock, flags);
@@ -1715,7 +1739,9 @@
 		} else {
 			physid_mask_t tmp;
 			tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
-			printk("Setting %d in the phys_id_present_map\n",
mp_ioapics[apic].mpc_apicid);
+			apic_printk(APIC_VERBOSE, "Setting %d in the "
+					"phys_id_present_map\n", 
+					mp_ioapics[apic].mpc_apicid);
 			physids_or(phys_id_present_map, phys_id_present_map, tmp);
 		}
 
@@ -1734,8 +1760,9 @@
 		 * Read the right value from the MPC table and
 		 * write it into the ID register.
 	 	 */
-		printk(KERN_INFO "...changing IO-APIC physical APIC ID to %d ...",
-					mp_ioapics[apic].mpc_apicid);
+		apic_printk(APIC_VERBOSE, KERN_INFO 
+			"...changing IO-APIC physical APIC ID to %d ...",
+			mp_ioapics[apic].mpc_apicid);
 
 		reg_00.bits.ID = mp_ioapics[apic].mpc_apicid;
 		spin_lock_irqsave(&ioapic_lock, flags);
@@ -1751,7 +1778,7 @@
 		if (reg_00.bits.ID != mp_ioapics[apic].mpc_apicid)
 			panic("could not set ID!\n");
 		else
-			printk(" ok.\n");
+			apic_printk(APIC_VERBOSE, " ok.\n");
 	}
 }
 #else
@@ -2085,11 +2112,11 @@
 	 * is from Maciej W. Rozycki - so we do not have to EOI from
 	 * the NMI handler or the timer interrupt.
 	 */ 
-	printk(KERN_INFO "activating NMI Watchdog ...");
+	apic_printk(APIC_VERBOSE, KERN_INFO "activating NMI Watchdog ...");
 
 	on_each_cpu(enable_NMI_through_LVT0, NULL, 1, 1);
 
-	printk(" done.\n");
+	apic_printk(APIC_VERBOSE, " done.\n");
 }
 
 /*
@@ -2266,7 +2293,8 @@
 		return;
 	}
 	printk(" failed :(.\n");
-	panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
+	panic("IO-APIC + timer doesn't work! ensure you boot with "
+		"ioapic=debug and pester mingo@redhat.com");
 }
 
 /*
@@ -2390,7 +2418,8 @@
 			panic("IOAPIC[%d]: Unable change apic_id!\n", ioapic);
 	}
 
-	printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic,
apic_id);
+	apic_printk(APIC_VERBOSE, KERN_INFO 
+			"IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
 
 	return apic_id;
 }
@@ -2456,9 +2485,10 @@
 
 	entry.vector = assign_irq_vector(irq);
 
-	Dprintk(KERN_DEBUG "IOAPIC[%d]: Set PCI routing entry (%d-%d -> 0x%x
-> "
-		"IRQ %d Mode:%i Active:%i)\n", ioapic, 
-		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq, edge_level,
active_high_low);
+	apic_printk(APIC_DEBUG, KERN_DEBUG "IOAPIC[%d]: Set PCI routing entry
"
+		"(%d-%d -> 0x%x -> IRQ %d Mode:%i Active:%i)\n", ioapic, 
+		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq, 
+		edge_level, active_high_low);
 
  	if (use_pci_vector() && !platform_legacy_irq(irq))
 		irq = IO_APIC_VECTOR(irq);
Index: linux-2.6.7/arch/i386/kernel/apic.c
===================================================================
--- linux-2.6.7/arch/i386/kernel/apic.c	(revision 1)
+++ linux-2.6.7/arch/i386/kernel/apic.c	(working copy)
@@ -40,6 +40,15 @@
 
 #include "io_ports.h"
 
+/* 
+ * Debug level
+ */
+extern int apic_verbosity;
+
+/* Ensure io_apic can see this level as well */
+EXPORT_SYMBOL(apic_verbosity);
+
+
 static void apic_pm_activate(void);
 
 void __init apic_intr_init(void)
@@ -163,7 +172,8 @@
 		 * PIC mode, enable APIC mode in the IMCR, i.e.
 		 * connect BSP's local APIC to INT and NMI lines.
 		 */
-		printk("leaving PIC mode, enabling APIC mode.\n");
+		apic_printk(APIC_VERBOSE, "leaving PIC mode, "
+				"enabling APIC mode.\n");
 		outb(0x70, 0x22);
 		outb(0x01, 0x23);
 	}
@@ -179,7 +189,8 @@
 		 * interrupts, including IPIs, won't work beyond
 		 * this point!  The only exception are INIT IPIs.
 		 */
-		printk("disabling APIC mode, entering PIC mode.\n");
+		apic_printk(APIC_VERBOSE, "disabling APIC mode, "
+				"entering PIC mode.\n");
 		outb(0x70, 0x22);
 		outb(0x00, 0x23);
 	}
@@ -220,10 +231,10 @@
 	 * The version register is read-only in a real APIC.
 	 */
 	reg0 = apic_read(APIC_LVR);
-	Dprintk("Getting VERSION: %x\n", reg0);
+	apic_printk(APIC_DEBUG, "Getting VERSION: %x\n", reg0);
 	apic_write(APIC_LVR, reg0 ^ APIC_LVR_MASK);
 	reg1 = apic_read(APIC_LVR);
-	Dprintk("Getting VERSION: %x\n", reg1);
+	apic_printk(APIC_DEBUG, "Getting VERSION: %x\n", reg1);
 
 	/*
 	 * The two version reads above should print the same
@@ -247,7 +258,7 @@
 	 * The ID register is read/write in a real APIC.
 	 */
 	reg0 = apic_read(APIC_ID);
-	Dprintk("Getting ID: %x\n", reg0);
+	apic_printk(APIC_DEBUG, "Getting ID: %x\n", reg0);
 
 	/*
 	 * The next two are just to see if we have sane values.
@@ -255,9 +266,9 @@
 	 * compatibility mode, but most boxes are anymore.
 	 */
 	reg0 = apic_read(APIC_LVT0);
-	Dprintk("Getting LVT0: %x\n", reg0);
+	apic_printk(APIC_DEBUG, "Getting LVT0: %x\n", reg0);
 	reg1 = apic_read(APIC_LVT1);
-	Dprintk("Getting LVT1: %x\n", reg1);
+	apic_printk(APIC_DEBUG, "Getting LVT1: %x\n", reg1);
 
 	return 1;
 }
@@ -269,7 +280,7 @@
 	 */
 	apic_wait_icr_idle();
 
-	Dprintk("Synchronizing Arb IDs.\n");
+	apic_printk(APIC_DEBUG, "Synchronizing Arb IDs.\n");
 	apic_write_around(APIC_ICR, APIC_DEST_ALLINC | APIC_INT_LEVELTRIG
 				| APIC_DM_INIT);
 }
@@ -417,10 +428,12 @@
 	value = apic_read(APIC_LVT0) & APIC_LVT_MASKED;
 	if (!smp_processor_id() && (pic_mode || !value)) {
 		value = APIC_DM_EXTINT;
-		printk("enabled ExtINT on CPU#%d\n", smp_processor_id());
+		apic_printk(APIC_VERBOSE, "enabled ExtINT on CPU#%d\n", 
+				smp_processor_id());
 	} else {
 		value = APIC_DM_EXTINT | APIC_LVT_MASKED;
-		printk("masked ExtINT on CPU#%d\n", smp_processor_id());
+		apic_printk(APIC_VERBOSE, "masked ExtINT on CPU#%d\n", 
+				smp_processor_id());
 	}
 	apic_write_around(APIC_LVT0, value);
 
@@ -440,7 +453,8 @@
 		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);
+		apic_printk(APIC_VERBOSE, "ESR value before enabling vector:"
+				" %08lx\n", value);
 
 		value = ERROR_APIC_VECTOR;      // enables sending errors
 		apic_write_around(APIC_LVTERR, value);
@@ -450,7 +464,8 @@
 		if (maxlvt > 3)
 			apic_write(APIC_ESR, 0);
 		value = apic_read(APIC_ESR);
-		printk("ESR value after enabling vector: %08lx\n", value);
+		apic_printk(APIC_VERBOSE, "ESR value after enabling vector:"
+				" %08lx\n", value);
 	} else {
 		if (esr_disable)	
 			/* 
@@ -625,6 +640,21 @@
 }
 __setup("lapic", lapic_enable);
 
+static int __init apic_set_verbosity(char *str)
+{
+	if (strcmp("debug", str) == 0)
+		apic_verbosity = APIC_DEBUG;
+	else if (strcmp("verbose", str) == 0)
+		apic_verbosity = APIC_VERBOSE;
+	else
+		printk(KERN_WARNING "APIC Verbosity level %s not recognised"
+				" use apic=verbose or apic=debug", str);
+
+	return 0;
+}
+
+__setup("apic=", apic_set_verbosity);
+
 static int __init detect_init_APIC (void)
 {
 	u32 h, l, features;
@@ -661,7 +691,8 @@
 		 */
 		rdmsr(MSR_IA32_APICBASE, l, h);
 		if (!(l & MSR_IA32_APICBASE_ENABLE)) {
-			printk("Local APIC disabled by BIOS -- reenabling.\n");
+			apic_printk(APIC_VERBOSE, "Local APIC disabled "
+					"by BIOS -- reenabling.\n");
 			l &= ~MSR_IA32_APICBASE_BASE;
 			l |= MSR_IA32_APICBASE_ENABLE | APIC_DEFAULT_PHYS_BASE;
 			wrmsr(MSR_IA32_APICBASE, l, h);
@@ -688,7 +719,7 @@
 	if (nmi_watchdog != NMI_NONE)
 		nmi_watchdog = NMI_LOCAL_APIC;
 
-	printk("Found and enabled local APIC!\n");
+	apic_printk(APIC_VERBOSE, "Found and enabled local APIC!\n");
 
 	apic_pm_activate();
 
@@ -715,7 +746,8 @@
 		apic_phys = mp_lapic_addr;
 
 	set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
-	Dprintk("mapped APIC to %08lx (%08lx)\n", APIC_BASE, apic_phys);
+	apic_printk(APIC_DEBUG, "mapped APIC to %08lx (%08lx)\n", APIC_BASE, 
+			apic_phys);
 
 	/*
 	 * Fetch the APIC ID of the BSP in case we have a
@@ -745,7 +777,8 @@
 				ioapic_phys = __pa(ioapic_phys);
 			}
 			set_fixmap_nocache(idx, ioapic_phys);
-			Dprintk("mapped IOAPIC to %08lx (%08lx)\n",
+			apic_printk(APIC_DEBUG, "mapped IOAPIC to "
+					"%08lx (%08lx)\n",
 					__fix_to_virt(idx), ioapic_phys);
 			idx++;
 		}
@@ -884,7 +917,7 @@
 	int i;
 	const int LOOPS = HZ/10;
 
-	printk("calibrating APIC timer ...\n");
+	apic_printk(APIC_VERBOSE, "calibrating APIC timer ...\n");
 
 	/*
 	 * Put whatever arbitrary (but long enough) timeout
@@ -929,11 +962,13 @@
 	result = (tt1-tt2)*APIC_DIVISOR/LOOPS;
 
 	if (cpu_has_tsc)
-		printk("..... CPU clock speed is %ld.%04ld MHz.\n",
+		apic_printk(APIC_VERBOSE, "..... CPU clock speed is "
+			"%ld.%04ld MHz.\n",
 			((long)(t2-t1)/LOOPS)/(1000000/HZ),
 			((long)(t2-t1)/LOOPS)%(1000000/HZ));
 
-	printk("..... host bus clock speed is %ld.%04ld MHz.\n",
+	apic_printk(APIC_VERBOSE, "..... host bus clock speed is "
+		"%ld.%04ld MHz.\n",
 		result/(1000000/HZ),
 		result%(1000000/HZ));
 
@@ -944,7 +979,7 @@
 
 void __init setup_boot_APIC_clock(void)
 {
-	printk("Using local APIC timer interrupts.\n");
+	apic_printk(APIC_VERBOSE, "Using local APIC timer interrupts.\n");
 	using_apic_timer = 1;
 
 	local_irq_disable();



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[maximilian attems]

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

On Sat, 10 Jul 2004, Mark Broadbent wrote:

> Hi,
> 
> > IOAPIC_DEBUG should also activate the Dprintk() statements normally
> > activated via editing include/asm-i386/apic.h:
> > 
> > #define APIC_DEBUG 0
> 
> Expanded the output reduction to covert the Dprintk's in apic.c to
> apic_printk.  Rediffed and attached.
> 
> Thanks
> Mark
> 
> Index: linux-2.6.7/arch/i386/kernel/io_apic.c
> ===================================================================
> --- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
> +++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
..
> @@ -1339,6 +1351,9 @@
>  	union IO_APIC_reg_03 reg_03;
>  	unsigned long flags;
>  
> +	if (apic_verbosity == APIC_QUIET)
> +		return;
> +		
>   	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
>  	for (i = 0; i < nr_ioapics; i++)
>  		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",

that looks scary, did you actually test your changes?
looks like a big change to print_IO_APIC()

please when rediffing please use the -p switch from diff(1)
as explained in Documentation/SubmittingPatches

> @@ -1476,6 +1491,9 @@
>  	unsigned int v;
>  	int i, j;
>  
> +	if (apic_verbosity == APIC_QUIET)
> +		return;
> +		
>  	printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
>  	for (i = 0; i < 8; i++) {
>  		v = apic_read(base + i*0x10);
> @@ -1493,6 +1511,9 @@
>  {
>  	unsigned int v, ver, maxlvt;
>  
> +	if (apic_verbosity == APIC_QUIET)
> +		return;
> +		
>  	printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
>  		smp_processor_id(), hard_smp_processor_id());
>  	v = apic_read(APIC_ID);
> @@ -1580,6 +1601,9 @@
>  	unsigned int v;
>  	unsigned long flags;
>  
> +	if (apic_verbosity == APIC_QUIET)
> +		return;
> +		
>  	printk(KERN_DEBUG "\nprinting PIC contents\n");
>  
>  	spin_lock_irqsave(&i8259A_lock, flags);
..

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Mark Broadbent]

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

On Sun, 2004-07-11 at 11:38, maximilian attems wrote:
> > @@ -1339,6 +1351,9 @@
> >  	union IO_APIC_reg_03 reg_03;
> >  	unsigned long flags;
> >  
> > +	if (apic_verbosity == APIC_QUIET)
> > +		return;
> > +		
> >   	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
> >  	for (i = 0; i < nr_ioapics; i++)
> >  		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
> 
> that looks scary, did you actually test your changes?
> looks like a big change to print_IO_APIC()

I seems to work fine, dmesg output from three runs with each output
level set available at http://www.wetlettuce.com/kernel

> please when rediffing please use the -p switch from diff(1)
> as explained in Documentation/SubmittingPatches

Ok, must have missed that - re-diff below with a minor typo fixed.

Thanks
Mark

Index: linux-2.6.7/include/asm-i386/apic.h
===================================================================
--- linux-2.6.7/include/asm-i386/apic.h	(revision 1)
+++ linux-2.6.7/include/asm-i386/apic.h	(working copy)
@@ -7,13 +7,26 @@
 #include <asm/apicdef.h>
 #include <asm/system.h>
 
-#define APIC_DEBUG 0
-
-#if APIC_DEBUG
-#define Dprintk(x...) printk(x)
-#else
 #define Dprintk(x...)
-#endif
+
+/* 
+ * Debugging macros
+ */
+#define APIC_QUIET   0
+#define APIC_VERBOSE 1
+#define APIC_DEBUG   2
+
+/*
+ * Define the default level of output to be very little
+ * This can be turned up by using apic=verbose for more
+ * information and apic=debug for _lots_ of information.
+ * apic_verbosity is defined in apic.c
+ */
+#define apic_printk(v, s, a...) do {       \
+		if ((v) <= apic_verbosity) \
+			printk(s, ##a);    \
+	} while (0)
+
 
 #ifdef CONFIG_X86_LOCAL_APIC
 
Index: linux-2.6.7/arch/i386/kernel/io_apic.c
===================================================================
--- linux-2.6.7/arch/i386/kernel/io_apic.c	(revision 1)
+++ linux-2.6.7/arch/i386/kernel/io_apic.c	(working copy)
@@ -58,6 +58,9 @@ int sis_apic_bug = -1;
  */
 int nr_ioapic_registers[MAX_IO_APICS];
 
+/* APIC verbosity support */
+int apic_verbosity;
+
 /*
  * Rough estimation of how many shared IRQs there are, can
  * be changed anytime.
@@ -745,13 +748,15 @@ static int __init ioapic_pirq_setup(char
 		pirq_entries[i] = -1;
 
 	pirqs_enabled = 1;
-	printk(KERN_INFO "PIRQ redirection, working around broken
MP-BIOS.\n");
+	apic_printk(APIC_VERBOSE, KERN_INFO 
+			"PIRQ redirection, working around broken MP-BIOS.\n");
 	max = MAX_PIRQS;
 	if (ints[0] < MAX_PIRQS)
 		max = ints[0];
 
 	for (i = 0; i < max; i++) {
-		printk(KERN_DEBUG "... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
+		apic_printk(APIC_VERBOSE, KERN_DEBUG 
+				"... PIRQ%d -> IRQ %d\n", i, ints[i+1]);
 		/*
 		 * PIRQs are mapped upside down, usually.
 		 */
@@ -812,8 +817,8 @@ int IO_APIC_get_PCI_irq_vector(int bus, 
 {
 	int apic, i, best_guess = -1;
 
-	Dprintk("querying PCI -> IRQ mapping bus:%d, slot:%d, pin:%d.\n",
-		bus, slot, pin);
+	apic_printk(APIC_DEBUG, "querying PCI -> IRQ mapping bus:%d, "
+		"slot:%d, pin:%d.\n", bus, slot, pin);
 	if (mp_bus_id_to_pci_bus[bus] == -1) {
 		printk(KERN_WARNING "PCI BIOS passed nonexistent PCI bus %d!\n",
bus);
 		return -1;
@@ -881,7 +886,8 @@ static int __init EISA_ELCR(unsigned int
 		unsigned int port = 0x4d0 + (irq >> 3);
 		return (inb(port) >> (irq & 7)) & 1;
 	}
-	printk(KERN_INFO "Broken MPtable reports ISA irq %d\n", irq);
+	apic_printk(APIC_VERBOSE, KERN_INFO 
+			"Broken MPtable reports ISA irq %d\n", irq);
 	return 0;
 }
 
@@ -1123,10 +1129,12 @@ static int pin_2_irq(int idx, int apic, 
 	if ((pin >= 16) && (pin <= 23)) {
 		if (pirq_entries[pin-16] != -1) {
 			if (!pirq_entries[pin-16]) {
-				printk(KERN_DEBUG "disabling PIRQ%d\n", pin-16);
+				apic_printk(APIC_VERBOSE, KERN_DEBUG 
+						"disabling PIRQ%d\n", pin-16);
 			} else {
 				irq = pirq_entries[pin-16];
-				printk(KERN_DEBUG "using PIRQ%d -> IRQ %d\n",
+				apic_printk(APIC_VERBOSE, KERN_DEBUG 
+						"using PIRQ%d -> IRQ %d\n",
 						pin-16, irq);
 			}
 		}
@@ -1216,7 +1224,7 @@ void __init setup_IO_APIC_irqs(void)
 	int apic, pin, idx, irq, first_notcon = 1, vector;
 	unsigned long flags;
 
-	printk(KERN_DEBUG "init IO_APIC IRQs\n");
+	apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
 
 	for (apic = 0; apic < nr_ioapics; apic++) {
 	for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {
@@ -1235,10 +1243,14 @@ void __init setup_IO_APIC_irqs(void)
 		idx = find_irq_entry(apic,pin,mp_INT);
 		if (idx == -1) {
 			if (first_notcon) {
-				printk(KERN_DEBUG " IO-APIC (apicid-pin) %d-%d",
mp_ioapics[apic].mpc_apicid, pin);
+				apic_printk(APIC_VERBOSE, KERN_DEBUG 
+						" IO-APIC (apicid-pin) %d-%d", 
+						mp_ioapics[apic].mpc_apicid, 
+						pin);
 				first_notcon = 0;
 			} else
-				printk(", %d-%d", mp_ioapics[apic].mpc_apicid, pin);
+				apic_printk(APIC_VERBOSE, ", %d-%d", 
+					mp_ioapics[apic].mpc_apicid, pin);
 			continue;
 		}
 
@@ -1279,7 +1291,7 @@ void __init setup_IO_APIC_irqs(void)
 	}
 
 	if (!first_notcon)
-		printk(" not connected.\n");
+		apic_printk(APIC_VERBOSE, " not connected.\n");
 }
 
 /*
@@ -1339,6 +1351,9 @@ void __init print_IO_APIC(void)
 	union IO_APIC_reg_03 reg_03;
 	unsigned long flags;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
  	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
 	for (i = 0; i < nr_ioapics; i++)
 		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1476,6 +1491,9 @@ static void print_APIC_bitfield (int bas
 	unsigned int v;
 	int i, j;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "0123456789abcdef0123456789abcdef\n" KERN_DEBUG);
 	for (i = 0; i < 8; i++) {
 		v = apic_read(base + i*0x10);
@@ -1493,6 +1511,9 @@ void /*__init*/ print_local_APIC(void * 
 {
 	unsigned int v, ver, maxlvt;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
 	printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
 		smp_processor_id(), hard_smp_processor_id());
 	v = apic_read(APIC_ID);
@@ -1580,6 +1601,9 @@ void /*__init*/ print_PIC(void)
 	unsigned int v;
 	unsigned long flags;
 
+	if (apic_verbosity == APIC_QUIET)
+		return;
+		
 	printk(KERN_DEBUG "\nprinting PIC contents\n");
 
 	spin_lock_irqsave(&i8259A_lock, flags);
@@ -1715,7 +1739,9 @@ static void __init setup_ioapic_ids_from
 		} else {
 			physid_mask_t tmp;
 			tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
-			printk("Setting %d in the phys_id_present_map\n",
mp_ioapics[apic].mpc_apicid);
+			apic_printk(APIC_VERBOSE, "Setting %d in the "
+					"phys_id_present_map\n", 
+					mp_ioapics[apic].mpc_apicid);
 			physids_or(phys_id_present_map, phys_id_present_map, tmp);
 		}
 
@@ -1734,8 +1760,9 @@ static void __init setup_ioapic_ids_from
 		 * Read the right value from the MPC table and
 		 * write it into the ID register.
 	 	 */
-		printk(KERN_INFO "...changing IO-APIC physical APIC ID to %d ...",
-					mp_ioapics[apic].mpc_apicid);
+		apic_printk(APIC_VERBOSE, KERN_INFO 
+			"...changing IO-APIC physical APIC ID to %d ...",
+			mp_ioapics[apic].mpc_apicid);
 
 		reg_00.bits.ID = mp_ioapics[apic].mpc_apicid;
 		spin_lock_irqsave(&ioapic_lock, flags);
@@ -1751,7 +1778,7 @@ static void __init setup_ioapic_ids_from
 		if (reg_00.bits.ID != mp_ioapics[apic].mpc_apicid)
 			panic("could not set ID!\n");
 		else
-			printk(" ok.\n");
+			apic_printk(APIC_VERBOSE, " ok.\n");
 	}
 }
 #else
@@ -2085,11 +2112,11 @@ static void setup_nmi (void)
 	 * is from Maciej W. Rozycki - so we do not have to EOI from
 	 * the NMI handler or the timer interrupt.
 	 */ 
-	printk(KERN_INFO "activating NMI Watchdog ...");
+	apic_printk(APIC_VERBOSE, KERN_INFO "activating NMI Watchdog ...");
 
 	on_each_cpu(enable_NMI_through_LVT0, NULL, 1, 1);
 
-	printk(" done.\n");
+	apic_printk(APIC_VERBOSE, " done.\n");
 }
 
 /*
@@ -2266,7 +2293,8 @@ static inline void check_timer(void)
 		return;
 	}
 	printk(" failed :(.\n");
-	panic("IO-APIC + timer doesn't work! pester mingo@redhat.com");
+	panic("IO-APIC + timer doesn't work! ensure you boot with "
+		"ioapic=debug and pester mingo@redhat.com");
 }
 
 /*
@@ -2390,7 +2418,8 @@ int __init io_apic_get_unique_id (int io
 			panic("IOAPIC[%d]: Unable change apic_id!\n", ioapic);
 	}
 
-	printk(KERN_INFO "IOAPIC[%d]: Assigned apic_id %d\n", ioapic,
apic_id);
+	apic_printk(APIC_VERBOSE, KERN_INFO 
+			"IOAPIC[%d]: Assigned apic_id %d\n", ioapic, apic_id);
 
 	return apic_id;
 }
@@ -2456,9 +2485,10 @@ int io_apic_set_pci_routing (int ioapic,
 
 	entry.vector = assign_irq_vector(irq);
 
-	Dprintk(KERN_DEBUG "IOAPIC[%d]: Set PCI routing entry (%d-%d -> 0x%x
-> "
-		"IRQ %d Mode:%i Active:%i)\n", ioapic, 
-		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq, edge_level,
active_high_low);
+	apic_printk(APIC_DEBUG, KERN_DEBUG "IOAPIC[%d]: Set PCI routing entry
"
+		"(%d-%d -> 0x%x -> IRQ %d Mode:%i Active:%i)\n", ioapic, 
+		mp_ioapics[ioapic].mpc_apicid, pin, entry.vector, irq, 
+		edge_level, active_high_low);
 
  	if (use_pci_vector() && !platform_legacy_irq(irq))
 		irq = IO_APIC_VECTOR(irq);
Index: linux-2.6.7/arch/i386/kernel/apic.c
===================================================================
--- linux-2.6.7/arch/i386/kernel/apic.c	(revision 1)
+++ linux-2.6.7/arch/i386/kernel/apic.c	(working copy)
@@ -40,6 +40,15 @@
 
 #include "io_ports.h"
 
+/* 
+ * Debug level
+ */
+extern int apic_verbosity;
+
+/* Ensure io_apic can see this level as well */
+EXPORT_SYMBOL(apic_verbosity);
+
+
 static void apic_pm_activate(void);
 
 void __init apic_intr_init(void)
@@ -163,7 +172,8 @@ void __init connect_bsp_APIC(void)
 		 * PIC mode, enable APIC mode in the IMCR, i.e.
 		 * connect BSP's local APIC to INT and NMI lines.
 		 */
-		printk("leaving PIC mode, enabling APIC mode.\n");
+		apic_printk(APIC_VERBOSE, "leaving PIC mode, "
+				"enabling APIC mode.\n");
 		outb(0x70, 0x22);
 		outb(0x01, 0x23);
 	}
@@ -179,7 +189,8 @@ void disconnect_bsp_APIC(void)
 		 * interrupts, including IPIs, won't work beyond
 		 * this point!  The only exception are INIT IPIs.
 		 */
-		printk("disabling APIC mode, entering PIC mode.\n");
+		apic_printk(APIC_VERBOSE, "disabling APIC mode, "
+				"entering PIC mode.\n");
 		outb(0x70, 0x22);
 		outb(0x00, 0x23);
 	}
@@ -220,10 +231,10 @@ int __init verify_local_APIC(void)
 	 * The version register is read-only in a real APIC.
 	 */
 	reg0 = apic_read(APIC_LVR);
-	Dprintk("Getting VERSION: %x\n", reg0);
+	apic_printk(APIC_DEBUG, "Getting VERSION: %x\n", reg0);
 	apic_write(APIC_LVR, reg0 ^ APIC_LVR_MASK);
 	reg1 = apic_read(APIC_LVR);
-	Dprintk("Getting VERSION: %x\n", reg1);
+	apic_printk(APIC_DEBUG, "Getting VERSION: %x\n", reg1);
 
 	/*
 	 * The two version reads above should print the same
@@ -247,7 +258,7 @@ int __init verify_local_APIC(void)
 	 * The ID register is read/write in a real APIC.
 	 */
 	reg0 = apic_read(APIC_ID);
-	Dprintk("Getting ID: %x\n", reg0);
+	apic_printk(APIC_DEBUG, "Getting ID: %x\n", reg0);
 
 	/*
 	 * The next two are just to see if we have sane values.
@@ -255,9 +266,9 @@ int __init verify_local_APIC(void)
 	 * compatibility mode, but most boxes are anymore.
 	 */
 	reg0 = apic_read(APIC_LVT0);
-	Dprintk("Getting LVT0: %x\n", reg0);
+	apic_printk(APIC_DEBUG, "Getting LVT0: %x\n", reg0);
 	reg1 = apic_read(APIC_LVT1);
-	Dprintk("Getting LVT1: %x\n", reg1);
+	apic_printk(APIC_DEBUG, "Getting LVT1: %x\n", reg1);
 
 	return 1;
 }
@@ -269,7 +280,7 @@ void __init sync_Arb_IDs(void)
 	 */
 	apic_wait_icr_idle();
 
-	Dprintk("Synchronizing Arb IDs.\n");
+	apic_printk(APIC_DEBUG, "Synchronizing Arb IDs.\n");
 	apic_write_around(APIC_ICR, APIC_DEST_ALLINC | APIC_INT_LEVELTRIG
 				| APIC_DM_INIT);
 }
@@ -417,10 +428,12 @@ void __init setup_local_APIC (void)
 	value = apic_read(APIC_LVT0) & APIC_LVT_MASKED;
 	if (!smp_processor_id() && (pic_mode || !value)) {
 		value = APIC_DM_EXTINT;
-		printk("enabled ExtINT on CPU#%d\n", smp_processor_id());
+		apic_printk(APIC_VERBOSE, "enabled ExtINT on CPU#%d\n", 
+				smp_processor_id());
 	} else {
 		value = APIC_DM_EXTINT | APIC_LVT_MASKED;
-		printk("masked ExtINT on CPU#%d\n", smp_processor_id());
+		apic_printk(APIC_VERBOSE, "masked ExtINT on CPU#%d\n", 
+				smp_processor_id());
 	}
 	apic_write_around(APIC_LVT0, value);
 
@@ -440,7 +453,8 @@ void __init setup_local_APIC (void)
 		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);
+		apic_printk(APIC_VERBOSE, "ESR value before enabling vector:"
+				" %08lx\n", value);
 
 		value = ERROR_APIC_VECTOR;      // enables sending errors
 		apic_write_around(APIC_LVTERR, value);
@@ -450,7 +464,8 @@ void __init setup_local_APIC (void)
 		if (maxlvt > 3)
 			apic_write(APIC_ESR, 0);
 		value = apic_read(APIC_ESR);
-		printk("ESR value after enabling vector: %08lx\n", value);
+		apic_printk(APIC_VERBOSE, "ESR value after enabling vector:"
+				" %08lx\n", value);
 	} else {
 		if (esr_disable)	
 			/* 
@@ -625,6 +640,21 @@ static int __init lapic_enable(char *str
 }
 __setup("lapic", lapic_enable);
 
+static int __init apic_set_verbosity(char *str)
+{
+	if (strcmp("debug", str) == 0)
+		apic_verbosity = APIC_DEBUG;
+	else if (strcmp("verbose", str) == 0)
+		apic_verbosity = APIC_VERBOSE;
+	else
+		printk(KERN_WARNING "APIC Verbosity level %s not recognised"
+				" use apic=verbose or apic=debug", str);
+
+	return 0;
+}
+
+__setup("apic=", apic_set_verbosity);
+
 static int __init detect_init_APIC (void)
 {
 	u32 h, l, features;
@@ -661,7 +691,8 @@ static int __init detect_init_APIC (void
 		 */
 		rdmsr(MSR_IA32_APICBASE, l, h);
 		if (!(l & MSR_IA32_APICBASE_ENABLE)) {
-			printk("Local APIC disabled by BIOS -- reenabling.\n");
+			apic_printk(APIC_VERBOSE, "Local APIC disabled "
+					"by BIOS -- reenabling.\n");
 			l &= ~MSR_IA32_APICBASE_BASE;
 			l |= MSR_IA32_APICBASE_ENABLE | APIC_DEFAULT_PHYS_BASE;
 			wrmsr(MSR_IA32_APICBASE, l, h);
@@ -688,7 +719,7 @@ static int __init detect_init_APIC (void
 	if (nmi_watchdog != NMI_NONE)
 		nmi_watchdog = NMI_LOCAL_APIC;
 
-	printk("Found and enabled local APIC!\n");
+	apic_printk(APIC_VERBOSE, "Found and enabled local APIC!\n");
 
 	apic_pm_activate();
 
@@ -715,7 +746,8 @@ void __init init_apic_mappings(void)
 		apic_phys = mp_lapic_addr;
 
 	set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
-	Dprintk("mapped APIC to %08lx (%08lx)\n", APIC_BASE, apic_phys);
+	apic_printk(APIC_DEBUG, "mapped APIC to %08lx (%08lx)\n", APIC_BASE, 
+			apic_phys);
 
 	/*
 	 * Fetch the APIC ID of the BSP in case we have a
@@ -745,7 +777,8 @@ fake_ioapic_page:
 				ioapic_phys = __pa(ioapic_phys);
 			}
 			set_fixmap_nocache(idx, ioapic_phys);
-			Dprintk("mapped IOAPIC to %08lx (%08lx)\n",
+			apic_printk(APIC_DEBUG, "mapped IOAPIC to "
+					"%08lx (%08lx)\n",
 					__fix_to_virt(idx), ioapic_phys);
 			idx++;
 		}
@@ -884,7 +917,7 @@ int __init calibrate_APIC_clock(void)
 	int i;
 	const int LOOPS = HZ/10;
 
-	printk("calibrating APIC timer ...\n");
+	apic_printk(APIC_VERBOSE, "calibrating APIC timer ...\n");
 
 	/*
 	 * Put whatever arbitrary (but long enough) timeout
@@ -929,11 +962,13 @@ int __init calibrate_APIC_clock(void)
 	result = (tt1-tt2)*APIC_DIVISOR/LOOPS;
 
 	if (cpu_has_tsc)
-		printk("..... CPU clock speed is %ld.%04ld MHz.\n",
+		apic_printk(APIC_VERBOSE, "..... CPU clock speed is "
+			"%ld.%04ld MHz.\n",
 			((long)(t2-t1)/LOOPS)/(1000000/HZ),
 			((long)(t2-t1)/LOOPS)%(1000000/HZ));
 
-	printk("..... host bus clock speed is %ld.%04ld MHz.\n",
+	apic_printk(APIC_VERBOSE, "..... host bus clock speed is "
+		"%ld.%04ld MHz.\n",
 		result/(1000000/HZ),
 		result%(1000000/HZ));
 
@@ -944,7 +979,7 @@ static unsigned int calibration_result;
 
 void __init setup_boot_APIC_clock(void)
 {
-	printk("Using local APIC timer interrupts.\n");
+	apic_printk(APIC_VERBOSE, "Using local APIC timer interrupts.\n");
 	using_apic_timer = 1;
 
 	local_irq_disable();



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Jeff Garzik]

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

Mark Broadbent wrote:
> --- linux-2.6.7/arch/i386/kernel/apic.c	(revision 1)
> +++ linux-2.6.7/arch/i386/kernel/apic.c	(working copy)
> @@ -40,6 +40,15 @@
>  
>  #include "io_ports.h"
>  
> +/* 
> + * Debug level
> + */
> +extern int apic_verbosity;
> +
> +/* Ensure io_apic can see this level as well */
> +EXPORT_SYMBOL(apic_verbosity);


Your patch looks OK to me except for one small bit, above.

First, you should put the 'extern int apic_verbosity' in a header, 
presumeably the same header into which you stashed 
APIC_{QUIET,VERBOSE,DEBUG} definitions.

Second, it is highly unlikely that this symbol should be exported to 
kernel modules.

	Jeff



[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Randy.Dunlap]

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

On Tue, 13 Jul 2004 20:20:31 +0100 Mark Broadbent wrote:

| Index: linux-2.6.7/include/asm-i386/apic.h
| ===================================================================
| --- linux-2.6.7/include/asm-i386/apic.h	(revision 1)
| +++ linux-2.6.7/include/asm-i386/apic.h	(working copy)
| @@ -7,13 +7,26 @@
|  #include <asm/apicdef.h>
|  #include <asm/system.h>
|  
| -#define APIC_DEBUG 0
| -
| -#if APIC_DEBUG
| -#define Dprintk(x...) printk(x)
| -#else
|  #define Dprintk(x...)
| -#endif
| +
| +/* 
| + * Debugging macros
| + */
| +#define APIC_QUIET   0
| +#define APIC_VERBOSE 1
| +#define APIC_DEBUG   2
| +
| +/*
| + * Define the default level of output to be very little
| + * This can be turned up by using apic=verbose for more
| + * information and apic=debug for _lots_ of information.
| + * apic_verbosity is defined in apic.c
| + */
| +#define apic_printk(v, s, a...) do {       \
| +		if ((v) <= apic_verbosity) \
| +			printk(s, ##a);    \
| +	} while (0)
| +

Isn't "##a" a problem with some versions of gcc, or am I thinking
of something else?  Sometimes we have to use "## a" instead....



| Index: linux-2.6.7/arch/i386/kernel/apic.c
| ===================================================================
| --- linux-2.6.7/arch/i386/kernel/apic.c	(revision 1)
| +++ linux-2.6.7/arch/i386/kernel/apic.c	(working copy)
| @@ -625,6 +640,21 @@ static int __init lapic_enable(char *str
|  }
|  __setup("lapic", lapic_enable);
|  
| +static int __init apic_set_verbosity(char *str)
| +{
| +	if (strcmp("debug", str) == 0)
| +		apic_verbosity = APIC_DEBUG;
| +	else if (strcmp("verbose", str) == 0)
| +		apic_verbosity = APIC_VERBOSE;
| +	else
| +		printk(KERN_WARNING "APIC Verbosity level %s not recognised"
| +				" use apic=verbose or apic=debug", str);
| +
| +	return 0;
| +}
| +
| +__setup("apic=", apic_set_verbosity);

Is this patch separate from the ioapic=<verbosity_level> patch?
The reason I ask is this message:

+	panic("IO-APIC + timer doesn't work! ensure you boot with "
+		"ioapic=debug and pester mingo@redhat.com");

which uses ioapic=<level>.  So are there both ioapic=level and apic=level?

Thanks,
--
~Randy

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [Kernel-janitors] [PATCH] IO-APIC debug message reducti

[Mark Broadbent]

> Isn't "##a" a problem with some versions of gcc, or am I thinking
> of something else?  Sometimes we have to use "## a" instead....

I haven't seen a case like that, I did some investigating and in
include/linux/kernel.h there is a similar usage to mine:
 #define pr_debug(fmt,arg...) \
	printk(KERN_DEBUG fmt,##arg)

However if someone knows better then I'll change it.

> | +__setup("apic=", apic_set_verbosity);
>
> Is this patch separate from the ioapic=<verbosity_level> patch?

The patch was extended to include apic and io-apic with one arg.

> The reason I ask is this message:
>
> +	panic("IO-APIC + timer doesn't work! ensure you boot with "
> +		"ioapicÞbug and pester mingo@redhat.com");
>
> which uses ioapic=<level>.  So are there both ioapic=level and
> apic=level?

It's a typo in the panic statement, it should be apic=, I'll ensure Andrew
gets a modified patch.
Thanks
Mark



_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors