[PATCH] Allow arch override for kmem_bufctl_t

[PATCH] Allow arch override for kmem_bufctl_t

[Manfred Spraul]

Hi Andrew,

The slab allocator keeps track of the free objects in a slab with a 
linked list of integers (typedef'ed to kmem_bufctl_t). Right now 
unsigned int is used for kmem_bufctl_t, i.e. 4 bytes per-object overhead.
The attached patch allows an arch override for this type: Theoretically, 
unsigned short is sufficient for kmem_bufctl_t and this would reduce the 
per-object overhead to 2 bytes. But some archs cannot operate on 16-bit 
values efficiently, thus it's not possible to switch everyone to ushort. 
The patch switches i386, the port maintainers should switch their arch 
if there are no problems with 16-bit quantities. All accesses to 
kmem_bufctl_t are under a per-cache spinlock and not in the hottest path.

Could you apply it to -mm?

--
    Manfred

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Manfred Spraul]

[-- Attachment #1: Type: text/plain, Size: 47 bytes --]

Attached is the patch - sorry.

--
    Manfred

[-- Attachment #2: patch-slab-mm-02-bufctl --]
[-- Type: text/plain, Size: 1352 bytes --]

// $Header$
// Kernel Version:
//  VERSION = 2
//  PATCHLEVEL = 6
//  SUBLEVEL = 6
//  EXTRAVERSION = -mm2
--- 2.6/mm/slab.c	2004-05-15 11:38:22.000000000 +0200
+++ build-2.6/mm/slab.c	2004-05-15 13:01:47.000000000 +0200
@@ -161,10 +161,9 @@
  * is less than 512 (PAGE_SIZE<<3), but greater than 256.
  */
 
-#define BUFCTL_END	0xffffFFFF
-#define BUFCTL_FREE	0xffffFFFE
-#define	SLAB_LIMIT	0xffffFFFD
-typedef unsigned int kmem_bufctl_t;
+#define BUFCTL_END	(((kmem_bufctl_t)(~0U))-0)
+#define BUFCTL_FREE	(((kmem_bufctl_t)(~0U))-1)
+#define	SLAB_LIMIT	(((kmem_bufctl_t)(~0U))-2)
 
 /* Max number of objs-per-slab for caches which use off-slab slabs.
  * Needed to avoid a possible looping condition in cache_grow().
--- 2.6/include/linux/types.h	2004-05-15 11:21:51.000000000 +0200
+++ build-2.6/include/linux/types.h	2004-05-15 12:57:29.000000000 +0200
@@ -140,6 +140,9 @@
 #define pgoff_t unsigned long
 #endif
 
+#ifndef HAVE_KMEM_BUFCTL_T
+typedef unsigned int kmem_bufctl_t;
+#endif
 #endif /* __KERNEL_STRICT_NAMES */
 
 /*
--- 2.6/include/asm-i386/types.h	2004-01-09 07:59:44.000000000 +0100
+++ build-2.6/include/asm-i386/types.h	2004-05-15 12:57:51.000000000 +0200
@@ -63,6 +63,9 @@
 #define HAVE_SECTOR_T
 #endif
 
+#define HAVE_KMEM_BUFCTL_T 1
+typedef unsigned short kmem_bufctl_t;
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* __KERNEL__ */

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Andi Kleen]

> --- 2.6/include/linux/types.h	2004-05-15 11:21:51.000000000 +0200
> +++ build-2.6/include/linux/types.h	2004-05-15 12:57:29.000000000 +0200
> @@ -140,6 +140,9 @@
>  #define pgoff_t unsigned long
>  #endif
>  
> +#ifndef HAVE_KMEM_BUFCTL_T
> +typedef unsigned int kmem_bufctl_t;
> +#endif

Please remove the ifdef and add type to every arch's asm-*/posix_types.h

-Andi

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Geert Uytterhoeven]

On Sat, 15 May 2004, Manfred Spraul wrote:
> The slab allocator keeps track of the free objects in a slab with a
> linked list of integers (typedef'ed to kmem_bufctl_t). Right now
> unsigned int is used for kmem_bufctl_t, i.e. 4 bytes per-object overhead.
> The attached patch allows an arch override for this type: Theoretically,
> unsigned short is sufficient for kmem_bufctl_t and this would reduce the
> per-object overhead to 2 bytes. But some archs cannot operate on 16-bit
> values efficiently, thus it's not possible to switch everyone to ushort.
> The patch switches i386, the port maintainers should switch their arch
> if there are no problems with 16-bit quantities. All accesses to
> kmem_bufctl_t are under a per-cache spinlock and not in the hottest path.

16-bit is no problem for m68k, so feel free to switch it.

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: [PATCH] Allow arch override for kmem_bufctl_t

[Ralf Baechle]

On Mon, May 17, 2004 at 11:11:59AM +0200, Geert Uytterhoeven wrote:

> > if there are no problems with 16-bit quantities. All accesses to
> > kmem_bufctl_t are under a per-cache spinlock and not in the hottest path.
> 
> 16-bit is no problem for m68k, so feel free to switch it.

No problem for MIPS either.

  Ralf

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Ivan Kokshaysky]

On Mon, May 17, 2004 at 11:11:59AM +0200, Geert Uytterhoeven wrote:
> 16-bit is no problem for m68k, so feel free to switch it.

Ditto for Alpha.

Ivan.

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Andi Kleen]

On Mon, May 17, 2004 at 03:39:52PM +0400, Ivan Kokshaysky wrote:
> On Mon, May 17, 2004 at 11:11:59AM +0200, Geert Uytterhoeven wrote:
> > 16-bit is no problem for m68k, so feel free to switch it.
> 
> Ditto for Alpha.

x86_64 has no problems with 16 bit neither.

-Andi

Re: [PATCH] Allow arch override for kmem_bufctl_t

[David Mosberger]

>>>>> On Sat, 15 May 2004 14:13:56 +0200, Manfred Spraul <manfred@colorfullife.com> said:

  Manfred> Hi Andrew, The slab allocator keeps track of the free
  Manfred> objects in a slab with a linked list of integers
  Manfred> (typedef'ed to kmem_bufctl_t). Right now unsigned int is
  Manfred> used for kmem_bufctl_t, i.e. 4 bytes per-object overhead.
  Manfred> The attached patch allows an arch override for this type:
  Manfred> Theoretically, unsigned short is sufficient for
  Manfred> kmem_bufctl_t and this would reduce the per-object overhead
  Manfred> to 2 bytes. But some archs cannot operate on 16-bit values
  Manfred> efficiently, thus it's not possible to switch everyone to
  Manfred> ushort.  The patch switches i386, the port maintainers
  Manfred> should switch their arch if there are no problems with
  Manfred> 16-bit quantities. All accesses to kmem_bufctl_t are under
  Manfred> a per-cache spinlock and not in the hottest path.

  Manfred> Could you apply it to -mm?

There was no patch attached.  I suspect we'd want to turn this on for
ia64 as well.  Do you have a good (worst-case) benchmark to evaluate
this change?

	--david

Re: [PATCH] Allow arch override for kmem_bufctl_t

[David S. Miller]

On Sat, 15 May 2004 14:13:56 +0200
Manfred Spraul <manfred@colorfullife.com> wrote:

> The attached patch allows an arch override for this type: Theoretically, 
> unsigned short is sufficient for kmem_bufctl_t and this would reduce the 
> per-object overhead to 2 bytes. But some archs cannot operate on 16-bit 
> values efficiently, thus it's not possible to switch everyone to ushort. 

Just out of curiosity, which arch knowingly has the problem?

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Matthew Wilcox]

On Mon, May 17, 2004 at 02:16:35PM -0700, David S. Miller wrote:
> Just out of curiosity, which arch knowingly has the problem?

I think it's ARM ...

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

Re: [PATCH] Allow arch override for kmem_bufctl_t

[Manfred Spraul]

Matthew Wilcox wrote:

>On Mon, May 17, 2004 at 02:16:35PM -0700, David S. Miller wrote:
>  
>
>>Just out of curiosity, which arch knowingly has the problem?
>>    
>>
>
>I think it's ARM ...
>
>  
>
I thought it was sparc and perhaps alpha, but I was wrong: it was a 
general comment from Mark Hemment (May 2000):

> Making s_free a short will really hurt performance.  So to does short 
> bufctls.  Don't worry too much about wasting a little memory (internal 
> fragmentation) for performance.  "shorts" are a performance hit (more 
> so on some architectures - Pentium Pros and definetly on true RISC).


--
    Manfred