Re: [LKML] Re: [PATCH v3] ad7877: keep dma rx buffers in seperate cache lines

[Nick Piggin <npiggin@suse.de>]

On Wed, May 19, 2010 at 02:17:47PM +0100, David Woodhouse wrote:
> On Wed, 2010-05-19 at 23:07 +1000, Nick Piggin wrote:
> > On Wed, May 19, 2010 at 01:48:45PM +0100, David Woodhouse wrote:
> > > However, we _do_ allow drivers to assume that kmalloc is DMA-safe. That
> > > happens to mean "cacheline-aligned" for cache-incoherent architectures,
> > > but drivers should never really have to think about that.
> > 
> > DMA-safe for GFP_DMA, or all kmalloc?
> 
> Try not to think about that. Seriously, look over there! A kitten!
> 
> (You have to use the DMA API anyway, so swiotlb will handle things if
> you're trying to use pages which are above the range that certain
> devices can reach. But we're only talking about the problems of sharing
> cache lines here; don't worry about that bit.)

Yeah, well, GFP_DMA is pretty crappy anyway. I was just curious, but
it's no big deal to support dma with regular kmalloc.

 
> > > > So whenever strengthening API guarantees like this, it is better to be
> > > > very careful and conservative. Probably even introducing a new API with
> > > > the stronger semantics (even if it is just a wrapper in the case where
> > > > KMALLOC_MINALIGNED *is* cacheline sized).
> > > 
> > > We're not talking about strengthening API guarantees. It's _always_ been
> > > this way; it's just that some architectures are buggy.
> > 
> > It just appeared, in the post I replied to, that there was a suggestion
> > of making it explicitly cacheline aligned. If I misread that, ignore
> > me.
> 
> Actually I think you read it just fine -- but it was misguided. They
> meant to say "cacheline aligned on architectures with cache-incoherent
> DMA which need that". But that's what ARCH_KMALLOC_MINALIGN is _already_
> supposed to do. If any architecture needs to set ARCH_KMALLOC_MINALIGN
> but doesn't, that's broken.
> 
> The 'cacheline aligned' misconception did manage to get into the ad7877
> driver in commit 3843384a though -- it now uses ____cacheline_aligned
> instead of __attribute__((__aligned__(ARCH_KMALLOC_MINALIGN))) as it
> should.

OK so long as there is not a "must be cacheline aligned" requirement.
Your proposal for a __dma_aligned attribute in an arch header looks
like a good idea there.