Re: [PATCH net-next v1] page_pool: check for dma_sync_size earlier

[Ilias Apalodimas]

On Fri, 11 Oct 2024 at 11:55, Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
>
> On 2024/10/11 14:31, Furong Xu wrote:
> > Hi Ilias,
> >
> > On Fri, 11 Oct 2024 08:06:04 +0300, Ilias Apalodimas <ilias.apalodimas@xxxxxxxxxx> wrote:
> >
> >> Hi Furong,
> >>
> >> On Fri, 11 Oct 2024 at 05:15, Furong Xu <0x1207@xxxxxxxxx> wrote:
> >>>
> >>> On Thu, 10 Oct 2024 19:53:39 +0800, Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:
> >>>
> >>>> Is there any reason that those drivers not to unset the PP_FLAG_DMA_SYNC_DEV
> >>>> when calling page_pool_create()?
> >>>> Does it only need dma sync for some cases and not need dma sync for other
> >>>> cases? if so, why not do the dma sync in the driver instead?
> >>>
> >>> The answer is in this commit:
> >>> https://git.kernel.org/netdev/net/c/5546da79e6cc
> >>
> >> I am not sure I am following. Where does the stmmac driver call a sync
> >> with len 0?
> > For now, only drivers/net/ethernet/freescale/fec_main.c does.
> > And stmmac driver does not yet, but I will send another patch to make it call sync with
> > len 0. This is a proper fix as Jakub Kicinski suggested.
>
> In order to support the above use case, it seems there might be two
> options here:
> 1. Driver calls page_pool_create() without PP_FLAG_DMA_SYNC_DEV and
>    handle the dma sync itself.
> 2. Page_pool may provides a non-dma-sync version of page_pool_put_page()
>    API even when Driver calls page_pool_create() with PP_FLAG_DMA_SYNC_DEV.
>
> Maybe option 2 is better one in the longer term as it may provide some
> flexibility for the user and enable removing of the DMA_SYNC_DEV in the
> future?

Case 2 would be what this patch does. We already treat -1 as the
maximum DMA memory size. I think it's ok to treat 0 as 'don't sync'. I
need to figure out why people need this.
IOW you still have to sync the page to use it. Now you can do it when
allocating it, but why is that cheaper or preferable?

Thanks
/Ilias