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

login
register
mail settings
Submitter Christoph Lameter
Date July 31, 2013, 3:45 p.m.
Message ID <000001403567762a-60a27288-f0b2-4855-b88c-6a6f21ec537c-000000@email.amazonses.com>
Download mbox | patch
Permalink /patch/263760/
State New
Headers show

Comments

Christoph Lameter - July 31, 2013, 3:45 p.m.
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>
Wladislav Wiebe - July 31, 2013, 4:33 p.m.
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.
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.
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.
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.

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;