Re: [PATCH net-next v8 5/5] page_pool: make page_pool_get_dma_addr() just wrap page_pool_get_dma_addr_netmem()

[Jesper Dangaard Brouer <hawk@kernel.org>]

On 02/07/2025 07.32, Byungchul Park wrote:
> The page pool members in struct page cannot be removed unless it's not
> allowed to access any of them via struct page.
> 
> Do not access 'page->dma_addr' directly in page_pool_get_dma_addr() but
> just wrap page_pool_get_dma_addr_netmem() safely.
> 
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> Reviewed-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   include/net/page_pool/helpers.h | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
> index 773fc65780b5..db180626be06 100644
> --- a/include/net/page_pool/helpers.h
> +++ b/include/net/page_pool/helpers.h
> @@ -444,12 +444,7 @@ static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
>    */
>   static inline dma_addr_t page_pool_get_dma_addr(const struct page *page)
>   {
> -	dma_addr_t ret = page->dma_addr;
> -
> -	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
> -		ret <<= PAGE_SHIFT;
> -
> -	return ret;
> +	return page_pool_get_dma_addr_netmem(page_to_netmem(page));

Wow - the amount of type casting shenanigans going on here make the code
hard to follow.

This code changes adds an extra "AND" operation, but we don't have a
micro-benchmark that tests the performance of a DMA enabled page_pool,
so I cannot tell if this add any overhead.  My experience tells me that
this extra AND-operation will not be measurable.

I see a lot of reviewed-by from people I trust, so you also get my 
page_pool maintainer ack.

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

--Jesper