netdev - Re: [PATCH v5 RFC 1/6] page_pool: frag API support for 32-bit arch with 64-bit DMA

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]

Date: Mon, 10 Jul 2023 11:36:54 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Yunsheng Lin <yunshenglin0825@...il.com>
Cc: Yunsheng Lin <linyunsheng@...wei.com>, davem@...emloft.net,
 pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Lorenzo Bianconi <lorenzo@...nel.org>, Alexander Duyck
 <alexander.duyck@...il.com>, Liang Chen <liangchen.linux@...il.com>,
 Alexander Lobakin <aleksander.lobakin@...el.com>, Saeed Mahameed
 <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>, Eric Dumazet
 <edumazet@...gle.com>, Jesper Dangaard Brouer <hawk@...nel.org>, Ilias
 Apalodimas <ilias.apalodimas@...aro.org>, linux-rdma@...r.kernel.org
Subject: Re: [PATCH v5 RFC 1/6] page_pool: frag API support for 32-bit arch
 with 64-bit DMA

On Sun, 9 Jul 2023 20:39:45 +0800 Yunsheng Lin wrote:
> On 2023/7/8 7:59, Jakub Kicinski wrote:
> > On Thu, 29 Jun 2023 20:02:21 +0800 Yunsheng Lin wrote:  
> >> +		/* Return error here to avoid mlx5e_page_release_fragmented()
> >> +		 * calling page_pool_defrag_page() to write to pp_frag_count
> >> +		 * which is overlapped with dma_addr_upper in 'struct page' for
> >> +		 * arch with PAGE_POOL_DMA_USE_PP_FRAG_COUNT being true.
> >> +		 */
> >> +		if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT) {
> >> +			err = -EINVAL;
> >> +			goto err_free_by_rq_type;
> >> +		}  
> > 
> > I told you not to do this in a comment on v4.
> > Keep the flag in page pool params and let the creation fail.  
> 
> There seems to be naming disagreement in the previous discussion,
> The simplest way seems to be reuse the
> PAGE_POOL_DMA_USE_PP_FRAG_COUNT and do the checking in the driver
> without introducing new macro or changing macro name.
> 
> Let's be more specific about what is your suggestion here:
> Do you mean keep the PP_FLAG_PAGE_FRAG flag and keep the below
> checking in page_pool_init(), right?
> 	if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
> 	    pool->p.flags & PP_FLAG_PAGE_FRAG)
> 		return -EINVAL;
> 
> Isn't it confusing to still say page frag is not supported
> for PAGE_POOL_DMA_USE_PP_FRAG_COUNT being true case when this
> patch will now add support for it, at least from API POV, the
> page_pool_alloc_frag() is always supported now.

I don't mind what the flag is called, I just want the check to stay
inside the page_pool code, acting on driver info passed inside
pp_params.

> Maybe remove the PP_FLAG_PAGE_FRAG and add a new macro named
> PP_FLAG_PAGE_SPLIT_IN_DRIVER, and do the checking as before in
> page_pool_init() like below?
> 	if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
> 	    pool->p.flags & PP_FLAG_PAGE_SPLIT_IN_DRIVER)
> 		return -EINVAL;

Yup, that sound good.