lkml.org 
[lkml]   [2009]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH] x86: fix IO APIC resource allocation error message
Date
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++) {

\
 
 \ /
  Last update: 2009-03-20 21:03    [W:0.066 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site