linux-kernel - 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]

Message-ID: <81a8b412-f2b5-fac9-caa4-149d5bf71510@gmail.com>
Date:   Sun, 9 Jul 2023 20:39:45 +0800
From:   Yunsheng Lin <yunshenglin0825@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Yunsheng Lin <linyunsheng@...wei.com>
Cc:     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 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.

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;

Or any better suggestion?