Re: [PATCH net-next] net: skb: introduce and use a single page frag cache

On Wed, 2022-09-21 at 13:23 -0700, Alexander H Duyck wrote:

On Wed, 2022-09-21 at 21:33 +0200, Paolo Abeni wrote:

quoted

Nice! I'll use that in v2, with page_ref_add(page, offset / SZ_1K - 1);
or we will leak the page.

No, the offset already takes care of the -1 via the "- SZ_1K". What we
are adding is references for the unused offset.

You are right. For some reasons I keep reading PAGE_SIZE instead of
'offset'.

quoted

quoted

It occurs to me that I think you are missing the check for the gfp_mask
and the reclaim and DMA flags values as a result with your change. I
think we will need to perform that check before we can do the direct
page allocation based on size.

Yes, the gtp_mask checks are required (it just stuck me a few moments
ago ;). I will move the code as you originally suggested.

quoted

quoted

Why? in the end we will still use an ancillary variable and the
napi_alloc_cache struct will be bigger (probaly not very relevant, but
for no gain at all).

It was mostly just about reducing instructions. The thought is we could
get rid of the storage of the napi cache entirely since the only thing
used is the page member, so if we just passed that around instead it
would save us the trouble and not really be another variable. Basically
we would be passing a frag cache pointer instead of a napi_alloc_cache.

In that case we will still duplicate a bit of code  -
this_cpu_ptr(&napi_alloc_cache) on both branches. gcc 11.3.1 here says
that the generated code is smaller without this change.

Cheers,

Paolo