2004-11-10 05:19:41

by Carl-Daniel Hailfinger

[permalink] [raw]
Subject: [RFC] [PATCH] kmem_alloc (generic wrapper for kmalloc and vmalloc)

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

--- ./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);
+ }
+}
+


2004-11-10 06:02:34

by Robert Love

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

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


2004-11-10 06:31:58

by Carl-Daniel Hailfinger

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

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

2004-11-10 06:57:04

by Robert Love

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

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


2004-11-10 07:11:04

by Nick Piggin

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

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.

2004-11-10 07:55:35

by Jens Axboe

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

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

2004-11-10 13:37:11

by Geert Uytterhoeven

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

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

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

2004-11-10 17:03:35

by Carl-Daniel Hailfinger

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

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

2004-11-10 17:17:11

by Roland Dreier

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

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.

2004-11-10 17:55:19

by Adam Heath

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

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?

2004-11-10 18:05:06

by Jens Axboe

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

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