[PATCH]: xen: fix too early kmalloc call

[PATCH]: xen: fix too early kmalloc call

[Mike Travis]

Subject: xen: fix too early kmalloc call

From: Christophe Saout <christophe@saout.de> 

Impact: fixes bootup of xen.

Christophe reported the following problem:

>  (basically it seems that SLAB is not yet up, with earlyprintk it is
>  giving me an Oops in __kmalloc before)

Replace call to kmalloc with alloc_bootmem.

Also from Christophe:
> (me)
> > Or I could copy the text and submit it as a new patch?
>
> Yes, I would prefer that.  Also, my commit message was not really
> following Kernel standards and it's only a very simple change.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 drivers/xen/events.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- linux-2.6-for-ingo.orig/drivers/xen/events.c
+++ linux-2.6-for-ingo/drivers/xen/events.c
@@ -26,6 +26,7 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include <linux/bootmem.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq.h>
@@ -831,8 +832,8 @@ void __init xen_init_IRQ(void)
 	int i;
 	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
 
-	cpu_evtchn_mask_p = kmalloc(size, GFP_KERNEL);
-	BUG_ON(cpu_evtchn_mask == NULL);
+	cpu_evtchn_mask_p = alloc_bootmem(size);
+	BUG_ON(cpu_evtchn_mask_p == NULL);
 
 	init_evtchn_cpu_bindings();
 

Re: [PATCH]: xen: fix too early kmalloc call

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> Subject: xen: fix too early kmalloc call
> 
> From: Christophe Saout <christophe@saout.de> 
> 
> Impact: fixes bootup of xen.
> 
> Christophe reported the following problem:
> 
> >  (basically it seems that SLAB is not yet up, with earlyprintk it is
> >  giving me an Oops in __kmalloc before)
> 
> Replace call to kmalloc with alloc_bootmem.
> 
> Also from Christophe:
> > (me)
> > > Or I could copy the text and submit it as a new patch?
> >
> > Yes, I would prefer that.  Also, my commit message was not really
> > following Kernel standards and it's only a very simple change.
> 
> Signed-off-by: Mike Travis <travis@sgi.com>

i've applied the fix to tip/cpus4096, in the form below.

Please submit clean log messages. Your commit description was confused - 
the patch is From: Christophe, but the message talks about Christophe in 
the third person. Also, it included conversational bits that have no 
value for the Git history.

	Ingo

-------------------->
>From 28e08861b9afab4168b758fb7b95aa7a4da0f668 Mon Sep 17 00:00:00 2001
From: Christophe Saout <christophe@saout.de>
Date: Sun, 11 Jan 2009 11:46:23 -0800
Subject: [PATCH] xen: fix too early kmalloc call

Impact: fix bootup crash on xen guests

SLAB is not yet up, with earlyprintk it is giving me an Oops in __kmalloc.

Replace call to kmalloc() with alloc_bootmem().

Reported-by: Christophe Saout <christophe@saout.de>
Signed-off-by: Mike Travis <travis@sgi.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/xen/events.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index ed7527b..3141e14 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -26,6 +26,7 @@
 #include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include <linux/bootmem.h>
 
 #include <asm/ptrace.h>
 #include <asm/irq.h>
@@ -831,8 +832,8 @@ void __init xen_init_IRQ(void)
 	int i;
 	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
 
-	cpu_evtchn_mask_p = kmalloc(size, GFP_KERNEL);
-	BUG_ON(cpu_evtchn_mask == NULL);
+	cpu_evtchn_mask_p = alloc_bootmem(size);
+	BUG_ON(cpu_evtchn_mask_p == NULL);
 
 	init_evtchn_cpu_bindings();
 

Re: [PATCH]: xen: fix too early kmalloc call

[Johannes Weiner]

On Mon, Jan 12, 2009 at 09:53:02AM +0100, Ingo Molnar wrote:
> 
> * Mike Travis <travis@sgi.com> wrote:
> 
> > Subject: xen: fix too early kmalloc call
> > 
> > From: Christophe Saout <christophe@saout.de> 
> > 
> > Impact: fixes bootup of xen.
> > 
> > Christophe reported the following problem:
> > 
> > >  (basically it seems that SLAB is not yet up, with earlyprintk it is
> > >  giving me an Oops in __kmalloc before)
> > 
> > Replace call to kmalloc with alloc_bootmem.
> > 
> > Also from Christophe:
> > > (me)
> > > > Or I could copy the text and submit it as a new patch?
> > >
> > > Yes, I would prefer that.  Also, my commit message was not really
> > > following Kernel standards and it's only a very simple change.
> > 
> > Signed-off-by: Mike Travis <travis@sgi.com>
> 
> i've applied the fix to tip/cpus4096, in the form below.
> 
> Please submit clean log messages. Your commit description was confused - 
> the patch is From: Christophe, but the message talks about Christophe in 
> the third person. Also, it included conversational bits that have no 
> value for the Git history.
> 
> 	Ingo
> 
> -------------------->
> >>From 28e08861b9afab4168b758fb7b95aa7a4da0f668 Mon Sep 17 00:00:00 2001
> From: Christophe Saout <christophe@saout.de>
> Date: Sun, 11 Jan 2009 11:46:23 -0800
> Subject: [PATCH] xen: fix too early kmalloc call
> 
> Impact: fix bootup crash on xen guests
> 
> SLAB is not yet up, with earlyprintk it is giving me an Oops in __kmalloc.
> 
> Replace call to kmalloc() with alloc_bootmem().
> 
> Reported-by: Christophe Saout <christophe@saout.de>
> Signed-off-by: Mike Travis <travis@sgi.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  drivers/xen/events.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index ed7527b..3141e14 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -26,6 +26,7 @@
>  #include <linux/irq.h>
>  #include <linux/module.h>
>  #include <linux/string.h>
> +#include <linux/bootmem.h>
>  
>  #include <asm/ptrace.h>
>  #include <asm/irq.h>
> @@ -831,8 +832,8 @@ void __init xen_init_IRQ(void)
>  	int i;
>  	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
>  
> -	cpu_evtchn_mask_p = kmalloc(size, GFP_KERNEL);
> -	BUG_ON(cpu_evtchn_mask == NULL);
> +	cpu_evtchn_mask_p = alloc_bootmem(size);
> +	BUG_ON(cpu_evtchn_mask_p == NULL);

Impossible condition :-) alloc_bootmem() crashes before you could
catch it here.

	Hannes

Re: [PATCH]: xen: fix too early kmalloc call

[Ingo Molnar]

* Johannes Weiner <hannes@cmpxchg.org> wrote:

> On Mon, Jan 12, 2009 at 09:53:02AM +0100, Ingo Molnar wrote:
> > 
> > * Mike Travis <travis@sgi.com> wrote:
> > 
> > > Subject: xen: fix too early kmalloc call
> > > 
> > > From: Christophe Saout <christophe@saout.de> 
> > > 
> > > Impact: fixes bootup of xen.
> > > 
> > > Christophe reported the following problem:
> > > 
> > > >  (basically it seems that SLAB is not yet up, with earlyprintk it is
> > > >  giving me an Oops in __kmalloc before)
> > > 
> > > Replace call to kmalloc with alloc_bootmem.
> > > 
> > > Also from Christophe:
> > > > (me)
> > > > > Or I could copy the text and submit it as a new patch?
> > > >
> > > > Yes, I would prefer that.  Also, my commit message was not really
> > > > following Kernel standards and it's only a very simple change.
> > > 
> > > Signed-off-by: Mike Travis <travis@sgi.com>
> > 
> > i've applied the fix to tip/cpus4096, in the form below.
> > 
> > Please submit clean log messages. Your commit description was confused - 
> > the patch is From: Christophe, but the message talks about Christophe in 
> > the third person. Also, it included conversational bits that have no 
> > value for the Git history.
> > 
> > 	Ingo
> > 
> > -------------------->
> > >>From 28e08861b9afab4168b758fb7b95aa7a4da0f668 Mon Sep 17 00:00:00 2001
> > From: Christophe Saout <christophe@saout.de>
> > Date: Sun, 11 Jan 2009 11:46:23 -0800
> > Subject: [PATCH] xen: fix too early kmalloc call
> > 
> > Impact: fix bootup crash on xen guests
> > 
> > SLAB is not yet up, with earlyprintk it is giving me an Oops in __kmalloc.
> > 
> > Replace call to kmalloc() with alloc_bootmem().
> > 
> > Reported-by: Christophe Saout <christophe@saout.de>
> > Signed-off-by: Mike Travis <travis@sgi.com>
> > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > ---
> >  drivers/xen/events.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index ed7527b..3141e14 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/irq.h>
> >  #include <linux/module.h>
> >  #include <linux/string.h>
> > +#include <linux/bootmem.h>
> >  
> >  #include <asm/ptrace.h>
> >  #include <asm/irq.h>
> > @@ -831,8 +832,8 @@ void __init xen_init_IRQ(void)
> >  	int i;
> >  	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
> >  
> > -	cpu_evtchn_mask_p = kmalloc(size, GFP_KERNEL);
> > -	BUG_ON(cpu_evtchn_mask == NULL);
> > +	cpu_evtchn_mask_p = alloc_bootmem(size);
> > +	BUG_ON(cpu_evtchn_mask_p == NULL);
> 
> Impossible condition :-) alloc_bootmem() crashes before you could catch 
> it here.

yeah ... but that's just a current behavior of bootmem. It's always 
prudent to check for allocation errors.

	Ingo

Re: [PATCH]: xen: fix too early kmalloc call

[Mike Travis]

Ingo Molnar wrote:
> * Mike Travis <travis@sgi.com> wrote:
> 
>> Subject: xen: fix too early kmalloc call
>>
>> From: Christophe Saout <christophe@saout.de> 
>>
>> Impact: fixes bootup of xen.
>>
>> Christophe reported the following problem:
>>
>>>  (basically it seems that SLAB is not yet up, with earlyprintk it is
>>>  giving me an Oops in __kmalloc before)
>> Replace call to kmalloc with alloc_bootmem.
>>
>> Also from Christophe:
>>> (me)
>>>> Or I could copy the text and submit it as a new patch?
>>> Yes, I would prefer that.  Also, my commit message was not really
>>> following Kernel standards and it's only a very simple change.
>> Signed-off-by: Mike Travis <travis@sgi.com>
> 
> i've applied the fix to tip/cpus4096, in the form below.
> 
> Please submit clean log messages. Your commit description was confused - 
> the patch is From: Christophe, but the message talks about Christophe in 
> the third person. Also, it included conversational bits that have no 
> value for the Git history.
> 
> 	Ingo

Ahh, ok, will do.  I wasn't sure how many "liberties" we can take in
submitting/changing something from someone else.

THanks,
Mike

> 
> -------------------->
>>From 28e08861b9afab4168b758fb7b95aa7a4da0f668 Mon Sep 17 00:00:00 2001
> From: Christophe Saout <christophe@saout.de>
> Date: Sun, 11 Jan 2009 11:46:23 -0800
> Subject: [PATCH] xen: fix too early kmalloc call
> 
> Impact: fix bootup crash on xen guests
> 
> SLAB is not yet up, with earlyprintk it is giving me an Oops in __kmalloc.
> 
> Replace call to kmalloc() with alloc_bootmem().
> 
> Reported-by: Christophe Saout <christophe@saout.de>
> Signed-off-by: Mike Travis <travis@sgi.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  drivers/xen/events.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index ed7527b..3141e14 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -26,6 +26,7 @@
>  #include <linux/irq.h>
>  #include <linux/module.h>
>  #include <linux/string.h>
> +#include <linux/bootmem.h>
>  
>  #include <asm/ptrace.h>
>  #include <asm/irq.h>
> @@ -831,8 +832,8 @@ void __init xen_init_IRQ(void)
>  	int i;
>  	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
>  
> -	cpu_evtchn_mask_p = kmalloc(size, GFP_KERNEL);
> -	BUG_ON(cpu_evtchn_mask == NULL);
> +	cpu_evtchn_mask_p = alloc_bootmem(size);
> +	BUG_ON(cpu_evtchn_mask_p == NULL);
>  
>  	init_evtchn_cpu_bindings();
>  

Re: [PATCH]: xen: fix too early kmalloc call

[Mike Travis]

Johannes Weiner wrote:
> On Mon, Jan 12, 2009 at 09:53:02AM +0100, Ingo Molnar wrote:
>> * Mike Travis <travis@sgi.com> wrote:
>>
>>> Subject: xen: fix too early kmalloc call
>>>
>>> From: Christophe Saout <christophe@saout.de> 
>>>
>>> Impact: fixes bootup of xen.
>>>
>>> Christophe reported the following problem:
>>>
>>>>  (basically it seems that SLAB is not yet up, with earlyprintk it is
>>>>  giving me an Oops in __kmalloc before)
>>> Replace call to kmalloc with alloc_bootmem.
>>>
>>> Also from Christophe:
>>>> (me)
>>>>> Or I could copy the text and submit it as a new patch?
>>>> Yes, I would prefer that.  Also, my commit message was not really
>>>> following Kernel standards and it's only a very simple change.
>>> Signed-off-by: Mike Travis <travis@sgi.com>
>> i've applied the fix to tip/cpus4096, in the form below.
>>
>> Please submit clean log messages. Your commit description was confused - 
>> the patch is From: Christophe, but the message talks about Christophe in 
>> the third person. Also, it included conversational bits that have no 
>> value for the Git history.
>>
>> 	Ingo
>>
>> -------------------->
>>> >From 28e08861b9afab4168b758fb7b95aa7a4da0f668 Mon Sep 17 00:00:00 2001
>> From: Christophe Saout <christophe@saout.de>
>> Date: Sun, 11 Jan 2009 11:46:23 -0800
>> Subject: [PATCH] xen: fix too early kmalloc call
>>
>> Impact: fix bootup crash on xen guests
>>
>> SLAB is not yet up, with earlyprintk it is giving me an Oops in __kmalloc.
>>
>> Replace call to kmalloc() with alloc_bootmem().
>>
>> Reported-by: Christophe Saout <christophe@saout.de>
>> Signed-off-by: Mike Travis <travis@sgi.com>
>> Signed-off-by: Ingo Molnar <mingo@elte.hu>
>> ---
>>  drivers/xen/events.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>> index ed7527b..3141e14 100644
>> --- a/drivers/xen/events.c
>> +++ b/drivers/xen/events.c
>> @@ -26,6 +26,7 @@
>>  #include <linux/irq.h>
>>  #include <linux/module.h>
>>  #include <linux/string.h>
>> +#include <linux/bootmem.h>
>>  
>>  #include <asm/ptrace.h>
>>  #include <asm/irq.h>
>> @@ -831,8 +832,8 @@ void __init xen_init_IRQ(void)
>>  	int i;
>>  	size_t size = nr_cpu_ids * sizeof(struct cpu_evtchn_s);
>>  
>> -	cpu_evtchn_mask_p = kmalloc(size, GFP_KERNEL);
>> -	BUG_ON(cpu_evtchn_mask == NULL);
>> +	cpu_evtchn_mask_p = alloc_bootmem(size);
>> +	BUG_ON(cpu_evtchn_mask_p == NULL);
> 
> Impossible condition :-) alloc_bootmem() crashes before you could
> catch it here.
> 
> 	Hannes

Yes, that's picked up in a subsequent patch (I didn't catch it the
first time I reviewed it as well.)  There appears to be a queue of
patches for Xen coming, so this last "memory reduction patch" will
follow (or be included with) those.

Thanks,
Mike

Re: [PATCH]: xen: fix too early kmalloc call

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> > i've applied the fix to tip/cpus4096, in the form below.
> > 
> > Please submit clean log messages. Your commit description was confused 
> > - the patch is From: Christophe, but the message talks about 
> > Christophe in the third person. Also, it included conversational bits 
> > that have no value for the Git history.
> > 
> > 	Ingo
> 
> Ahh, ok, will do.  I wasn't sure how many "liberties" we can take in 
> submitting/changing something from someone else.

You are free to perfect anyone's commit log in essence - i do it for 90% 
of all commits that go via -tip. I fix tyops and grammar mistaiks in every 
second commit, and have to fix basic formatting, structure, explanations 
and reported-by credits and tags in most of them. Submitting patches with 
a well-structured commit log gives you extra credit :-)

[ Obviously you are not free to make a commit log worse and then make it 
  appear as if the guy who submitted it did it ;-) ]

	Ingo

Re: [PATCH]: xen: fix too early kmalloc call

[Christophe Saout]

Hi Mike,

> Yes, that's picked up in a subsequent patch (I didn't catch it the
> first time I reviewed it as well.)  There appears to be a queue of
> patches for Xen coming, so this last "memory reduction patch" will
> follow (or be included with) those.

It appears Jeremy is on vacation and I also don't know which of the
patches are meant for upstream (there seem still to be some bugs that
might need fixing), and it doesn't really matter if the XEN patches are
updated to your patch or the other way around.  The patches in the XEN
mercurial tree already do not apply the way they were before Jeremy went
on vacation, so in my opinion you can as well just go ahead.

Cheers,
	Christophe