Leon Romanovsky <[email protected]> writes: > On Sat, Jul 25, 2026 at 11:34:11AM -0300, Jason Gunthorpe wrote: >> On Thu, Jul 23, 2026 at 10:57:04AM +0300, Leon Romanovsky wrote: >> > On Wed, Jul 22, 2026 at 04:59:12PM -0300, Jason Gunthorpe wrote: >> > > On Tue, Jul 21, 2026 at 06:33:21PM +0300, Leon Romanovsky wrote: >> > > >> > > > 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. > >> >> > 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
