[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230710113654.71d1ac84@kernel.org>
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.
Powered by blists - more mailing lists