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 <[email protected]>
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
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 <[email protected]>
Looks good to me. Arjan, you had some objections last time around. Are
you okay with the change?
Acked-by: Pekka Enberg <[email protected]>
Pekka
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 <[email protected]>
>
> 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????
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
Hi Pekka, Arjan,
On Mon, 23 Jan 2006 18:52:02 +0200
Pekka Enberg <[email protected]> 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
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 <[email protected]> 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.