[PATCH net-next] net: shrink napi_skb_cache_put()

[PATCH net-next] net: shrink napi_skb_cache_put()

[Eric Dumazet]

Following loop in napi_skb_cache_put() is unrolled by the compiler
even if CONFIG_KASAN is not enabled:

for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
	kasan_mempool_unpoison_object(nc->skb_cache[i],
				kmem_cache_size(net_hotdata.skbuff_cache));

We have 32 times this sequence, for a total of 384 bytes.

	48 8b 3d 00 00 00 00 	net_hotdata.skbuff_cache,%rdi
	e8 00 00 00 00       	call   kmem_cache_size

This is because kmem_cache_size() is an extern function,
and kasan_unpoison_object_data() is an inline function.

Cache kmem_cache_size() result in a temporary variable, and
make the loop conditional to CONFIG_KASAN.

After this patch, napi_skb_cache_put() is inlined in its callers.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexander Lobakin <aleksander.lobakin@intel.com>
---
 net/core/skbuff.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..5a8b48b201843f94b5fdaab3241801f642fbd1f0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1426,10 +1426,13 @@ static void napi_skb_cache_put(struct sk_buff *skb)
 	nc->skb_cache[nc->skb_count++] = skb;
 
 	if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
-		for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
-			kasan_mempool_unpoison_object(nc->skb_cache[i],
-						kmem_cache_size(net_hotdata.skbuff_cache));
+		if (IS_ENABLED(CONFIG_KASAN)) {
+			u32 size = kmem_cache_size(net_hotdata.skbuff_cache);
 
+			for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
+				kasan_mempool_unpoison_object(nc->skb_cache[i],
+							      size);
+		}
 		kmem_cache_free_bulk(net_hotdata.skbuff_cache, NAPI_SKB_CACHE_HALF,
 				     nc->skb_cache + NAPI_SKB_CACHE_HALF);
 		nc->skb_count = NAPI_SKB_CACHE_HALF;
-- 
2.51.0.869.ge66316f041-goog

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Kuniyuki Iwashima]

On Wed, Oct 15, 2025 at 4:38 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Following loop in napi_skb_cache_put() is unrolled by the compiler
> even if CONFIG_KASAN is not enabled:
>
> for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
>         kasan_mempool_unpoison_object(nc->skb_cache[i],
>                                 kmem_cache_size(net_hotdata.skbuff_cache));
>
> We have 32 times this sequence, for a total of 384 bytes.
>
>         48 8b 3d 00 00 00 00    net_hotdata.skbuff_cache,%rdi
>         e8 00 00 00 00          call   kmem_cache_size
>
> This is because kmem_cache_size() is an extern function,
> and kasan_unpoison_object_data() is an inline function.
>
> Cache kmem_cache_size() result in a temporary variable, and
> make the loop conditional to CONFIG_KASAN.
>
> After this patch, napi_skb_cache_put() is inlined in its callers.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Oh exactly, it's interesting that the compiler cannot optimise out
unused retval even though kmem_cache_size() is a single load :o

$ grep KASAN .config
CONFIG_HAVE_ARCH_KASAN=y
CONFIG_HAVE_ARCH_KASAN_VMALLOC=y
CONFIG_CC_HAS_KASAN_GENERIC=y
CONFIG_CC_HAS_KASAN_SW_TAGS=y
# CONFIG_KASAN is not set

$ objdump --disassemble=napi_skb_cache_put vmlinux
...
ffffffff81e9fe10 <napi_skb_cache_put>:
...
ffffffff81e9fe40: 48 8b 3d 89 bd d6 00 mov    0xd6bd89(%rip),%rdi
  # ffffffff82c0bbd0 <net_hotdata+0x150>
ffffffff81e9fe47: e8 a4 79 60 ff       call   ffffffff814a77f0 <kmem_cache_size>
ffffffff81e9fe4c: 83 eb 01             sub    $0x1,%ebx
ffffffff81e9fe4f: 75 ef                jne    ffffffff81e9fe40
<napi_skb_cache_put+0x30>

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Alexander Lobakin]

From: Eric Dumazet <edumazet@google.com>
Date: Wed, 15 Oct 2025 23:38:01 +0000

> Following loop in napi_skb_cache_put() is unrolled by the compiler
> even if CONFIG_KASAN is not enabled:
> 
> for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> 	kasan_mempool_unpoison_object(nc->skb_cache[i],
> 				kmem_cache_size(net_hotdata.skbuff_cache));
> 
> We have 32 times this sequence, for a total of 384 bytes.
> 
> 	48 8b 3d 00 00 00 00 	net_hotdata.skbuff_cache,%rdi
> 	e8 00 00 00 00       	call   kmem_cache_size
> 
> This is because kmem_cache_size() is an extern function,
> and kasan_unpoison_object_data() is an inline function.
> 
> Cache kmem_cache_size() result in a temporary variable, and
> make the loop conditional to CONFIG_KASAN.
> 
> After this patch, napi_skb_cache_put() is inlined in its callers.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Alexander Lobakin <aleksander.lobakin@intel.com>
> ---
>  net/core/skbuff.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..5a8b48b201843f94b5fdaab3241801f642fbd1f0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1426,10 +1426,13 @@ static void napi_skb_cache_put(struct sk_buff *skb)
>  	nc->skb_cache[nc->skb_count++] = skb;
>  
>  	if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
> -		for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> -			kasan_mempool_unpoison_object(nc->skb_cache[i],
> -						kmem_cache_size(net_hotdata.skbuff_cache));
> +		if (IS_ENABLED(CONFIG_KASAN)) {
> +			u32 size = kmem_cache_size(net_hotdata.skbuff_cache);
>  
> +			for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> +				kasan_mempool_unpoison_object(nc->skb_cache[i],
> +							      size);
> +		}

Very interesting; back when implementing napi_skb_cache*() family and
someone (most likely Jakub) asked me to add KASAN-related checks here,
I was comparing the object code and stopped on the current variant, as
without KASAN, the entire loop got optimized away (but only when
kmem_cache_size() is *not* a temporary variable).

Or does this patch addresses KASAN-enabled kernels? Either way, if this
patch really optimizes things:

Acked-by: Alexander Lobakin <aleksander.lobakin@intel.com>

>  		kmem_cache_free_bulk(net_hotdata.skbuff_cache, NAPI_SKB_CACHE_HALF,
>  				     nc->skb_cache + NAPI_SKB_CACHE_HALF);
>  		nc->skb_count = NAPI_SKB_CACHE_HALF;

Thanks,
Olek

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Eric Dumazet]

On Thu, Oct 16, 2025 at 3:20 AM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
> Date: Wed, 15 Oct 2025 23:38:01 +0000
>
> > Following loop in napi_skb_cache_put() is unrolled by the compiler
> > even if CONFIG_KASAN is not enabled:
> >
> > for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> >       kasan_mempool_unpoison_object(nc->skb_cache[i],
> >                               kmem_cache_size(net_hotdata.skbuff_cache));
> >
> > We have 32 times this sequence, for a total of 384 bytes.
> >
> >       48 8b 3d 00 00 00 00    net_hotdata.skbuff_cache,%rdi
> >       e8 00 00 00 00          call   kmem_cache_size
> >
> > This is because kmem_cache_size() is an extern function,
> > and kasan_unpoison_object_data() is an inline function.
> >
> > Cache kmem_cache_size() result in a temporary variable, and
> > make the loop conditional to CONFIG_KASAN.
> >
> > After this patch, napi_skb_cache_put() is inlined in its callers.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Alexander Lobakin <aleksander.lobakin@intel.com>
> > ---
> >  net/core/skbuff.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..5a8b48b201843f94b5fdaab3241801f642fbd1f0 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -1426,10 +1426,13 @@ static void napi_skb_cache_put(struct sk_buff *skb)
> >       nc->skb_cache[nc->skb_count++] = skb;
> >
> >       if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
> > -             for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> > -                     kasan_mempool_unpoison_object(nc->skb_cache[i],
> > -                                             kmem_cache_size(net_hotdata.skbuff_cache));
> > +             if (IS_ENABLED(CONFIG_KASAN)) {
> > +                     u32 size = kmem_cache_size(net_hotdata.skbuff_cache);
> >
> > +                     for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> > +                             kasan_mempool_unpoison_object(nc->skb_cache[i],
> > +                                                           size);
> > +             }
>
> Very interesting; back when implementing napi_skb_cache*() family and
> someone (most likely Jakub) asked me to add KASAN-related checks here,
> I was comparing the object code and stopped on the current variant, as
> without KASAN, the entire loop got optimized away (but only when
> kmem_cache_size() is *not* a temporary variable).
>
> Or does this patch addresses KASAN-enabled kernels? Either way, if this
> patch really optimizes things:

No, this is when CONFIG_KASAN is _not_ enabled.

(I have not checked when it is enabled, I do not care about the cost
of KASAN as long as it is not too expensive)

Compiler does not know anything about kmem_cache_size()
It could contain some memory cloberring, memory freeing, some kind of
destructive action.

So it has to call it 32 times.

And reload net_hotdata.skbuff_cache 32 times, because the value could
have been changed
by kmem_cache_size() (if kmem_cache_size() wanted to)

Not sure if kmem_cache_size() could be inlined.

Its use has been discouraged so I guess nobody cared.

>
> Acked-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>
> >               kmem_cache_free_bulk(net_hotdata.skbuff_cache, NAPI_SKB_CACHE_HALF,
> >                                    nc->skb_cache + NAPI_SKB_CACHE_HALF);
> >               nc->skb_count = NAPI_SKB_CACHE_HALF;
>
> Thanks,
> Olek

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Alexander Lobakin]

From: Eric Dumazet <edumazet@google.com>
Date: Thu, 16 Oct 2025 03:31:37 -0700

> On Thu, Oct 16, 2025 at 3:20 AM Alexander Lobakin
> <aleksander.lobakin@intel.com> wrote:
>>
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Wed, 15 Oct 2025 23:38:01 +0000
>>
>>> Following loop in napi_skb_cache_put() is unrolled by the compiler
>>> even if CONFIG_KASAN is not enabled:
>>>
>>> for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
>>>       kasan_mempool_unpoison_object(nc->skb_cache[i],
>>>                               kmem_cache_size(net_hotdata.skbuff_cache));
>>>
>>> We have 32 times this sequence, for a total of 384 bytes.
>>>
>>>       48 8b 3d 00 00 00 00    net_hotdata.skbuff_cache,%rdi
>>>       e8 00 00 00 00          call   kmem_cache_size
>>>
>>> This is because kmem_cache_size() is an extern function,
>>> and kasan_unpoison_object_data() is an inline function.
>>>
>>> Cache kmem_cache_size() result in a temporary variable, and
>>> make the loop conditional to CONFIG_KASAN.
>>>
>>> After this patch, napi_skb_cache_put() is inlined in its callers.
>>>
>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>> Cc: Alexander Lobakin <aleksander.lobakin@intel.com>
>>> ---
>>>  net/core/skbuff.c | 9 ++++++---
>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>>> index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..5a8b48b201843f94b5fdaab3241801f642fbd1f0 100644
>>> --- a/net/core/skbuff.c
>>> +++ b/net/core/skbuff.c
>>> @@ -1426,10 +1426,13 @@ static void napi_skb_cache_put(struct sk_buff *skb)
>>>       nc->skb_cache[nc->skb_count++] = skb;
>>>
>>>       if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
>>> -             for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
>>> -                     kasan_mempool_unpoison_object(nc->skb_cache[i],
>>> -                                             kmem_cache_size(net_hotdata.skbuff_cache));
>>> +             if (IS_ENABLED(CONFIG_KASAN)) {
>>> +                     u32 size = kmem_cache_size(net_hotdata.skbuff_cache);
>>>
>>> +                     for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
>>> +                             kasan_mempool_unpoison_object(nc->skb_cache[i],
>>> +                                                           size);
>>> +             }
>>
>> Very interesting; back when implementing napi_skb_cache*() family and
>> someone (most likely Jakub) asked me to add KASAN-related checks here,
>> I was comparing the object code and stopped on the current variant, as
>> without KASAN, the entire loop got optimized away (but only when
>> kmem_cache_size() is *not* a temporary variable).
>>
>> Or does this patch addresses KASAN-enabled kernels? Either way, if this
>> patch really optimizes things:
> 
> No, this is when CONFIG_KASAN is _not_ enabled.
> 
> (I have not checked when it is enabled, I do not care about the cost
> of KASAN as long as it is not too expensive)
> 
> Compiler does not know anything about kmem_cache_size()
> It could contain some memory cloberring, memory freeing, some kind of
> destructive action.
> 
> So it has to call it 32 times.
> 
> And reload net_hotdata.skbuff_cache 32 times, because the value could
> have been changed
> by kmem_cache_size() (if kmem_cache_size() wanted to)
> 
> Not sure if kmem_cache_size() could be inlined.

BTW doesn't napi_skb_cache_get() (inc. get_bulk()) suffer the same way?

> 
> Its use has been discouraged so I guess nobody cared.
> 
>>
>> Acked-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>>
>>>               kmem_cache_free_bulk(net_hotdata.skbuff_cache, NAPI_SKB_CACHE_HALF,
>>>                                    nc->skb_cache + NAPI_SKB_CACHE_HALF);
>>>               nc->skb_count = NAPI_SKB_CACHE_HALF;
Thanks,
Olek

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Eric Dumazet]

On Thu, Oct 16, 2025 at 4:08 AM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> BTW doesn't napi_skb_cache_get() (inc. get_bulk()) suffer the same way?

Probably, like other calls to napi_skb_cache_put(()

No loop there, so I guess there is no big deal.

I was looking at napi_skb_cache_put() because there is a lack of NUMA awareness,
and was curious to experiment with some strategies there.

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Eric Dumazet]

On Thu, Oct 16, 2025 at 5:56 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Oct 16, 2025 at 4:08 AM Alexander Lobakin
> <aleksander.lobakin@intel.com> wrote:
> >
> > From: Eric Dumazet <edumazet@google.com>
> >
> > BTW doesn't napi_skb_cache_get() (inc. get_bulk()) suffer the same way?
>
> Probably, like other calls to napi_skb_cache_put(()
>
> No loop there, so I guess there is no big deal.
>
> I was looking at napi_skb_cache_put() because there is a lack of NUMA awareness,
> and was curious to experiment with some strategies there.

If we cache kmem_cache_size() in net_hotdata, the compiler is able to
eliminate dead code
for CONFIG_KASAN=n

Maybe this looks better ?

diff --git a/include/net/hotdata.h b/include/net/hotdata.h
index 1aca9db99320f942b06b7d412d428a3045e87e60..f643e6a4647cc5e694a7044797f01a1107db46a9
100644
--- a/include/net/hotdata.h
+++ b/include/net/hotdata.h
@@ -33,9 +33,10 @@ struct net_hotdata {
        struct kmem_cache       *skbuff_cache;
        struct kmem_cache       *skbuff_fclone_cache;
        struct kmem_cache       *skb_small_head_cache;
+       u32                     skbuff_cache_size;
 #ifdef CONFIG_RPS
-       struct rps_sock_flow_table __rcu *rps_sock_flow_table;
        u32                     rps_cpu_mask;
+       struct rps_sock_flow_table __rcu *rps_sock_flow_table;
 #endif
        struct skb_defer_node __percpu *skb_defer_nodes;
        int                     gro_normal_batch;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c9e33f26852b63e930e33a406c19cc02f1821746..62b1acca55c7fd3e1fb7614cb0c625206db0ab3f
100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -365,7 +365,7 @@ static struct sk_buff *napi_skb_cache_get(void)

        skb = nc->skb_cache[--nc->skb_count];
        local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
-       kasan_mempool_unpoison_object(skb,
kmem_cache_size(net_hotdata.skbuff_cache));
+       kasan_mempool_unpoison_object(skb, net_hotdata.skbuff_cache_size);

        return skb;
 }
@@ -1504,7 +1504,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
        if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
                for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
                        kasan_mempool_unpoison_object(nc->skb_cache[i],
-
kmem_cache_size(net_hotdata.skbuff_cache));
+                                               net_hotdata.skbuff_cache_size);

                kmem_cache_free_bulk(net_hotdata.skbuff_cache,
NAPI_SKB_CACHE_HALF,
                                     nc->skb_cache + NAPI_SKB_CACHE_HALF);
@@ -5164,6 +5164,7 @@ void __init skb_init(void)
                                              offsetof(struct sk_buff, cb),
                                              sizeof_field(struct sk_buff, cb),
                                              NULL);
+       net_hotdata.skbuff_cache_size =
kmem_cache_size(net_hotdata.skbuff_cache);
        net_hotdata.skbuff_fclone_cache =
kmem_cache_create("skbuff_fclone_cache",
                                                sizeof(struct sk_buff_fclones),
                                                0,

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Eric Dumazet]

On Thu, Oct 16, 2025 at 6:29 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Oct 16, 2025 at 5:56 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Thu, Oct 16, 2025 at 4:08 AM Alexander Lobakin
> > <aleksander.lobakin@intel.com> wrote:
> > >
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > BTW doesn't napi_skb_cache_get() (inc. get_bulk()) suffer the same way?
> >
> > Probably, like other calls to napi_skb_cache_put(()
> >
> > No loop there, so I guess there is no big deal.
> >
> > I was looking at napi_skb_cache_put() because there is a lack of NUMA awareness,
> > and was curious to experiment with some strategies there.
>
> If we cache kmem_cache_size() in net_hotdata, the compiler is able to
> eliminate dead code
> for CONFIG_KASAN=n
>
> Maybe this looks better ?

No need to put this in net_hotdata, I was distracted by a 4byte hole
there, we can keep this hole for something  hot later.

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..f3b9356bebc06548a055355c5d1eb04c480f813f
100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -274,6 +274,8 @@ void *__netdev_alloc_frag_align(unsigned int
fragsz, unsigned int align_mask)
 }
 EXPORT_SYMBOL(__netdev_alloc_frag_align);

+u32 skbuff_cache_size __read_mostly;
+
 static struct sk_buff *napi_skb_cache_get(void)
 {
        struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
@@ -293,7 +295,7 @@ static struct sk_buff *napi_skb_cache_get(void)

        skb = nc->skb_cache[--nc->skb_count];
        local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
-       kasan_mempool_unpoison_object(skb,
kmem_cache_size(net_hotdata.skbuff_cache));
+       kasan_mempool_unpoison_object(skb, skbuff_cache_size);

        return skb;
 }
@@ -1428,7 +1430,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
        if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
                for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
                        kasan_mempool_unpoison_object(nc->skb_cache[i],
-
kmem_cache_size(net_hotdata.skbuff_cache));
+                                               skbuff_cache_size);

                kmem_cache_free_bulk(net_hotdata.skbuff_cache,
NAPI_SKB_CACHE_HALF,
                                     nc->skb_cache + NAPI_SKB_CACHE_HALF);
@@ -5116,6 +5118,8 @@ void __init skb_init(void)
                                              offsetof(struct sk_buff, cb),
                                              sizeof_field(struct sk_buff, cb),
                                              NULL);
+       skbuff_cache_size = kmem_cache_size(net_hotdata.skbuff_cache);
+
        net_hotdata.skbuff_fclone_cache =
kmem_cache_create("skbuff_fclone_cache",
                                                sizeof(struct sk_buff_fclones),
                                                0,

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Alexander Lobakin]

From: Eric Dumazet <edumazet@google.com>
Date: Thu, 16 Oct 2025 06:36:55 -0700

> On Thu, Oct 16, 2025 at 6:29 AM Eric Dumazet <edumazet@google.com> wrote:
>>
>> On Thu, Oct 16, 2025 at 5:56 AM Eric Dumazet <edumazet@google.com> wrote:
>>>
>>> On Thu, Oct 16, 2025 at 4:08 AM Alexander Lobakin
>>> <aleksander.lobakin@intel.com> wrote:
>>>>
>>>> From: Eric Dumazet <edumazet@google.com>
>>>>
>>>> BTW doesn't napi_skb_cache_get() (inc. get_bulk()) suffer the same way?
>>>
>>> Probably, like other calls to napi_skb_cache_put(()
>>>
>>> No loop there, so I guess there is no big deal.
>>>
>>> I was looking at napi_skb_cache_put() because there is a lack of NUMA awareness,
>>> and was curious to experiment with some strategies there.
>>
>> If we cache kmem_cache_size() in net_hotdata, the compiler is able to
>> eliminate dead code
>> for CONFIG_KASAN=n
>>
>> Maybe this looks better ?
> 
> No need to put this in net_hotdata, I was distracted by a 4byte hole
> there, we can keep this hole for something  hot later.

Yeah this looks good! It's not "hot" anyway, so let it lay freestanding.

> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..f3b9356bebc06548a055355c5d1eb04c480f813f
> 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -274,6 +274,8 @@ void *__netdev_alloc_frag_align(unsigned int
> fragsz, unsigned int align_mask)
>  }
>  EXPORT_SYMBOL(__netdev_alloc_frag_align);
> 
> +u32 skbuff_cache_size __read_mostly;

...but probably `static`?

> +
>  static struct sk_buff *napi_skb_cache_get(void)
>  {
>         struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
> @@ -293,7 +295,7 @@ static struct sk_buff *napi_skb_cache_get(void)
> 
>         skb = nc->skb_cache[--nc->skb_count];
>         local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
> -       kasan_mempool_unpoison_object(skb,
> kmem_cache_size(net_hotdata.skbuff_cache));
> +       kasan_mempool_unpoison_object(skb, skbuff_cache_size);
> 
>         return skb;
>  }
> @@ -1428,7 +1430,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
>         if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
>                 for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
>                         kasan_mempool_unpoison_object(nc->skb_cache[i],
> -
> kmem_cache_size(net_hotdata.skbuff_cache));
> +                                               skbuff_cache_size);
> 
>                 kmem_cache_free_bulk(net_hotdata.skbuff_cache,
> NAPI_SKB_CACHE_HALF,
>                                      nc->skb_cache + NAPI_SKB_CACHE_HALF);
> @@ -5116,6 +5118,8 @@ void __init skb_init(void)
>                                               offsetof(struct sk_buff, cb),
>                                               sizeof_field(struct sk_buff, cb),
>                                               NULL);
> +       skbuff_cache_size = kmem_cache_size(net_hotdata.skbuff_cache);
> +
>         net_hotdata.skbuff_fclone_cache =
> kmem_cache_create("skbuff_fclone_cache",
>                                                 sizeof(struct sk_buff_fclones),
>                                                 0,

Thanks,
Olek

Re: [PATCH net-next] net: shrink napi_skb_cache_put()

[Eric Dumazet]

On Thu, Oct 16, 2025 at 8:24 AM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
> Date: Thu, 16 Oct 2025 06:36:55 -0700
>
> > On Thu, Oct 16, 2025 at 6:29 AM Eric Dumazet <edumazet@google.com> wrote:
> >>
> >> On Thu, Oct 16, 2025 at 5:56 AM Eric Dumazet <edumazet@google.com> wrote:
> >>>
> >>> On Thu, Oct 16, 2025 at 4:08 AM Alexander Lobakin
> >>> <aleksander.lobakin@intel.com> wrote:
> >>>>
> >>>> From: Eric Dumazet <edumazet@google.com>
> >>>>
> >>>> BTW doesn't napi_skb_cache_get() (inc. get_bulk()) suffer the same way?
> >>>
> >>> Probably, like other calls to napi_skb_cache_put(()
> >>>
> >>> No loop there, so I guess there is no big deal.
> >>>
> >>> I was looking at napi_skb_cache_put() because there is a lack of NUMA awareness,
> >>> and was curious to experiment with some strategies there.
> >>
> >> If we cache kmem_cache_size() in net_hotdata, the compiler is able to
> >> eliminate dead code
> >> for CONFIG_KASAN=n
> >>
> >> Maybe this looks better ?
> >
> > No need to put this in net_hotdata, I was distracted by a 4byte hole
> > there, we can keep this hole for something  hot later.
>
> Yeah this looks good! It's not "hot" anyway, so let it lay freestanding.
>
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index bc12790017b0b5c0be99f8fb9d362b3730fa4eb0..f3b9356bebc06548a055355c5d1eb04c480f813f
> > 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -274,6 +274,8 @@ void *__netdev_alloc_frag_align(unsigned int
> > fragsz, unsigned int align_mask)
> >  }
> >  EXPORT_SYMBOL(__netdev_alloc_frag_align);
> >
> > +u32 skbuff_cache_size __read_mostly;
>
> ...but probably `static`?

Sure, I will add the static.

>
> > +
> >  static struct sk_buff *napi_skb_cache_get(void)
> >  {
> >         struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
> > @@ -293,7 +295,7 @@ static struct sk_buff *napi_skb_cache_get(void)
> >
> >         skb = nc->skb_cache[--nc->skb_count];
> >         local_unlock_nested_bh(&napi_alloc_cache.bh_lock);
> > -       kasan_mempool_unpoison_object(skb,
> > kmem_cache_size(net_hotdata.skbuff_cache));
> > +       kasan_mempool_unpoison_object(skb, skbuff_cache_size);
> >
> >         return skb;
> >  }
> > @@ -1428,7 +1430,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
> >         if (unlikely(nc->skb_count == NAPI_SKB_CACHE_SIZE)) {
> >                 for (i = NAPI_SKB_CACHE_HALF; i < NAPI_SKB_CACHE_SIZE; i++)
> >                         kasan_mempool_unpoison_object(nc->skb_cache[i],
> > -
> > kmem_cache_size(net_hotdata.skbuff_cache));
> > +                                               skbuff_cache_size);
> >
> >                 kmem_cache_free_bulk(net_hotdata.skbuff_cache,
> > NAPI_SKB_CACHE_HALF,
> >                                      nc->skb_cache + NAPI_SKB_CACHE_HALF);
> > @@ -5116,6 +5118,8 @@ void __init skb_init(void)
> >                                               offsetof(struct sk_buff, cb),
> >                                               sizeof_field(struct sk_buff, cb),
> >                                               NULL);
> > +       skbuff_cache_size = kmem_cache_size(net_hotdata.skbuff_cache);
> > +
> >         net_hotdata.skbuff_fclone_cache =
> > kmem_cache_create("skbuff_fclone_cache",
> >                                                 sizeof(struct sk_buff_fclones),
> >                                                 0,
>
> Thanks,
> Olek