Re: [PATCH net-next v6 1/6] page_pool: frag API support for 32-bit arch with 64-bit DMA

[Ilias Apalodimas]

On Wed, 16 Aug 2023 at 15:49, Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
>
> On 2023/8/16 19:26, Ilias Apalodimas wrote:
> > Hi Yunsheng
> >
> > On Mon, 14 Aug 2023 at 15:59, Yunsheng Lin <linyunsheng@xxxxxxxxxx> 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
> >