Message ID | 000001403567762a-60a27288-f0b2-4855-b88c-6a6f21ec537c-000000@email.amazonses.com |
---|---|
State | New, archived |
Headers | show |
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; >
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.
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
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.
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;
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>