Messages in this thread Patch in this message | | | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: [PATCH] x86: fix IO APIC resource allocation error message | Date | Fri, 20 Mar 2009 21:00:30 +0100 |
| |
On Friday 20 March 2009, Ingo Molnar wrote: > > * Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote: > > > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> > > Subject: [PATCH] x86: fix IO APIC resource allocation error message > > > > Impact: fix incorrect error message > > > > - IO APIC resource allocation error message contains one too many "be". > > > > - Print the error message iff there are IO APICs in the system. > > > > Cc: Alan Bartlett <ajb.stxsl@googlemail.com> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> > > --- > > I've seen this error message for some time on my x86-32 laptop... > > > > arch/x86/kernel/io_apic.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Index: b/arch/x86/kernel/io_apic.c > > =================================================================== > > --- a/arch/x86/kernel/io_apic.c > > +++ b/arch/x86/kernel/io_apic.c > > @@ -4150,9 +4150,9 @@ static int __init ioapic_insert_resource > > int i; > > struct resource *r = ioapic_resources; > > > > - if (!r) { > > + if (!r && nr_ioapics > 0) { > > printk(KERN_ERR > > - "IO APIC resources could be not be allocated.\n"); > > + "IO APIC resources couldn't be allocated.\n"); > > return -1; > > } > > looks good, but there's one weirdness: > > so if nr_ioapics == 0 && !r we'll drop into this codepath: > > for (i = 0; i < nr_ioapics; i++) { > insert_resource(&iomem_resource, r); > r++; > } > > return 0; > > we survive the loop by luck, and then return 0 - which we'll survive
s/by luck/by design/ -- we want to return 0 and thus have the correct return value of the initcall
> too but still it's a bit unexpected and hence fragile.
agreed on this one
> So i think we should rather add a standalone: > > if (nr_ioapics > 0) > printk(KERN_ERR > > check to the printk only. That wont affect the remaining code flow.
here is the revised version
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> Subject: [PATCH] x86: fix IO APIC resource allocation error message (take 2)
Impact: fix incorrect error message
- IO APIC resource allocation error message contains one too many "be".
- Print the error message iff there are IO APICs in the system.
Cc: Alan Bartlett <ajb.stxsl@googlemail.com> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> --- I've seen this error message for some time on my x86-32 laptop...
arch/x86/kernel/io_apic.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Index: b/arch/x86/kernel/io_apic.c =================================================================== --- a/arch/x86/kernel/io_apic.c +++ b/arch/x86/kernel/io_apic.c @@ -4151,9 +4151,12 @@ static int __init ioapic_insert_resource struct resource *r = ioapic_resources; if (!r) { - printk(KERN_ERR - "IO APIC resources could be not be allocated.\n"); - return -1; + if (nr_ioapics > 0) { + printk(KERN_ERR + "IO APIC resources couldn't be allocated.\n"); + return -1; + } + return 0; } for (i = 0; i < nr_ioapics; i++) {
| |