[PATCH] slab: respect architecture and caller mandated alignment

[PATCH] slab: respect architecture and caller mandated alignment

[Pekka J Enberg]

From: Pekka Enberg <penberg@cs.helsinki.fi>

As explained by Heiko, on s390 (32-bit) ARCH_KMALLOC_MINALIGN is set to eight
because their common I/O layer allocates data structures that need to have an
eight byte alignment. This does not work when CONFIG_SLAB_DEBUG is enabled
because kmem_cache_create will override alignment to BYTES_PER_WORD which is
four.

So change kmem_cache_create to ensure cache alignment is always at minimum
what the architecture or caller mandates even if slab debugging is enabled.

Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

 mm/slab.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Index: 2.6/mm/slab.c
===================================================================
--- 2.6.orig/mm/slab.c
+++ 2.6/mm/slab.c
@@ -2097,6 +2097,15 @@ kmem_cache_create (const char *name, siz
 	} else {
 		ralign = BYTES_PER_WORD;
 	}
+
+	/*
+	 * Redzoning and user store require word alignment. Note this will be
+	 * overridden by architecture or caller mandated alignment if either
+	 * is greater than BYTES_PER_WORD.
+	 */
+	if (flags & SLAB_RED_ZONE || flags & SLAB_STORE_USER)
+		ralign = BYTES_PER_WORD;
+
 	/* 2) arch mandated alignment: disables debug if necessary */
 	if (ralign < ARCH_SLAB_MINALIGN) {
 		ralign = ARCH_SLAB_MINALIGN;
@@ -2110,8 +2119,7 @@ kmem_cache_create (const char *name, siz
 			flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	}
 	/*
-	 * 4) Store it. Note that the debug code below can reduce
-	 *    the alignment to BYTES_PER_WORD.
+	 * 4) Store it.
 	 */
 	align = ralign;
 
@@ -2123,20 +2131,19 @@ kmem_cache_create (const char *name, siz
 #if DEBUG
 	cachep->obj_size = size;
 
+	/*
+	 * Both debugging options require word-alignment which is calculated
+	 * into align above.
+	 */
 	if (flags & SLAB_RED_ZONE) {
-		/* redzoning only works with word aligned caches */
-		align = BYTES_PER_WORD;
-
 		/* add space for red zone words */
 		cachep->obj_offset += BYTES_PER_WORD;
 		size += 2 * BYTES_PER_WORD;
 	}
 	if (flags & SLAB_STORE_USER) {
-		/* user store requires word alignment and
-		 * one word storage behind the end of the real
-		 * object.
+		/* user store requires one word storage behind the end of
+		 * the real object.
 		 */
-		align = BYTES_PER_WORD;
 		size += BYTES_PER_WORD;
 	}
 #if FORCED_DEBUG && defined(CONFIG_DEBUG_PAGEALLOC)

Re: [PATCH] slab: respect architecture and caller mandated alignment

[Christoph Lameter]

On Thu, 27 Jul 2006, Pekka J Enberg wrote:

> As explained by Heiko, on s390 (32-bit) ARCH_KMALLOC_MINALIGN is set to eight
> because their common I/O layer allocates data structures that need to have an
> eight byte alignment. This does not work when CONFIG_SLAB_DEBUG is enabled
> because kmem_cache_create will override alignment to BYTES_PER_WORD which is
> four.
> 
> So change kmem_cache_create to ensure cache alignment is always at minimum
> what the architecture or caller mandates even if slab debugging is enabled.

Note that this will disable SLAB_RED_ZONE and SLAB_STORE_USER 
for the following SLAB_DEBUG cases:

1. For all slabs if an arch sets ARCH_SLAB_MINALIGN > BYTES_PER_WORD

by:

      /* 2) arch mandated alignment: disables debug if necessary */
        if (ralign < ARCH_SLAB_MINALIGN) {
                ralign = ARCH_SLAB_MINALIGN;
                if (ralign > BYTES_PER_WORD)
                        flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
        }

ralign = BYTES_PER_WORD per your change for slab debug.
ARCH_SLAB_MINALIGN > BYTES_PER_WORD -> SLAB_RED_ZONE and SLAB_STORE_USER 
off.

2. For all general (kmalloc) slabs if an arch sets
   ARCH_KMALLOC_MINALIGN > BYTES_PER_WORD

by:

     /* 3) caller mandated alignment: disables debug if necessary */
        if (ralign < align) {
                ralign = align;
                if (ralign > BYTES_PER_WORD)
                        flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
        }

ralign = BYTES_PER_WORD by your change.
align = ARCH_KMALLOC_MINALIGN (passed by kmem_cache_init)
Therefore SLAB_RED_ZONE and SLAB_STORE_USER are always off.

F.e. S/390 will not be able to use slab debug for the general slabs.

You may want to document that change somewhere.


Note that it is not possible to do Redzoning and aligning at the same 
time. Redzoning adds a word before and after the object. If you would 
align it then you would align the whole thing which would result in an 
alignment visible to the slab user of alignment + sizeof(word).

Re: [PATCH] slab: respect architecture and caller mandated alignment

[Heiko Carstens]

On Thu, Jul 27, 2006 at 07:25:56PM -0700, Christoph Lameter wrote:
> On Thu, 27 Jul 2006, Pekka J Enberg wrote:
> 
> > As explained by Heiko, on s390 (32-bit) ARCH_KMALLOC_MINALIGN is set to eight
> > because their common I/O layer allocates data structures that need to have an
> > eight byte alignment. This does not work when CONFIG_SLAB_DEBUG is enabled
> > because kmem_cache_create will override alignment to BYTES_PER_WORD which is
> > four.
> > 
> > So change kmem_cache_create to ensure cache alignment is always at minimum
> > what the architecture or caller mandates even if slab debugging is enabled.
> 
> Note that this will disable SLAB_RED_ZONE and SLAB_STORE_USER 
> for the following SLAB_DEBUG cases:
> 
> 1. For all slabs if an arch sets ARCH_SLAB_MINALIGN > BYTES_PER_WORD
> [...]
> 2. For all general (kmalloc) slabs if an arch sets
>    ARCH_KMALLOC_MINALIGN > BYTES_PER_WORD
> [...]
> F.e. S/390 will not be able to use slab debug for the general slabs.
> 
> You may want to document that change somewhere.

It is already documented (see top of slab.c). The only thing that was wrong was
that ARCH_SLAB_MINALIGN and ARCH_KMALLOC_MINALIGN didn't have the effect like
one would expect from the documentation.

Re: [PATCH] slab: respect architecture and caller mandated alignment

[Christoph Lameter]

On Fri, 28 Jul 2006, Heiko Carstens wrote:

> > You may want to document that change somewhere.
> 
> It is already documented (see top of slab.c). The only thing that was 
> wrong was that ARCH_SLAB_MINALIGN and ARCH_KMALLOC_MINALIGN didn't have 
> the effect like one would expect from the documentation.

Some of the passages there are a bit fuzzy to me. This is the first time 
though that we enforce these alignments for the SLAB_DEBUG case.