[patch 1/2] slab: always consider caller mandated alignment

[patch 1/2] slab: always consider caller mandated alignment

[Heiko Carstens, Heiko Carstens]

In case of CONFIG_DEBUG_SLAB kmem_cache_create() creates caches with an
alignment lesser than ARCH_KMALLOC_MINALIGN. This breaks s390 (32bit),
since it needs an eight byte alignment. Also it doesn't behave like it's
decribed in mm/slab.c :

 * Enforce a minimum alignment for the kmalloc caches.
 * Usually, the kmalloc caches are cache_line_size() aligned, except when
 * DEBUG and FORCED_DEBUG are enabled, then they are BYTES_PER_WORD aligned.
 * Some archs want to perform DMA into kmalloc caches and need a guaranteed
 * alignment larger than BYTES_PER_WORD. ARCH_KMALLOC_MINALIGN allows that.
 * Note that this flag disables some debug features.

For example the following might happen if kmem_cache_create() gets called
with -- size: 64; align: 8; flags with SLAB_HWCACHE_ALIGN, SLAB_RED_ZONE and
SLAB_STORE_USER set.
These are the steps as numbered in kmem_cache_create() where 5) is after the
"if (flags & SLAB_RED_ZONE)" statement.

1) align: 8 ralign 64 
2) align: 8 ralign 64 
3) align: 8 ralign 64 
4) align: 64 ralign 64
5) align: 4 ralign 64 

Note that in this case in step 3) the flags SLAB_RED_ZONE and SLAB_STORE_USER
don't get masked out and that this causes an BYTES_PER_WORD alignment in
step 5) which breaks s390.

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

 mm/slab.c |    3 +++
 1 files changed, 3 insertions(+)

Index: linux-2.6/mm/slab.c
===================================================================
--- linux-2.6.orig/mm/slab.c	2006-07-24 09:41:36.000000000 +0200
+++ linux-2.6/mm/slab.c	2006-07-26 09:55:54.000000000 +0200
@@ -2109,6 +2109,9 @@
 		if (ralign > BYTES_PER_WORD)
 			flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	}
+	if (align > BYTES_PER_WORD)
+		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
+
 	/*
 	 * 4) Store it. Note that the debug code below can reduce
 	 *    the alignment to BYTES_PER_WORD.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 2/2] slab: always consider arch mandated alignment

[Heiko Carstens, Heiko Carstens]

Since ARCH_KMALLOC_MINALIGN didn't work on s390 I tried ARCH_SLAB_MINALIGN
instead, just to find out that it didn't work too.
In case of CONFIG_DEBUG_SLAB kmem_cache_create() creates caches with an
alignment lesser than ARCH_SLAB_MINALIGN, which it shouldn't according to
this comment in mm/slab.c :

 * Enforce a minimum alignment for all caches.
 * Intended for archs that get misalignment faults even for BYTES_PER_WORD
 * aligned buffers. Includes ARCH_KMALLOC_MINALIGN.
 * If possible: Do not enable this flag for CONFIG_DEBUG_SLAB, it disables
 * some debug features.

For example the following might happen if kmem_cache_create() gets called
with -- size: 64; align: 0; flags with SLAB_HWCACHE_ALIGN, SLAB_RED_ZONE and
SLAB_STORE_USER set. ARCH_SLAB_MINALIGN is 8.
These are the steps as numbered in kmem_cache_create() where 5) is after the
"if (flags & SLAB_RED_ZONE)" statement.

1) align: 0 ralign 64
2) align: 0 ralign 64
3) align: 0 ralign 64
4) align: 64 ralign 64
5) align: 4 ralign 64

Note that in this case in step 2) the flags SLAB_RED_ZONE and SLAB_STORE_USER
don't get masked out and that this causes an BYTES_PER_WORD alignment in
step 5) which is lesser than ARCH_SLAB_MINALIGN.

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

 mm/slab.c |    3 +++
 1 files changed, 3 insertions(+)

Index: linux-2.6/mm/slab.c
===================================================================
--- linux-2.6.orig/mm/slab.c	2006-07-26 09:55:54.000000000 +0200
+++ linux-2.6/mm/slab.c	2006-07-26 09:57:07.000000000 +0200
@@ -2103,6 +2103,9 @@
 		if (ralign > BYTES_PER_WORD)
 			flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
 	}
+	if (BYTES_PER_WORD < ARCH_SLAB_MINALIGN)
+		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
+
 	/* 3) caller mandated alignment: disables debug if necessary */
 	if (ralign < align) {
 		ralign = align;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Pekka J Enberg]

On Wed, 26 Jul 2006, Heiko Carstens wrote:
> Since ARCH_KMALLOC_MINALIGN didn't work on s390 I tried ARCH_SLAB_MINALIGN
> instead, just to find out that it didn't work too.
> In case of CONFIG_DEBUG_SLAB kmem_cache_create() creates caches with an
> alignment lesser than ARCH_SLAB_MINALIGN, which it shouldn't according to
> this comment in mm/slab.c :

[snip]

> Index: linux-2.6/mm/slab.c
> ===================================================================
> --- linux-2.6.orig/mm/slab.c	2006-07-26 09:55:54.000000000 +0200
> +++ linux-2.6/mm/slab.c	2006-07-26 09:57:07.000000000 +0200
> @@ -2103,6 +2103,9 @@
>  		if (ralign > BYTES_PER_WORD)
>  			flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
>  	}
> +	if (BYTES_PER_WORD < ARCH_SLAB_MINALIGN)
> +		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
> +
>  	/* 3) caller mandated alignment: disables debug if necessary */
>  	if (ralign < align) {
>  		ralign = align;

This is similar to my patch and should be enough to fix the problem. The 
first patch seems bogus and I don't really understand why you would need 
it.

					Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Heiko Carstens]

On Wed, Jul 26, 2006 at 01:05:43PM +0300, Pekka J Enberg wrote:
> On Wed, 26 Jul 2006, Heiko Carstens wrote:
> > Since ARCH_KMALLOC_MINALIGN didn't work on s390 I tried ARCH_SLAB_MINALIGN
> > instead, just to find out that it didn't work too.
> > In case of CONFIG_DEBUG_SLAB kmem_cache_create() creates caches with an
> > alignment lesser than ARCH_SLAB_MINALIGN, which it shouldn't according to
> > this comment in mm/slab.c :
> 
> [snip]
> 
> > Index: linux-2.6/mm/slab.c
> > ===================================================================
> > --- linux-2.6.orig/mm/slab.c	2006-07-26 09:55:54.000000000 +0200
> > +++ linux-2.6/mm/slab.c	2006-07-26 09:57:07.000000000 +0200
> > @@ -2103,6 +2103,9 @@
> >  		if (ralign > BYTES_PER_WORD)
> >  			flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
> >  	}
> > +	if (BYTES_PER_WORD < ARCH_SLAB_MINALIGN)
> > +		flags &= ~(SLAB_RED_ZONE | SLAB_STORE_USER);
> > +
> >  	/* 3) caller mandated alignment: disables debug if necessary */
> >  	if (ralign < align) {
> >  		ralign = align;
> 
> This is similar to my patch and should be enough to fix the problem. The 
> first patch seems bogus and I don't really understand why you would need 
> it.

It's enough to fix the ARCH_SLAB_MINALIGN problem. But it does _not_ fix the
ARCH_KMALLOC_MINALIGN problem. s390 currently only uses ARCH_KMALLOC_MINALIGN
since that should be good enough and it doesn't disable as much debugging
as ARCH_SLAB_MINALIGN does.
What exactly isn't clear from the description of the first patch? Or why do
you consider it bogus?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Pekka J Enberg]

On Wed, 26 Jul 2006, Heiko Carstens wrote:
> It's enough to fix the ARCH_SLAB_MINALIGN problem. But it does _not_ fix the
> ARCH_KMALLOC_MINALIGN problem. s390 currently only uses ARCH_KMALLOC_MINALIGN
> since that should be good enough and it doesn't disable as much debugging
> as ARCH_SLAB_MINALIGN does.
> What exactly isn't clear from the description of the first patch? Or why do
> you consider it bogus?

Now I am confused. What do you mean by "doesn't disable as much debugging 
as ARCH_SLAB_MINALIGN does"? AFAICT, the SLAB_RED_ZONE and SLAB_STORE_USER 
options _require_ BYTES_PER_WORD alignment, so if s390 requires 8 
byte alignment, you can't have them debugging anyhow...

				Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Heiko Carstens]

On Wed, Jul 26, 2006 at 01:37:42PM +0300, Pekka J Enberg wrote:
> On Wed, 26 Jul 2006, Heiko Carstens wrote:
> > It's enough to fix the ARCH_SLAB_MINALIGN problem. But it does _not_ fix the
> > ARCH_KMALLOC_MINALIGN problem. s390 currently only uses ARCH_KMALLOC_MINALIGN
> > since that should be good enough and it doesn't disable as much debugging
> > as ARCH_SLAB_MINALIGN does.
> > What exactly isn't clear from the description of the first patch? Or why do
> > you consider it bogus?
> 
> Now I am confused. What do you mean by "doesn't disable as much debugging 
> as ARCH_SLAB_MINALIGN does"? AFAICT, the SLAB_RED_ZONE and SLAB_STORE_USER 
> options _require_ BYTES_PER_WORD alignment, so if s390 requires 8 
> byte alignment, you can't have them debugging anyhow...

We only specify ARCH_KMALLOC_MINALIGN, since that aligns only the kmalloc
caches, but it doesn't disable debugging on other caches that are created
via kmem_cache_create() where an alignment of e.g. 0 is specified.

The point of the first patch is: why should the slab cache be allowed to chose
an aligment that is less than what the caller specified? This does very likely
break things.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Pekka J Enberg]

On Wed, 26 Jul 2006, Heiko Carstens wrote:
> We only specify ARCH_KMALLOC_MINALIGN, since that aligns only the kmalloc
> caches, but it doesn't disable debugging on other caches that are created
> via kmem_cache_create() where an alignment of e.g. 0 is specified.
> 
> The point of the first patch is: why should the slab cache be allowed to chose
> an aligment that is less than what the caller specified? This does very likely
> break things.

Ah, yes, you are absolutely right. We need to respect caller mandated 
alignment too. How about this?

			Pekka

[PATCH] slab: respect architecture and caller mandated alignment

Ensure cache alignment is always at minimum what the architecture or 
caller mandates even if slab debugging is enabled.

Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---

diff --git a/mm/slab.c b/mm/slab.c
index 0f20843..3767460 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2097,6 +2097,15 @@ #endif
 	} 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;
@@ -2123,20 +2132,19 @@ #endif
 #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)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Heiko Carstens]

On Wed, Jul 26, 2006 at 02:16:06PM +0300, Pekka J Enberg wrote:
> On Wed, 26 Jul 2006, Heiko Carstens wrote:
> > We only specify ARCH_KMALLOC_MINALIGN, since that aligns only the kmalloc
> > caches, but it doesn't disable debugging on other caches that are created
> > via kmem_cache_create() where an alignment of e.g. 0 is specified.
> > 
> > The point of the first patch is: why should the slab cache be allowed to chose
> > an aligment that is less than what the caller specified? This does very likely
> > break things.
> 
> Ah, yes, you are absolutely right. We need to respect caller mandated 
> alignment too. How about this?

Works fine and looks much better than my two patches. Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Manfred Spraul]

Pekka J Enberg wrote:

>On Wed, 26 Jul 2006, Heiko Carstens wrote:
>  
>
>>We only specify ARCH_KMALLOC_MINALIGN, since that aligns only the kmalloc
>>caches, but it doesn't disable debugging on other caches that are created
>>via kmem_cache_create() where an alignment of e.g. 0 is specified.
>>
>>The point of the first patch is: why should the slab cache be allowed to chose
>>an aligment that is less than what the caller specified? This does very likely
>>break things.
>>    
>>
>
>Ah, yes, you are absolutely right. We need to respect caller mandated 
>alignment too. How about this?
>
>  
>
Good catch - I obviously never tested the code for an HWCACHE_ALIGN cache...


>			Pekka
>
>[PATCH] slab: respect architecture and caller mandated alignment
>
>Ensure cache alignment is always at minimum what the architecture or 
>caller mandates even if slab debugging is enabled.
>
>Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
>  
>
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>

--
    Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Christoph Lameter]

On Wed, 26 Jul 2006, Manfred Spraul wrote:

> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>

Good bye to all those cacheline contentions that helped us find so many 
race conditions in the past if we switched on SLAB_DEBUG. I thought this 
was intentional?


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Manfred Spraul]

Christoph Lameter wrote:

>On Wed, 26 Jul 2006, Manfred Spraul wrote:
>
>  
>
>>Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
>>    
>>
>
>Good bye to all those cacheline contentions that helped us find so many 
>race conditions in the past if we switched on SLAB_DEBUG. I thought this 
>was intentional?
>
>  
>
Relax, align is nearly never set.

- kmalloc uses align==0, except if the architecture requests it 
(ARCH_KMALLOC_MINALIGN not 0)
- on my i386 system, the following users explicitely use align:
* the pmd structure (4096: hardware requirement)
* the pgd structure (32 bytes: hardware requirement)
* the task structure (16 byte. fxsave)
* sigqueue, pid: both request 4 byte alignment (based on __alignof__()). 
Doesn't affect debugging.

 From the other mail:

>Thus the patch is correct, it's a bug in the slab allocator. If HWCACHE_ALIGN
>> is set, then the allocator ignores align or ARCH_SLAB_MINALIGN.
>  
>
>
>But then Heiko does not want to set ARCH_SLAB_MINALIGN at all. This is not 
>the issue we are discussing. In the DEBUG case he wants 
>ARCH_KMALLOC_MINALIGN to be enforced even if ARCH_SLAB_MINALIGN is not 
>set.
>  
>
The kmalloc caches are allocated with 
HWCACHE_ALIGN+ARCH_KMALLOC_MINALIGN. The logic in kmem_cache_create 
didn't handle that case correctly.
On most architectures, ARCH_KMALLOC_MINALIGN is 0. Thus SLAB_DEBUG 
redzones everything.
On s390, ARCH_KMALLOC_MINALIGN is 8. This disables redzoning.

Ok?

--
    Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Christoph Lameter]

On Wed, 26 Jul 2006, Manfred Spraul wrote:

> > Thus the patch is correct, it's a bug in the slab allocator. If
> > HWCACHE_ALIGN
> > > is set, then the allocator ignores align or ARCH_SLAB_MINALIGN.
> >  
> > 
> > But then Heiko does not want to set ARCH_SLAB_MINALIGN at all. This is not
> > the issue we are discussing. In the DEBUG case he wants
> > ARCH_KMALLOC_MINALIGN to be enforced even if ARCH_SLAB_MINALIGN is not set.
> >  
> The kmalloc caches are allocated with HWCACHE_ALIGN+ARCH_KMALLOC_MINALIGN. The
> logic in kmem_cache_create didn't handle that case correctly.
> On most architectures, ARCH_KMALLOC_MINALIGN is 0. Thus SLAB_DEBUG redzones
> everything.
> On s390, ARCH_KMALLOC_MINALIGN is 8. This disables redzoning.
> 
> Ok?

So Redzoning etc will now be diabled regardless even if  
ARCH_SLAB_MINALIGN is not set but another alignment is given to 
kmem_cache_alloc?

So we sacrifice the ability to worsen the performance of slabs by 
misaligning them for debugging purposes.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Manfred Spraul]

Christoph Lameter wrote:

>So Redzoning etc will now be diabled regardless even if  
>ARCH_SLAB_MINALIGN is not set but another alignment is given to 
>kmem_cache_alloc?
>  
>
kmem_cache_create().

>So we sacrifice the ability to worsen the performance of slabs by 
>misaligning them for debugging purposes.
>  
>
If a slab user can live with misaligned objects, then he shouldn't set 
align. Thus we do not sacrifice anything.

--
    Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Christoph Lameter]

On Wed, 26 Jul 2006, Manfred Spraul wrote:

> > So we sacrifice the ability to worsen the performance of slabs by
> > misaligning them for debugging purposes.
> >  
> If a slab user can live with misaligned objects, then he shouldn't set align.
> Thus we do not sacrifice anything.

A slab user is setting alignment in order to optimize performance not for 
correctness. Most users that I know of can live with misalignments. If 
that would not be the case then this code would never have worked.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Manfred Spraul]

Christoph Lameter wrote:

>A slab user is setting alignment in order to optimize performance not for 
>correctness. Most users that I know of can live with misalignments. If 
>that would not be the case then this code would never have worked.
>  
>

Which users do you know that set align and that can live with misalignments?
As I wrote, there are no such users in my (i386) kernel.

--
    Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Christoph Lameter]

On Wed, 26 Jul 2006, Manfred Spraul wrote:

> Christoph Lameter wrote:
> 
> > A slab user is setting alignment in order to optimize performance not for
> > correctness. Most users that I know of can live with misalignments. If that
> > would not be the case then this code would never have worked.
> >  
> 
> Which users do you know that set align and that can live with misalignments?
> As I wrote, there are no such users in my (i386) kernel.

The users of SLAB_HWCACHE_ALIGN can live with that.

Systems running with slab debugging on must be very buggy at this point or 
we were very lucky:

The list is a bit strange:

>* the pmd structure (4096: hardware requirement)

It is already exempted from debug since the size is 4096.

>* the pgd structure (32 bytes: hardware requirement)

We were lucky on that one in the past? This should break.

>* the task structure (16 byte. fxsave)

Would only break if floating point is used I think.

>* sigqueue, pid: both request 4 byte alignment (based on __alignof__()). 
>Doesn't affect debugging.
So also not relevant.


We now want to say that SLAB_HWCACHE_ALIGN is only a suggestion to be 
disposed of if debug is on whereas an explicitly specified alignment must be enforced?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Pekka J Enberg]

On Wed, 26 Jul 2006, Christoph Lameter wrote:
> A slab user is setting alignment in order to optimize performance not for 
> correctness. Most users that I know of can live with misalignments. If 
> that would not be the case then this code would never have worked.

No, caller and architecture mandated alignment is set for correctness. The 
slab allocator can already optimize for performance on its own 
(SLAB_HWCACHE_ALIGN).

				Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Pekka J Enberg]

On Wed, 26 Jul 2006, Christoph Lameter wrote:
> We now want to say that SLAB_HWCACHE_ALIGN is only a suggestion to be 
> disposed of if debug is on whereas an explicitly specified alignment must be enforced?

Yes and that's what we have been saying all along. When you want 
performance, you use SLAB_HWCACHE_ALIGN and let the allocator do its job. 
I don't see much point from API point of view for the caller to explicitly 
ask for a given alignment and then in addition pass a 'yes I really meant' 
flag (SLAB_DEBUG_OVERRIDE).

So Christoph, unless you can point out a specific problem with my patch, 
I'll queue it up to Andrew. Thanks.

				Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] slab: always consider arch mandated alignment

[Pekka J Enberg]

On Thu, 27 Jul 2006, Pekka J Enberg wrote:
> Yes and that's what we have been saying all along. When you want 
> performance, you use SLAB_HWCACHE_ALIGN and let the allocator do its job. 
> I don't see much point from API point of view for the caller to explicitly 
> ask for a given alignment and then in addition pass a 'yes I really meant' 
> flag (SLAB_DEBUG_OVERRIDE).

Btw, /proc/slabinfo for UML with defconfig reveals change for only one 
cache with my patch applied. The 'dquot' cache is created by dquot_init in 
fs/dquot.c and doesn't really seem to need the alignment for anything...

				Pekka

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>