Re: [PATCH] x86: fix IO APIC resource allocation error message

From: Ingo Molnar
Date: Fri Mar 20 2009 - 15:23:46 EST



* Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> wrote:

> From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> ---
> 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
too but still it's a bit unexpected and hence fragile.

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.

Ingo
--
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/