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

On 2023/8/17 1:01, Ilias Apalodimas wrote:

On Wed, 16 Aug 2023 at 15:49, Yunsheng Lin [off-list ref] wrote:

quoted

On 2023/8/16 19:26, Ilias Apalodimas wrote:

quoted

Hi Yunsheng

On Mon, 14 Aug 2023 at 15:59, Yunsheng Lin [off-list ref] wrote:

quoted

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?

I used the key in the  original report:

https://www.spinics.net/lists/netdev/msg779890.html

quoted

Please see the bisection report below about a boot failure on
rk3288-rock2-square which is pointing to this patch.  The issue
appears to only happen with CONFIG_ARM_LPAE=y.

grepping the 'CONFIG_PHYS_ADDR_T_64BIT' seems to be more common?
https://elixir.free-electrons.com/linux/v6.4-rc6/K/ident/CONFIG_PHYS_ADDR_T_64BIT

quoted

quoted

quoted

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

Did you mean a xdp frame containing a frag page can not be passed to the
xdp core?
What is exact reason why the XDP verifier need a packet per page?
Is there a code block that you can point me to?

I wonder if it is still the case for now, as bnxt and mlx5 seems to be
supporting frag page and xdp now.

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

quoted

quoted

Thanks
/Ilias

.