On Wed, 15 Jul 2015 14:14:40 -0700 "Sean O. Stalley" <sean.stalley@xxxxxxxxx> wrote: > Currently the __GFP_ZERO flag is ignored by dma_pool_alloc(). > Make dma_pool_alloc() zero the memory if this flag is set. > > ... > > --- a/mm/dmapool.c > +++ b/mm/dmapool.c > @@ -334,7 +334,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags, > /* pool_alloc_page() might sleep, so temporarily drop &pool->lock */ > spin_unlock_irqrestore(&pool->lock, flags); > > - page = pool_alloc_page(pool, mem_flags); > + page = pool_alloc_page(pool, mem_flags & (~__GFP_ZERO)); > if (!page) > return NULL; > > @@ -375,6 +375,10 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags, > memset(retval, POOL_POISON_ALLOCATED, pool->size); > #endif > spin_unlock_irqrestore(&pool->lock, flags); > + > + if (mem_flags & __GFP_ZERO) > + memset(retval, 0, pool->size); > + > return retval; > } > EXPORT_SYMBOL(dma_pool_alloc); hm, this code is all a bit confused. We'd really prefer that the __GFP_ZERO be passed all the way to the bottom level, so that places which are responsible for zeroing memory (eg, the page allocator) can do their designated function. One reason for this is that if someone comes up with a whizzy way of zeroing memory on their architecture (eg, non-temporal store) then that will be implemented in the core page allocator and the dma code will miss out. Also, and just from a brief look around, drivers/base/dma-coherent.c:dma_alloc_from_coherent() is already zeroing the memory so under some circumstances I think we'll zero the memory twice? We could fix that by passing the gfp_t to dma_alloc_from_coherent() and then changing dma_alloc_from_coherent() to *not* zero the memory if __GFP_ZERO, but wouldn't that be peculiar? Also, passing __GFP_ZERO will now cause pool_alloc_page()'s memset(POOL_POISON_FREED) to be wiped out. I guess that's harmless, but a bit inefficient? -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
