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

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

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++) {