Prev: generic rwsem: implement down_read_critical() / up_read_critical()
Next: kvm: network problem with Solaris 10u8 guest
From: Johannes Stezenbach on 19 May 2010 09:40
Hi,

I have some comments/questions, I hope it's not too silly:

On Wed, May 19, 2010 at 12:01:42PM +0100, David Woodhouse wrote:
> +#ifndef ARCH_KMALLOC_MINALIGN
> +/*
> + * 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 the alignment of a 64-bit integer.
> + * ARCH_KMALLOC_MINALIGN allows that.
> + * Note that increasing this value may disable some debug features.
> + */
> +#define ARCH_KMALLOC_MINALIGN __alignof__(unsigned long long)
> +#endif

I think the comment is confusing. IIRC kmalloc() API guarantees that
the allocated buffer is suitable for DMA, so if cache coherence is not
handled by hardware the arch might need to set this to the cache line size,
and that's what ARCH_KMALLOC_MINALIGN is about. Nothing else.

I'm not sure if kmalloc() API makes any guarantee regarding alignment,
like 32bit or 64bit or sizeof(void *) ...? At least I cannot find
any documentation about this.


> +#ifndef ARCH_SLAB_MINALIGN
> +/*
> + * 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.
> + */
> +#define ARCH_SLAB_MINALIGN 0
> +#endif

Why is this needed at all? If code calls kmem_cache_create()
with wrong align parameter, or has wrong expectations wrt kmalloc()
alignment guarantees, this code needs to be fixed?
I mean, portable code cannot assume that unaligned accesses work?


Thanks
Johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Woodhouse on 19 May 2010 14:20
On Wed, 2010-05-19 at 20:03 +0200, Manfred Spraul wrote:
> Is this text better?

Not really...

> * Unless debug options are enabled, the kernel uses cache_line_size()
> automatically.

.... that's a lie.

--
David Woodhouse Open Source Technology Centre
David.Woodhouse(a)intel.com Intel Corporation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Woodhouse on 19 May 2010 15:30
On Wed, 2010-05-19 at 14:19 -0500, Christoph Lameter wrote:
>
> Maybe we can consolidate that into slab.h so that the alignment is the
> same for all allocators?

Doing so at least for slub and slab would make sense. I wanted to move
them first though, as a separate step.

--
David Woodhouse Open Source Technology Centre
David.Woodhouse(a)intel.com Intel Corporation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: generic rwsem: implement down_read_critical() / up_read_critical()
Next: kvm: network problem with Solaris 10u8 guest