Skip to contentSkip to conversation

check for mem in slab

QUESTION New Cyrill Gorcunov 4 participants 7 messages Mar 29, 2007 – Apr 2, 2007 Memory management

lore
Share & tools
lore

Cyrill Gorcunov

author
#0 lore raw

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

#1 lore raw
Quoted 10 lines

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 <--

[...]

Quoted 1 line

is that OK? or it's a bug?

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

#2 lore raw
Quoted 7 lines · Heiko Carstens
Quoted 7 lines

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;
Quoted 7 lines
Quoted 3 lines
              ptr = kmalloc(sizeof(struct arraycache_init), GFP_KERNEL);

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

[...]

Quoted 1 line

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.

author
#3 lore raw

[Pekka Enberg - Fri, Mar 30, 2007 at 02:55:26PM +0300]

| On 3/30/07, Heiko Carstens <[email protected]> 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

#4 lore raw

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

In reply to Pekka Enberg

Hi,

Quoted 10 lines · Heiko Carstens
Quoted 9 lines
Quoted 9 lines

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);
Quoted 8 lines
Quoted 5 lines
Quoted 1 line

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

[...]

Quoted 1 line

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 <[email protected]>
mm/slab.c +2 -0
index 57f7aa4..6d7e486 100644
@@ -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)
#5 lore raw

PATCH 1/1 Catch kmalloc() failure in kmem_cache_init()

In reply to Cyrill Gorcunov

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 <[email protected]>
mm/slab.c +2 -0
index 57f7aa4..6d7e486 100644
@@ -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)
Acked-by on v1 1/1
#6 lore raw

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

Quoted 3 lines · Johannes Weiner

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 <[email protected]>