[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAC_iWjJ4Pi7Pj9Rm13y4aXBB3RsP9pTsfRf_A-OraXKwaO_xGA@mail.gmail.com>
Date: Wed, 16 Aug 2023 20:01:57 +0300
From: Ilias Apalodimas <ilias.apalodimas@...aro.org>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: davem@...emloft.net, kuba@...nel.org, 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>,
linux-rdma@...r.kernel.org
Subject: Re: [PATCH net-next v6 1/6] page_pool: frag API support for 32-bit
arch with 64-bit DMA
On Wed, 16 Aug 2023 at 15:49, Yunsheng Lin <linyunsheng@...wei.com> wrote:
>
> On 2023/8/16 19:26, Ilias Apalodimas wrote:
> > Hi Yunsheng
> >
> > On Mon, 14 Aug 2023 at 15:59, Yunsheng Lin <linyunsheng@...wei.com> wrote:
> >>
> >> Currently page_pool_alloc_frag() is not supported in 32-bit
> >> arch with 64-bit DMA because of the overlap issue between
> >> pp_frag_count and dma_addr_upper in 'struct page' for those
> >> arches, which seems to be quite common, see [1], which means
> >> driver may need to handle it when using frag API.
> >
> > That wasn't so common. IIRC it was a single TI platform that was breaking?
>
> I am not so sure about that as grepping 'ARM_LPAE' has a long
> list for that.
Shouldn't we be grepping for CONFIG_ARCH_DMA_ADDR_T_64BIT and
PHYS_ADDR_T_64BIT to find the affected platforms? Why LPAE?
>
> >
> >>
> >> In order to simplify the driver's work when using frag API
> >> this patch allows page_pool_alloc_frag() to call
> >> page_pool_alloc_pages() to return pages for those arches.
> >
> > Do we have any use cases of people needing this? Those architectures
> > should be long dead and although we have to support them in the
> > kernel, I don't personally see the advantage of adjusting the API to
> > do that. Right now we have a very clear separation between allocating
> > pages or fragments. Why should we hide a page allocation under a
> > frag allocation? A driver writer can simply allocate pages for those
> > boards. Am I the only one not seeing a clean win here?
>
> It is also a part of removing the per page_pool PP_FLAG_PAGE_FRAG flag
> in this patchset.
Yes, that happens *because* of this patchset. I am not against the
change. In fact, I'll have a closer look tomorrow. I am just trying
to figure out if we really need it. When the recycling patches were
introduced into page pool we had a very specific reason. Due to the
XDP verifier we *had* to allocate a packet per page. That was
expensive so we added the recycling capabilities to compensate and get
some performance back. Eventually we added page fragments and had a
very clear separation on the API.
Regards
/Ilias
>
> >
> > Thanks
> > /Ilias
> >
Powered by blists - more mailing lists