Re: [PATCH] ARCH_SLAB_MINALIGN for 2.6.10-rc3

[Manfred Spraul <manfred@colorfullife.com>]

Paul Mundt wrote:

>Hi Manfred,
>
>On Sun, Dec 05, 2004 at 10:30:46PM +0100, Manfred Spraul wrote:
>  
>
>>>--- orig/include/asm-sh64/uaccess.h
>>>+++ mod/include/asm-sh64/uaccess.h
>>>@@ -313,6 +313,12 @@
>>>  sh64 at the moment). */
>>>#define ARCH_KMALLOC_MINALIGN 8
>>>
>>>+/*
>>>+ * We want 8-byte alignment for the slab caches as well, otherwise we have
>>>+ * the same BYTES_PER_WORD (sizeof(void *)) min align in 
>>>kmem_cache_create().
>>>+ */
>>>+#define ARCH_SLAB_MINALIGN 8
>>>+
>>>
>>>
>>>      
>>>
>>Could you make that dependant on !CONFIG_DEBUG_SLAB? Setting align to a 
>>non-zero value disables some debug code.
>>
>>    
>>
>align is only being set to ARCH_SLAB_MINALIGN in kmem_cache_create()
>where it is otherwise being set to BYTES_PER_WORD as a default. Unless I
>am missing something, that will always set it non-zero irregardless of
>whether ARCH_SLAB_MINALIGN is set.
>
>  
>
No, you are right. I didn't read the source carefully enough.
Now that I have reread it, I see one problem:
ARCH_KMALLOC_MINALIGN is a hard limit: It's always honored, the only 
exception is that values larger than the kmalloc block size are ignored. 
I.e. _MINALIGN 32 guarantees that the objects are 32-byte aligned (since 
the smallest block size is 32-bytes). The define was added, because some 
archs really need a certain alignment, otherwise they won't boot. The 
normal alignment for kmalloc caches is cache line alignment, except with 
CONFIG_DEBUG_SLAB, then it's word alignment.

With your patch, ARCH_SLAB_MINALIGN is not a hard limit: A few lines 
further down align is reset to word size if SLAB_RED_ZONE is set. I 
don't like the asymmetry - it just asks for trouble.

I must think about it. Perhaps just rename ARCH_SLAB_MINALIGN to 
ARCH_SLAB_DEFAULTALIGN.

--
    Manfred