Re: [PATCH v8 01/23] dma-direct: return struct page from dma_direct_alloc_from_pool()

Leon Romanovsky [off-list ref] writes:

On Sat, Jul 25, 2026 at 11:34:11AM -0300, Jason Gunthorpe wrote:

quoted

On Thu, Jul 23, 2026 at 10:57:04AM +0300, Leon Romanovsky wrote:

quoted

On Wed, Jul 22, 2026 at 04:59:12PM -0300, Jason Gunthorpe wrote:

quoted

On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote:

quoted

Something like, but probably void* needs to be phys_addr_t:

static inline struct page *dma_phys_to_page(void *addr)
{
#ifdef CONFIG_DMA_DIRECT_REMAP
	return vmalloc_to_page(addr);
#else
	return virt_to_page(addr);
#endif
}

I don't like this, we don't know for sure the addr will be in a vmap
and this will fail if it isn't.

Of course we know. The existing "#ifdef CONFIG_DMA_DIRECT_REMAP" is
relevant for addresses acquired from pool.

Yeah, but I still don't like it :) It is hard to follow if you make
those kinds of leaps, someone will call this new helper on something
they shouldn't

This issue is straightforward to address today. Limit the scope, use
an appropriate function name, and add a comment indicating that the
function is local to and specific to the pool. The latter helps
AI-based review tools flag incorrect usage outside the intended scope.

quoted

quoted

dma_phys_to_page() is a bad name for some low-level conversion function.
It needs to be internal to DMA logic, in the level when we convert from
phys to page.

I think we should not convert from phys to page, that's also easy to
do wrong

Perhaps I'll reiterate my complaint. DMA internals are already hard to
understand. Part of that complexity is necessary to support every
possible use case, but another part comes from a maze of types that is
entirely self-inflicted. I think this code only makes the latter worse.

Currently, the phys type universally describes memory and can be
reliably translated into any required representation. It is the most
fundamental type we have.

I think we can address the suggested changes as a follow-up patch series
on top of this.

How about we get this picked up for the next merge window first, so that
we can get the CC_SHARED related changes in? The follow-up changes
should be easier once we have agreement on the overall design, and can
then be done as incremental cleanups.

-aneesh