diff mbox

mm/slab: ppc: ubi: kmalloc_slab WARNING / PPC + UBI driver

Message ID 000001403567762a-60a27288-f0b2-4855-b88c-6a6f21ec537c-000000@email.amazonses.com
State New, archived
Headers show

Commit Message

Christoph Lameter July 31, 2013, 3:45 p.m. UTC
Crap you cannot do PAGE_SIZE allocations with kmalloc_large. Fails when
freeing pages. Need to only do the multiple page allocs with
kmalloc_large.

Subject: seq_file: Use kmalloc_large for page sized allocation

There is no point in using the slab allocation functions for
large page order allocation. Use kmalloc_large().

This fixes the warning about large allocs but it will still cause
large contiguous allocs that could fail because of memory fragmentation.

Signed-off-by: Christoph Lameter <cl@linux.com>

Comments

Wladislav Wiebe July 31, 2013, 4:33 p.m. UTC | #1
Hi Christoph,

On 31/07/13 17:45, Christoph Lameter wrote:
> Crap you cannot do PAGE_SIZE allocations with kmalloc_large. Fails when
> freeing pages. Need to only do the multiple page allocs with
> kmalloc_large.
> 
> Subject: seq_file: Use kmalloc_large for page sized allocation
> 
> There is no point in using the slab allocation functions for
> large page order allocation. Use kmalloc_large().
> 
> This fixes the warning about large allocs but it will still cause
> large contiguous allocs that could fail because of memory fragmentation.

Thanks for the point, do you plan to make kmalloc_large available for extern access in a separate mainline patch?
Since kmalloc_large is statically defined in slub_def.h and when including it to seq_file.c
we have a lot of conflicting types:
..
In file included from ../linux/fs/seq_file.c:8:0:
../linux/include/linux/slub_def.h: In function 'kmalloc':
../linux/include/linux/slub_def.h:161:14: error: 'KMALLOC_MAX_CACHE_SIZE' undeclared (first use in this function)
../results/linux/include/linux/slub_def.h:161:14: note: each undeclared identifier is reported only once for each function it appears in
../linux/include/linux/slub_def.h:165:4: error: implicit declaration of function 'kmalloc_index' [-Werror=implicit-function-declaration]
../linux/include/linux/slub_def.h:168:12: error: 'ZERO_SIZE_PTR' undeclared (first use in this function)
../linux/include/linux/slub_def.h:170:34: error: 'kmalloc_caches' undeclared (first use in this function)
..


Thanks & BR
Wladislav Wiebe

> 
> Signed-off-by: Christoph Lameter <cl@linux.com>
> 
> Index: linux/fs/seq_file.c
> ===================================================================
> --- linux.orig/fs/seq_file.c	2013-07-31 10:39:03.050472030 -0500
> +++ linux/fs/seq_file.c	2013-07-31 10:39:03.050472030 -0500
> @@ -136,7 +136,7 @@ static int traverse(struct seq_file *m,
>  Eoverflow:
>  	m->op->stop(m, p);
>  	kfree(m->buf);
> -	m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +	m->buf = kmalloc_large(m->size <<= 1, GFP_KERNEL);
>  	return !m->buf ? -ENOMEM : -EAGAIN;
>  }
> 
> @@ -232,7 +232,7 @@ ssize_t seq_read(struct file *file, char
>  			goto Fill;
>  		m->op->stop(m, p);
>  		kfree(m->buf);
> -		m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
> +		m->buf = kmalloc_large(m->size <<= 1, GFP_KERNEL);
>  		if (!m->buf)
>  			goto Enomem;
>  		m->count = 0;
>
Christoph Lameter July 31, 2013, 5:04 p.m. UTC | #2
On Wed, 31 Jul 2013, Wladislav Wiebe wrote:

> Thanks for the point, do you plan to make kmalloc_large available for extern access in a separate mainline patch?
> Since kmalloc_large is statically defined in slub_def.h and when including it to seq_file.c
> we have a lot of conflicting types:

You cannot separatly include slub_def.h. slab.h includes slub_def.h for
you. What problem did you try to fix by doing so?

There is a patch pending that moves kmalloc_large to slab.h. So maybe we
have to wait a merge period in order to be able to use it with other
allocators than slub.
Wladislav Wiebe Aug. 6, 2013, 7:15 a.m. UTC | #3
Hi,

On 31/07/13 19:04, Christoph Lameter wrote:
> On Wed, 31 Jul 2013, Wladislav Wiebe wrote:
> 
>> Thanks for the point, do you plan to make kmalloc_large available for extern access in a separate mainline patch?
>> Since kmalloc_large is statically defined in slub_def.h and when including it to seq_file.c
>> we have a lot of conflicting types:
> 
> You cannot separatly include slub_def.h. slab.h includes slub_def.h for
> you. What problem did you try to fix by doing so?
> 
> There is a patch pending that moves kmalloc_large to slab.h. So maybe we
> have to wait a merge period in order to be able to use it with other
> allocators than slub.
> 
> 

ok, just saw in slab/for-linus branch that those stuff is reverted again..

commit 4932163637fbb9aaa654ca0703c5a624b7809da2
Author: Pekka Enberg <penberg@kernel.org>
Date:   Wed Jul 10 10:16:01 2013 +0300

    Revert "mm/sl[aou]b: Move kmalloc_node functions to common code"

..

commit 35be03cafb8f5ddcc1236e90144b6ec76296b789
Author: Pekka Enberg <penberg@kernel.org>
Date:   Wed Jul 10 09:56:49 2013 +0300

    Revert "mm/sl[aou]b: Move kmalloc definitions to slab.h"


--
WBR, WLadislav Wiebe
Christoph Lameter Aug. 6, 2013, 2:57 p.m. UTC | #4
On Tue, 6 Aug 2013, Wladislav Wiebe wrote:

> ok, just saw in slab/for-linus branch that those stuff is reverted again..

No that was only for the 3.11 merge by Linus. The 3.12 patches have not
been put into pekkas tree.
diff mbox

Patch

Index: linux/fs/seq_file.c
===================================================================
--- linux.orig/fs/seq_file.c	2013-07-31 10:39:03.050472030 -0500
+++ linux/fs/seq_file.c	2013-07-31 10:39:03.050472030 -0500
@@ -136,7 +136,7 @@  static int traverse(struct seq_file *m,
 Eoverflow:
 	m->op->stop(m, p);
 	kfree(m->buf);
-	m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
+	m->buf = kmalloc_large(m->size <<= 1, GFP_KERNEL);
 	return !m->buf ? -ENOMEM : -EAGAIN;
 }

@@ -232,7 +232,7 @@  ssize_t seq_read(struct file *file, char
 			goto Fill;
 		m->op->stop(m, p);
 		kfree(m->buf);
-		m->buf = kmalloc(m->size <<= 1, GFP_KERNEL);
+		m->buf = kmalloc_large(m->size <<= 1, GFP_KERNEL);
 		if (!m->buf)
 			goto Enomem;
 		m->count = 0;