Re: [PATCH] ARCH_SLAB_MINALIGN for 2.6.10-rc3

[Manfred Spraul]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/