[RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Carl-Daniel Hailfinger]

Hi,

it seems there is a bunch of drivers which want to allocate memory as
efficiently as possible in a wide range of allocation sizes. XFS and
NTFS seem to be examples. Implement a generic wrapper to reduce code
duplication.
Functions have the my_ prefixes to avoid name clash with XFS.

Patch is compile tested

Comments/flames?

Regards,
Carl-Daniel
http://www.hailfinger.org/

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.kernel.2004@gmx.net>

--- ./linux-2.6.9/mm/slab.c~	2004-11-05 14:27:49.000000000 +0100
+++ ./linux-2.6.9/mm/slab.c	2004-11-10 03:27:19.000000000 +0100
@@ -507,6 +507,8 @@
 #undef CACHE
 };

+size_t MAX_KMALLOC_SIZE;
+
 static struct arraycache_init initarray_cache __initdata =
 	{ { 0, BOOT_CPUCACHE_ENTRIES, 1, 0} };
 static struct arraycache_init initarray_generic __initdata =
@@ -728,6 +730,10 @@
 	struct cache_sizes *sizes;
 	struct cache_names *names;

+#define CACHE(x) MAX_KMALLOC_SIZE = x;
+#include <linux/kmalloc_sizes.h>
+#undef CACHE
+
 	/*
 	 * Fragmentation resistance on low memory - only use bigger
 	 * page orders on machines with more than 32MB of memory.
@@ -3039,3 +3045,25 @@

 	return size;
 }
+
+void * my_kmem_alloc(size_t size, int flags)
+{
+	void	*ptr = NULL;
+
+	if (size < MAX_KMALLOC_SIZE)
+		ptr = kmalloc(size, flags);
+	if (!ptr)
+		ptr = __vmalloc(size, flags, PAGE_KERNEL);
+	return ptr;
+}
+
+void my_kmem_free(void *ptr)
+{
+	if (((unsigned long)ptr < VMALLOC_START) ||
+	    ((unsigned long)ptr >= VMALLOC_END)) {
+		kfree(ptr);
+	} else {
+		vfree(ptr);
+	}
+}
+

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Robert Love]

On Wed, 2004-11-10 at 06:19 +0100, Carl-Daniel Hailfinger wrote:
> Hi,
> 
> it seems there is a bunch of drivers which want to allocate memory as
> efficiently as possible in a wide range of allocation sizes. XFS and
> NTFS seem to be examples. Implement a generic wrapper to reduce code
> duplication.
> Functions have the my_ prefixes to avoid name clash with XFS.

No, no, no.  A good patch would be fixing places where you see this.

Code needs to conscientiously decide to use vmalloc over kmalloc.  The
behavior is different and the choice needs to be explicit.

	Robert Love
 

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Carl-Daniel Hailfinger]

Robert Love schrieb:
> On Wed, 2004-11-10 at 06:19 +0100, Carl-Daniel Hailfinger wrote:
> 
>>Hi,
>>
>>it seems there is a bunch of drivers which want to allocate memory as
>>efficiently as possible in a wide range of allocation sizes. XFS and
>>NTFS seem to be examples. Implement a generic wrapper to reduce code
>>duplication.
>>Functions have the my_ prefixes to avoid name clash with XFS.
> 
> 
> No, no, no.  A good patch would be fixing places where you see this.
> 
> Code needs to conscientiously decide to use vmalloc over kmalloc.  The
> behavior is different and the choice needs to be explicit.

Yes, but what do you suggest for the following problem:
alloc(max_loop*sizeof(struct loop_device))

where sizeof(struct loop_device)==304 and 1<=max_loop<=16384

For the smallest allocation (304 bytes) vmalloc is clearly wasteful
and for the largest allocation (~ 5 MBytes) kmalloc doesn't work.


Regards,
Carl-Daniel

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Robert Love]

On Wed, 2004-11-10 at 07:31 +0100, Carl-Daniel Hailfinger wrote:

> Yes, but what do you suggest for the following problem:
> alloc(max_loop*sizeof(struct loop_device))
> 
> where sizeof(struct loop_device)==304 and 1<=max_loop<=16384
> 
> For the smallest allocation (304 bytes) vmalloc is clearly wasteful
> and for the largest allocation (~ 5 MBytes) kmalloc doesn't work.

Stab in the dark: Break it into two separate loops?

Two loops would be faster than the branch on each alloc, too.

Even better: Re-architect so as not to need that mess at all?

	Robert Love

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Nick Piggin]

Carl-Daniel Hailfinger wrote:
> Robert Love schrieb:
> 
>>On Wed, 2004-11-10 at 06:19 +0100, Carl-Daniel Hailfinger wrote:
>>
>>
>>>Hi,
>>>
>>>it seems there is a bunch of drivers which want to allocate memory as
>>>efficiently as possible in a wide range of allocation sizes. XFS and
>>>NTFS seem to be examples. Implement a generic wrapper to reduce code
>>>duplication.
>>>Functions have the my_ prefixes to avoid name clash with XFS.
>>
>>
>>No, no, no.  A good patch would be fixing places where you see this.
>>
>>Code needs to conscientiously decide to use vmalloc over kmalloc.  The
>>behavior is different and the choice needs to be explicit.
> 
> 
> Yes, but what do you suggest for the following problem:
> alloc(max_loop*sizeof(struct loop_device))
> 
> where sizeof(struct loop_device)==304 and 1<=max_loop<=16384
> 
> For the smallest allocation (304 bytes) vmalloc is clearly wasteful
> and for the largest allocation (~ 5 MBytes) kmalloc doesn't work.
> 

Can't you change it to use a hash or something?

Even a linked list if it is not performance critical.

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Geert Uytterhoeven]

On Wed, 10 Nov 2004, Carl-Daniel Hailfinger wrote:
> Robert Love schrieb:
> > On Wed, 2004-11-10 at 06:19 +0100, Carl-Daniel Hailfinger wrote:
> >>it seems there is a bunch of drivers which want to allocate memory as
> >>efficiently as possible in a wide range of allocation sizes. XFS and
> >>NTFS seem to be examples. Implement a generic wrapper to reduce code
> >>duplication.
> >>Functions have the my_ prefixes to avoid name clash with XFS.
> > 
> > 
> > No, no, no.  A good patch would be fixing places where you see this.
> > 
> > Code needs to conscientiously decide to use vmalloc over kmalloc.  The
> > behavior is different and the choice needs to be explicit.
> 
> Yes, but what do you suggest for the following problem:
> alloc(max_loop*sizeof(struct loop_device))
> 
> where sizeof(struct loop_device)==304 and 1<=max_loop<=16384
> 
> For the smallest allocation (304 bytes) vmalloc is clearly wasteful
> and for the largest allocation (~ 5 MBytes) kmalloc doesn't work.

Try vmalloc() if kmalloc() fails?

BTW, is there a simple and portable way to distinguish between memory allocated
using kmalloc() and vmalloc(), or should the called remember the allocation
method?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Jens Axboe]

On Wed, Nov 10 2004, Robert Love wrote:
> On Wed, 2004-11-10 at 06:19 +0100, Carl-Daniel Hailfinger wrote:
> > Hi,
> > 
> > it seems there is a bunch of drivers which want to allocate memory as
> > efficiently as possible in a wide range of allocation sizes. XFS and
> > NTFS seem to be examples. Implement a generic wrapper to reduce code
> > duplication.
> > Functions have the my_ prefixes to avoid name clash with XFS.
> 
> No, no, no.  A good patch would be fixing places where you see this.
> 
> Code needs to conscientiously decide to use vmalloc over kmalloc.  The
> behavior is different and the choice needs to be explicit.

Plus, you cannot use vfree() from interrupt context. This patch is a bad
idea.

-- 
Jens Axboe

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Carl-Daniel Hailfinger]

Jens Axboe wrote:
> On Wed, Nov 10 2004, Robert Love wrote:
> 
>>On Wed, 2004-11-10 at 06:19 +0100, Carl-Daniel Hailfinger wrote:
>>
>>>it seems there is a bunch of drivers which want to allocate memory as
>>>efficiently as possible in a wide range of allocation sizes. XFS and
>>>NTFS seem to be examples. Implement a generic wrapper to reduce code
>>>duplication.
>>>Functions have the my_ prefixes to avoid name clash with XFS.
>>
>>No, no, no.  A good patch would be fixing places where you see this.
>>
>>Code needs to conscientiously decide to use vmalloc over kmalloc.  The
>>behavior is different and the choice needs to be explicit.
> 
> Plus, you cannot use vfree() from interrupt context. This patch is a bad
> idea.

OK, so how should I allocate memory for 512 struct loop_device's?
Because of its odd size (304 bytes) it seems that if I use kmalloc
seperately for each struct, I'd waste 208 bytes per allocation. 68%
overhead would be a step backwards. Or am I missing something here?


Regards,
Carl-Daniel

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Roland Dreier]

    Carl-Daniel> OK, so how should I allocate memory for 512 struct
    Carl-Daniel> loop_device's?  Because of its odd size (304 bytes)
    Carl-Daniel> it seems that if I use kmalloc seperately for each
    Carl-Daniel> struct, I'd waste 208 bytes per allocation. 68%
    Carl-Daniel> overhead would be a step backwards. Or am I missing
    Carl-Daniel> something here?

Would creating your own slab cache with kmem_cache_create() and
allocating with kmem_cache_alloc() work?  You can fit 13 304 byte
objects in a 4K page, which wastes 4096 - 13*304 = 144 bytes per page
or about 11 bytes per object.  Less than 4% overhead seems OK to me.

 - R.

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Adam Heath]

On Wed, 10 Nov 2004, Jens Axboe wrote:

> Plus, you cannot use vfree() from interrupt context. This patch is a bad
> idea.

why not have a work queue, that frees things later, after a delay?

Re: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

[Jens Axboe]

On Wed, Nov 10 2004, Adam Heath wrote:
> On Wed, 10 Nov 2004, Jens Axboe wrote:
> 
> > Plus, you cannot use vfree() from interrupt context. This patch is a bad
> > idea.
> 
> why not have a work queue, that frees things later, after a delay?

Kludge upon kludge does not make the interface any better. The best fix
is to split the allocations, like suggested a slab cache for the loop
would do nicely.

-- 
Jens Axboe