* [PATCH] slab: Adds two missing kmalloc() checks.
@ 2006-01-23 15:31 Luiz Fernando Capitulino
2006-01-23 16:38 ` Pekka Enberg
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Fernando Capitulino @ 2006-01-23 15:31 UTC (permalink / raw)
To: akpm; +Cc: lkml, penberg
Adds two missing kmalloc() checks in kmem_cache_init(). The check is worth
because if kmalloc() fails we'll have a nice message instead of a OOPS (which
will make us look for a bug).
We're using BUG_ON() so that embedded people can disable it.
Signed-off-by: Luiz Capitulino <lcapitulino@mandriva.com.br>
mm/slab.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/slab.c b/mm/slab.c
index 6f8495e..2fcfd08 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1130,6 +1130,7 @@ void __init kmem_cache_init(void)
void *ptr;
ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
+ BUG_ON(!ptr);
local_irq_disable();
BUG_ON(ac_data(&cache_cache) != &initarray_cache.cache);
@@ -1139,6 +1140,7 @@ void __init kmem_cache_init(void)
local_irq_enable();
ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
+ BUG_ON(!ptr);
local_irq_disable();
BUG_ON(ac_data(malloc_sizes[INDEX_AC].cs_cachep)
--
Luiz Fernando N. Capitulino
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] slab: Adds two missing kmalloc() checks.
2006-01-23 15:31 [PATCH] slab: Adds two missing kmalloc() checks Luiz Fernando Capitulino
@ 2006-01-23 16:38 ` Pekka Enberg
2006-01-23 16:44 ` Arjan van de Ven
0 siblings, 1 reply; 6+ messages in thread
From: Pekka Enberg @ 2006-01-23 16:38 UTC (permalink / raw)
To: Luiz Fernando Capitulino; +Cc: akpm, lkml, arjan
Hi,
On Mon, 2006-01-23 at 13:31 -0200, Luiz Fernando Capitulino wrote:
> Adds two missing kmalloc() checks in kmem_cache_init(). The check is worth
> because if kmalloc() fails we'll have a nice message instead of a OOPS (which
> will make us look for a bug).
>
> We're using BUG_ON() so that embedded people can disable it.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@mandriva.com.br>
Looks good to me. Arjan, you had some objections last time around. Are
you okay with the change?
Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
Pekka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] slab: Adds two missing kmalloc() checks.
2006-01-23 16:38 ` Pekka Enberg
@ 2006-01-23 16:44 ` Arjan van de Ven
2006-01-23 16:52 ` Pekka Enberg
0 siblings, 1 reply; 6+ messages in thread
From: Arjan van de Ven @ 2006-01-23 16:44 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Luiz Fernando Capitulino, akpm, lkml
On Mon, 2006-01-23 at 18:38 +0200, Pekka Enberg wrote:
> Hi,
>
> On Mon, 2006-01-23 at 13:31 -0200, Luiz Fernando Capitulino wrote:
> > Adds two missing kmalloc() checks in kmem_cache_init(). The check is worth
> > because if kmalloc() fails we'll have a nice message instead of a OOPS (which
> > will make us look for a bug).
> >
> > We're using BUG_ON() so that embedded people can disable it.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@mandriva.com.br>
>
> Looks good to me. Arjan, you had some objections last time around. Are
> you okay with the change?
I still fail to see the point. Has anyone EVER seen these trigger????
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] slab: Adds two missing kmalloc() checks.
2006-01-23 16:44 ` Arjan van de Ven
@ 2006-01-23 16:52 ` Pekka Enberg
2006-01-23 17:01 ` Luiz Fernando Capitulino
0 siblings, 1 reply; 6+ messages in thread
From: Pekka Enberg @ 2006-01-23 16:52 UTC (permalink / raw)
To: Arjan van de Ven; +Cc: Luiz Fernando Capitulino, akpm, lkml
On Mon, 2006-01-23 at 18:38 +0200, Pekka Enberg wrote:
> > Looks good to me. Arjan, you had some objections last time around. Are
> > you okay with the change?
On Mon, 2006-01-23 at 17:44 +0100, Arjan van de Ven wrote:
> I still fail to see the point. Has anyone EVER seen these trigger????
Yeah, we probably won't. They seem useful for people who hunt unchecked
kmalloc() calls, though.
Pekka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] slab: Adds two missing kmalloc() checks.
2006-01-23 16:52 ` Pekka Enberg
@ 2006-01-23 17:01 ` Luiz Fernando Capitulino
2006-01-23 17:06 ` Arjan van de Ven
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Fernando Capitulino @ 2006-01-23 17:01 UTC (permalink / raw)
To: Pekka Enberg; +Cc: arjan, akpm, linux-kernel
Hi Pekka, Arjan,
On Mon, 23 Jan 2006 18:52:02 +0200
Pekka Enberg <penberg@cs.helsinki.fi> wrote:
| On Mon, 2006-01-23 at 18:38 +0200, Pekka Enberg wrote:
| > > Looks good to me. Arjan, you had some objections last time around. Are
| > > you okay with the change?
|
| On Mon, 2006-01-23 at 17:44 +0100, Arjan van de Ven wrote:
| > I still fail to see the point. Has anyone EVER seen these trigger????
|
| Yeah, we probably won't. They seem useful for people who hunt unchecked
| kmalloc() calls, though.
It really looks useful to me. You don't check for fail because someone has
seen the fail happen. You check for fail in order to have a robust program.
There are zilions of checks in the kernel and in programs out there which I
think they will never fail. But they are there.
On the other hand, I'm not going to make too much noise for a such trivial
patch. If you think it's not useful, let's drop it. No problem.
--
Luiz Fernando N. Capitulino
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] slab: Adds two missing kmalloc() checks.
2006-01-23 17:01 ` Luiz Fernando Capitulino
@ 2006-01-23 17:06 ` Arjan van de Ven
0 siblings, 0 replies; 6+ messages in thread
From: Arjan van de Ven @ 2006-01-23 17:06 UTC (permalink / raw)
To: Luiz Fernando Capitulino; +Cc: Pekka Enberg, akpm, linux-kernel
On Mon, 2006-01-23 at 15:01 -0200, Luiz Fernando Capitulino wrote:
> Hi Pekka, Arjan,
>
> On Mon, 23 Jan 2006 18:52:02 +0200
> Pekka Enberg <penberg@cs.helsinki.fi> wrote:
>
> | On Mon, 2006-01-23 at 18:38 +0200, Pekka Enberg wrote:
> | > > Looks good to me. Arjan, you had some objections last time around. Are
> | > > you okay with the change?
> |
> | On Mon, 2006-01-23 at 17:44 +0100, Arjan van de Ven wrote:
> | > I still fail to see the point. Has anyone EVER seen these trigger????
> |
> | Yeah, we probably won't. They seem useful for people who hunt unchecked
> | kmalloc() calls, though.
>
> It really looks useful to me. You don't check for fail because someone has
> seen the fail happen. You check for fail in order to have a robust program.
Generally you are right. You check for fail because you can recover the
failure. In this case the code happens during real early boot, and if it
fails you CANNOT BOOT. And you have so little memory, that it's highly
unlikely that you even got this far. -> You only make the kernel bigger
without any win at all.
Don't get me wrong. Most of the null pointer checks are useful. Just the
ones where you can't recover ANYWAY are not. A BUG_ON() is not better
than just hitting a GPF due to a null pointer, in either case you don't
boot, and probably so early you don't get to see the message either, and
when you do see it.. you get no additional information.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-01-23 17:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-23 15:31 [PATCH] slab: Adds two missing kmalloc() checks Luiz Fernando Capitulino
2006-01-23 16:38 ` Pekka Enberg
2006-01-23 16:44 ` Arjan van de Ven
2006-01-23 16:52 ` Pekka Enberg
2006-01-23 17:01 ` Luiz Fernando Capitulino
2006-01-23 17:06 ` Arjan van de Ven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox