Re: [PATCH v1 06/15] net: page_pool: add ->scrub mem provider callback

[Pavel Begunkov <asml.silence@gmail.com>]

On 10/9/24 22:00, Mina Almasry wrote:
> On Mon, Oct 7, 2024 at 3:16 PM David Wei <dw@davidwei.uk> wrote:
>>
>> From: Pavel Begunkov <asml.silence@gmail.com>
>>
>> page pool is now waiting for all ppiovs to return before destroying
>> itself, and for that to happen the memory provider might need to push
>> some buffers, flush caches and so on.
>>
>> todo: we'll try to get by without it before the final release
>>
> 
> Is the intention to drop this todo and stick with this patch, or to
> move ahead with this patch?

Heh, I overlooked this todo. The plan is to actually leave it
as is, it's by far the simplest way and doesn't really gets
into anyone's way as it's a slow path.

> To be honest, I think I read in a follow up patch that you want to
> unref all the memory on page_pool_destory, which is not how the
> page_pool is used today. Tdoay page_pool_destroy does not reclaim
> memory. Changing that may be OK.

It doesn't because it can't (not breaking anything), which is a
problem as the page pool might never get destroyed. io_uring
doesn't change that, a buffer can't be reclaimed while anything
in the kernel stack holds it. It's only when it's given to the
user we can force it back out of there.

And it has to happen one way or another, we can't trust the
user to put buffers back, it's just devmem does that by temporarily
attaching the lifetime of such buffers to a socket.

> But I'm not sure this is generic change that should be put in the
> page_pool providers. I don't envision other providers implementing
> this. I think they'll be more interested in using the page_pool the
> way it's used today.

If the pp/net maintainers abhor it, I could try to replace it
with some "inventive" solution, which most likely would need
referencing all io_uring zcrx requests, but otherwise I'd
prefer to leave it as is.

> I would suggest that instead of making this a page_pool provider
> thing, to instead have your iouring code listen to a notification that
> a new generic notificatino that page_pool is being destroyed or an
> rx-queue is being destroyed or something like that, and doing the
> scrubbing based on that, maybe?

You can say it listens to the page pool being destroyed, exactly
what it's interesting in. Trying to catch the destruction of an
rx-queue is the same thing but with jumping more hops and indirectly
deriving that the page pool is killed.

-- 
Pavel Begunkov