Messages in this thread Patch in this message | | | Date | Sun, 7 Sep 2008 14:00:00 +0400 | From | Cyrill Gorcunov <> | Subject | Re: [patch 3/3] x86: io-apic - code style cleaning for setup_IO_APIC_irqs |
| |
[Ingo Molnar - Sat, Sep 06, 2008 at 08:49:14PM +0200] | | * Maciej W. Rozycki <macro@linux-mips.org> wrote: | | > On Sat, 6 Sep 2008, Cyrill Gorcunov wrote: | > | > > Ingo, how about the following approach? We don't introduce new | > > functions but rather srink the code by new printout form. | > | > Honestly, this one should probably use sprintf() or suchlike to avoid the | > mess of printk() calls building a line of output from pieces. It's quite | > easy to calculate here what the maximum size of the buffer required could | > be and automatic arrays can have variable size, so no need for the hassle | > of heap management. Calls to printk() without a trailing newline should | > be avoided where possible as it messes up logging priority if a message | > pops up from an interrupt inbetween. | | hm, is it worth the trouble? This is during very early init. | | Ingo |
Ingo, Maciej,
here is an another attempt to satisfy the requirements :) Please review and comment if it looks better or worse now.
- Cyrill -
--- From: Cyrill Gorcunov <gorcunov@gmail.com> Subject: [PATCH] x86: io-apic - cleanup of setup_IO_APIC_irqs v3
Use local buffer to accumulate all not connected pin info on each io-apic and printout it if needed.
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com> ---
Index: linux-2.6.git/arch/x86/kernel/io_apic.c =================================================================== --- linux-2.6.git.orig/arch/x86/kernel/io_apic.c 2008-09-07 10:10:15.000000000 +0400 +++ linux-2.6.git/arch/x86/kernel/io_apic.c 2008-09-07 13:50:46.000000000 +0400 @@ -1514,46 +1514,69 @@ static void setup_IO_APIC_irq(int apic, ioapic_write_entry(apic, pin, entry); } +/* check and setup io-apic irq */ +static int __init setup_ioapic_irq(int apic, int pin) +{ + int idx, irq; + + idx = find_irq_entry(apic, pin, mp_INT); + if (idx == -1) + return -1; + + irq = pin_2_irq(idx, apic, pin); + +#ifdef CONFIG_X86_32 + if (multi_timer_check(apic, irq)) + return 0; +#endif + + add_pin_to_irq(irq, apic, pin); + setup_IO_APIC_irq(apic, pin, irq, + irq_trigger(idx), irq_polarity(idx)); + return 0; +} + static void __init setup_IO_APIC_irqs(void) { - int apic, pin, idx, irq; - int notcon = 0; + int apic, pin; + char buf[64], *p; + int len; apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n"); + p = buf, buf[0] = 0, len = 0; + for (apic = 0; apic < nr_ioapics; apic++) { for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) { - idx = find_irq_entry(apic, pin, mp_INT); - if (idx == -1) { - apic_printk(APIC_VERBOSE, - KERN_DEBUG " %d-%d", - mp_ioapics[apic].mp_apicid, pin); - if (!notcon) - notcon = 1; + if (!setup_ioapic_irq(apic, pin)) continue; - } - irq = pin_2_irq(idx, apic, pin); -#ifdef CONFIG_X86_32 - if (multi_timer_check(apic, irq)) + /* + * pin is not connected + * - we try to save this info in the buffer + * and print it out later + * - we don't expect too much not connected + * pins (as MP specification says the maximum + * value is 2 not connected pins) but if we + * have buggy BIOS or just get too many of them - + * truncate output with "..." + */ + if (!p) continue; -#endif - add_pin_to_irq(irq, apic, pin); - - setup_IO_APIC_irq(apic, pin, irq, - irq_trigger(idx), irq_polarity(idx)); + len += snprintf(p + len, sizeof(buf) - len, " %d-%d", + mp_ioapics[apic].mp_apicid, pin); + if (len >= sizeof(buf)) { + memcpy(&buf[sizeof(buf) - 4], "...", 3); + p = NULL; + } } - if (notcon) { - apic_printk(APIC_VERBOSE, - KERN_DEBUG " (apicid-pin) not connected\n"); - notcon = 0; + if (buf[0]) { + apic_printk(APIC_VERBOSE, KERN_DEBUG + " (apicid-pin) not connected:%s\n", buf); + p = buf, buf[0] = 0, len = 0; } } - - if (notcon) - apic_printk(APIC_VERBOSE, - KERN_DEBUG " (apicid-pin) not connected\n"); } /*
| |