lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <720df841-8300-490a-af77-8d20f833c042@kernel.org>
Date: Thu, 3 Jul 2025 15:27:13 +0200
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Byungchul Park <byungchul@...com>, willy@...radead.org,
 netdev@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 kernel_team@...ynix.com, kuba@...nel.org, almasrymina@...gle.com,
 ilias.apalodimas@...aro.org, harry.yoo@...cle.com,
 akpm@...ux-foundation.org, davem@...emloft.net, john.fastabend@...il.com,
 andrew+netdev@...n.ch, asml.silence@...il.com, toke@...hat.com,
 tariqt@...dia.com, edumazet@...gle.com, pabeni@...hat.com,
 saeedm@...dia.com, leon@...nel.org, ast@...nel.org, daniel@...earbox.net,
 david@...hat.com, lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com,
 vbabka@...e.cz, rppt@...nel.org, surenb@...gle.com, mhocko@...e.com,
 horms@...nel.org, linux-rdma@...r.kernel.org, bpf@...r.kernel.org,
 vishal.moola@...il.com, hannes@...xchg.org, ziy@...dia.com,
 jackmanb@...gle.com
Subject: Re: [PATCH net-next v8 5/5] page_pool: make page_pool_get_dma_addr()
 just wrap page_pool_get_dma_addr_netmem()



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@...com>
> Reviewed-by: Mina Almasry <almasrymina@...gle.com>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@...aro.org>
> Reviewed-by: Toke Høiland-Jørgensen <toke@...hat.com>
> Reviewed-by: Pavel Begunkov <asml.silence@...il.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@...nel.org>

--Jesper

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ