Re: [PATCH net-next v1] net: page_pool: add page_pool_put_page_nosync()

[Alexander Lobakin]

From: Jakub Kicinski <kuba@xxxxxxxxxx>
Date: Thu, 19 Dec 2024 06:24:38 -0800

(to the author of the patch)

> On Thu, 19 Dec 2024 11:11:38 +0800 Guowei Dang wrote:
>> Add page_pool_put_page_nosync() to respond to dma_sync_size being 0.

If PP_FLAG_DMA_SYNC_DEV is set, dma_sync_size == 0 can happen only when
the HW didn't write anything *and* the driver uses only one page per
frame, no frags. Very unlikely case I'd say, adding a separate wrapper
for it makes no sense.

>>
>> The purpose of this is to make the semantics more obvious and may
>> enable removing some checkings in the future.

Which checks do you want to remove?

>>
>> And in the long term, treating the nosync scenario separately provides
>> more flexibility for the user and enable removing of the
>> PP_FLAG_DMA_SYNC_DEV in the future.

Why remove SYNC_DEV?

>>
>> Since we do have a page_pool_put_full_page(), adding a variant for
>> the nosync seems reasonable.

Not really. put_full_page() is for cases when either the HW-written size
is unknown or the driver uses frags, those are common and widely-used.

> 
> You should provide an upstream user with the API.

Would be nice to see a real example as I don't understand the purpose of
this function as well.

> But IMHO this just complicates the already very large API, 
> for little benefit. 
> I'm going to leave this in patchwork for a day in case page
> pool maintainers disagree, but I vote "no".

I don't see a reason for this either.

Thanks,
Olek