ad7877: keep dma rx buffers in seperate cache lines
in [Kernel]
|
From: Michal Simek on 17 May 2010 02:20 Mike Frysinger wrote: > On Tue, May 11, 2010 at 17:53, Dmitry Torokhov wrote: >> On Tue, May 11, 2010 at 11:48:36PM +0200, Johannes Weiner wrote: >>> On Tue, May 11, 2010 at 04:54:41PM -0400, Mike Frysinger wrote: >>>> does the phrase "DMA safe buffer" imply cache alignment ? >>> I guess that depends on the architectural requirements. On x86, >>> apparently, not so much. On ARM, probably yes, as it's the >>> requirement to properly maintain coherency. >> It looks liek ARM (and a few others) do this: >> >> [dtor(a)hammer work]$ grep -r ARCH_KMALLOC_MINALIGN arch/ >> arch/sh/include/asm/page.h:#define ARCH_KMALLOC_MINALIGN L1_CACHE_BYTES >> arch/frv/include/asm/mem-layout.h:#define ARCH_KMALLOC_MINALIGN 8 >> arch/powerpc/include/asm/page_32.h:#define ARCH_KMALLOC_MINALIGN L1_CACHE_BYTES >> arch/arm/include/asm/cache.h:#define ARCH_KMALLOC_MINALIGN L1_CACHE_BYTES >> arch/microblaze/include/asm/page.h:#define ARCH_KMALLOC_MINALIGN L1_CACHE_BYTES >> arch/mips/include/asm/mach-tx49xx/kmalloc.h: * All happy, no need to define ARCH_KMALLOC_MINALIGN >> arch/mips/include/asm/mach-ip27/kmalloc.h: * All happy, no need to define ARCH_KMALLOC_MINALIGN >> arch/mips/include/asm/mach-ip32/kmalloc.h:#define ARCH_KMALLOC_MINALIGN 32 >> arch/mips/include/asm/mach-ip32/kmalloc.h:#define ARCH_KMALLOC_MINALIGN 128 >> arch/mips/include/asm/mach-generic/kmalloc.h:#define ARCH_KMALLOC_MINALIGN 128 >> arch/avr32/include/asm/cache.h:#define ARCH_KMALLOC_MINALIGN L1_CACHE_BYTES > > if ARCH_KMALLOC_MINALIGN is not defined, the current allocators default to: > slub - alignof(unsigned long long) > slab - alignof(unsigned long long) > slob - alignof(unsigned long) > which for many arches can mean an alignment of merely 4 or 8 > > lets look at the cacheline sizes for arches that dont set > ARCH_KMALLOC_MINALIGN to L1_CACHE_BYTES: > - alplha - 32 or 64 > - frv - 32 or 64 > - blackfin - 32 > - parisc - 32 or 64 > - mn10300 - 16 > - s390 - 256 > - score - 16 > - sparc - 32 > - xtensa - 16 or 32 Just for completion. microblaze - 16 or 32. Michal -- Michal Simek, Ing. (M.Eng) PetaLogix - Linux Solutions for a Reconfigurable World w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663 -- 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 09:10
On Tue, 2010-05-11 at 16:54 -0400, Mike Frysinger wrote: > > > kmalloc returns a pointer to a DMA safe buffer. There is no requirement on > > the x86 hardware that the DMA buffers have to be cache aligned. Cachelines > > will be invalidated as needed. > > so this guarantee is made by the kmalloc() API ? and for arches where > the cacheline invalidation is handled in software rather than > hardware, they must declare a min alignment value for kmalloc to be at > least as big as their cache alignment ? > > does the phrase "DMA safe buffer" imply cache alignment ? Not necessarily. If you have cache-coherent DMA, then there's no need to align things. You only need cache-alignment when you have cache-incoherent DMA and have to manage the cache in software. And in that case, the architecture must set ARCH_KMALLOC_MINALIGN to an appropriate value so that kmalloc() meets the guarantee. However, your problem is kind of unrelated to that -- your problem is actually that you're putting both a DMA buffer _and_ other stuff into the same kmalloc'd buffer. Don't do that. Instead, allocate your DMA buffer separately and put a _pointer_ to it into the structure. Or if you really _must_ include it in your structure, then align to ARCH_KMALLOC_MINALIGN bytes both before and after the DMA buffer by adding __attribute__((__aligned__(ARCH_KMALLOC_MINALIGN))) to both the DMA buffer _and_ the element which follows it in your struct (if any). Using ____cacheline_aligned wastes a lot of space on the architectures that don't need it. Arguably, this __dma_aligned definition should go somewhere generic... diff --git a/drivers/input/touchscreen/ad7877.c b/drivers/input/touchscreen/ad7877.c index 0d2d7e5..ae5d56e 100644 --- a/drivers/input/touchscreen/ad7877.c +++ b/drivers/input/touchscreen/ad7877.c @@ -151,6 +151,11 @@ enum { /* * Non-touchscreen sensors only use single-ended conversions. */ +#ifdef ARCH_KMALLOC_MINALIGN +#define __dma_aligned __attribute__((__aligned__(ARCH_KMALLOC_MINALIGN))) +#else +#define __dma_aligned +#endif struct ser_req { u16 reset; @@ -163,7 +168,7 @@ struct ser_req { * DMA (thus cache coherency maintenance) requires the * transfer buffers to live in their own cache lines. */ - u16 sample ____cacheline_aligned; + u16 sample __dma_aligned; }; struct ad7877 { @@ -203,7 +208,7 @@ struct ad7877 { * DMA (thus cache coherency maintenance) requires the * transfer buffers to live in their own cache lines. */ - u16 conversion_data[AD7877_NR_SENSE] ____cacheline_aligned; + u16 conversion_data[AD7877_NR_SENSE] __dma_aligned; }; static int gpio3; -- 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/ |