On Tue, Jul 14, 2026 at 09:27:01AM +0530, Aneesh Kumar K.V wrote: > Jason Gunthorpe <[email protected]> writes: > > > On Wed, Jul 01, 2026 at 11:19:08AM +0530, Aneesh Kumar K.V (Arm) wrote: > >> dma_direct_alloc_pages() may satisfy atomic allocations from the coherent > >> atomic pools. The pool allocation is keyed by the virtual address stored in > >> the gen_pool, but the pages API returns only the backing struct page. > >> > >> On architectures with CONFIG_DMA_DIRECT_REMAP, atomic pool chunks are added > >> to the gen_pool using their remapped virtual address. > >> dma_direct_free_pages() reconstructs a linear-map address with > >> page_address(page) and passes that to dma_free_from_pool(). That address > >> does not match the gen_pool virtual range, so the pool lookup can fail and > >> the code can fall through to freeing a pool-owned page through the normal > >> page allocator path. > >> > >> Add a page-based pool free helper that looks up the owning pool chunk by > >> physical address, translates it back to the gen_pool virtual address, and > >> frees that address to the pool. Use it from dma_direct_free_pages() while > >> keeping the existing virtual-address helper for coherent allocation frees. > >> > >> Tested-by: Michael Kelley <[email protected]> > >> Tested-by: Mostafa Saleh <[email protected]> > >> Signed-off-by: Aneesh Kumar K.V (Arm) <[email protected]> > >> --- > >> include/linux/dma-map-ops.h | 1 + > >> kernel/dma/direct.c | 4 +-- > >> kernel/dma/pool.c | 54 +++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 57 insertions(+), 2 deletions(-) > > > > This seems pretty suboptimal? > > > > If !CONFIG_DMA_DIRECT_REMAP then page_to_virt() was used to compute > > the genpool's addr so dma_free_from_pool_page() can use the same > > logic, which is how things must be working at all today > > > > The CONFIG_DMA_DIRECT_REMAP scenario does look broken, so I'm > > surprised there isn't a Fixes line on this commit? I don't have an > > opinion on the search, but since alloc_pages() is used there is 8 > > bytes in the struct page that could be used to store the remapped > > vaddr to avoid the search if someday someone wants to improve > > this. Maybe a small comment hinting that direction would be a nice > > addition. > > Something like > > +/* > + * FIXME!! We could avoid this by storing the remapped virtual address in > + * struct page and using that for lookup. > + */ > bool dma_free_from_pool_page(struct device *dev, struct page *page, size_t > size)
Plus some if (!IS_ENABLED()) that does the direct lookup I would just use : not !! :) Jason
