[QUESTION] check for mem in slab

[QUESTION] check for mem in slab

[Cyrill Gorcunov]

Hi list,

in file mm/slab.c and routine kmem_cache_init() I found there
is no checking for allocated memory on line:

	/* 4) Replace the bootstrap head arrays */
	{
		struct array_cache *ptr;

		ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);

--> no check for ptr == NULL <--

		local_irq_disable();
		BUG_ON(cpu_cache_get(&cache_cache) != &initarray_cache.cache);
		memcpy(ptr, cpu_cache_get(&cache_cache),
		       sizeof(struct arraycache_init));

		...

is that OK? or it's a bug?

		Cyrill

Re: [QUESTION] check for mem in slab

[Heiko Carstens]

> in file mm/slab.c and routine kmem_cache_init() I found there
> is no checking for allocated memory on line:
> 
> 	/* 4) Replace the bootstrap head arrays */
> 	{
> 		struct array_cache *ptr;
> 
> 		ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
> 
> --> no check for ptr == NULL <--
[...]
> is that OK? or it's a bug?

It's ok. If that allocation fails your machine won't come up anyway.

Re: [QUESTION] check for mem in slab

[Pekka Enberg]

On 3/30/07, Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> > in file mm/slab.c and routine kmem_cache_init() I found there
> > is no checking for allocated memory on line:
> >
> >       /* 4) Replace the bootstrap head arrays */
> >       {
> >               struct array_cache *ptr;
> >
> >               ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
> >
> > --> no check for ptr == NULL <--
> [...]
> > is that OK? or it's a bug?
>
> It's ok. If that allocation fails your machine won't come up anyway.

We ought to add a BUG_ON or comment at least there as this keeps
popping up again and again.

Re: [QUESTION] check for mem in slab

[Cyrill Gorcunov]

[Pekka Enberg - Fri, Mar 30, 2007 at 02:55:26PM +0300]
| On 3/30/07, Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
| >> in file mm/slab.c and routine kmem_cache_init() I found there
| >> is no checking for allocated memory on line:
| >>
| >>       /* 4) Replace the bootstrap head arrays */
| >>       {
| >>               struct array_cache *ptr;
| >>
| >>               ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
| >>
| >> --> no check for ptr == NULL <--
| >[...]
| >> is that OK? or it's a bug?
| >
| >It's ok. If that allocation fails your machine won't come up anyway.
| 
| We ought to add a BUG_ON or comment at least there as this keeps
| popping up again and again.
| 

Thanks for answer

		Cyrill

[PATCH] Catch kmalloc failure in kmem_cache_init() (was: [QUESTION] check for mem in slab)

[Johannes Weiner]

[-- Attachment #1: Type: text/plain, Size: 730 bytes --]

Hi,

> On 3/30/07, Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> >> in file mm/slab.c and routine kmem_cache_init() I found there
> >> is no checking for allocated memory on line:
> >>
> >>       /* 4) Replace the bootstrap head arrays */
> >>       {
> >>               struct array_cache *ptr;
> >>
> >>               ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
> >>
> >> --> no check for ptr == NULL <--
> >[...]
> >> is that OK? or it's a bug?
> >
> >It's ok. If that allocation fails your machine won't come up anyway.
> 
> We ought to add a BUG_ON or comment at least there as this keeps
> popping up again and again.

Done. Patch attached.

Signed-off-by: Johannes Weiner <hannes-kernel@saeurebad.de>


[-- Attachment #2: slab-cache-init-kmalloc-failure-catch.patch --]
[-- Type: text/plain, Size: 598 bytes --]

diff --git a/mm/slab.c b/mm/slab.c
index 57f7aa4..6d7e486 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1512,6 +1512,7 @@ void __init kmem_cache_init(void)
 		struct array_cache *ptr;
 
 		ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
+		BUG_ON(!ptr);
 
 		local_irq_disable();
 		BUG_ON(cpu_cache_get(&cache_cache) != &initarray_cache.cache);
@@ -1526,6 +1527,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(cpu_cache_get(malloc_sizes[INDEX_AC].cs_cachep)

[PATCH] Catch kmalloc() failure in kmem_cache_init()

[Johannes Weiner]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

Hi,

This patch makes kmem_cache_init() trigger a BUG when the calls to
kmalloc() fail. Altough the kernel would not boot anyway on failure,
this adds some clarity to the code.

Signed-off-by: Johannes Weiner <hannes-kernel@saeurebad.de>


[-- Attachment #2: slab-cache-init-kmalloc-failure-catch.patch --]
[-- Type: text/plain, Size: 598 bytes --]

diff --git a/mm/slab.c b/mm/slab.c
index 57f7aa4..6d7e486 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1512,6 +1512,7 @@ void __init kmem_cache_init(void)
 		struct array_cache *ptr;
 
 		ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);
+		BUG_ON(!ptr);
 
 		local_irq_disable();
 		BUG_ON(cpu_cache_get(&cache_cache) != &initarray_cache.cache);
@@ -1526,6 +1527,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(cpu_cache_get(malloc_sizes[INDEX_AC].cs_cachep)

Re: [PATCH] Catch kmalloc() failure in kmem_cache_init()

[Pekka J Enberg]

On Mon, 2 Apr 2007, Johannes Weiner wrote:
> This patch makes kmem_cache_init() trigger a BUG when the calls to
> kmalloc() fail. Altough the kernel would not boot anyway on failure,
> this adds some clarity to the code.

Indeed. This keeps popping up again and again, so

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>