[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CANn89iKr4LUSaXk_5p-cot6rxDngLJ8G6_F1eouF3mGRXdHhUg@mail.gmail.com>
Date: Fri, 7 Nov 2025 01:19:59 -0800
From: Eric Dumazet <edumazet@...gle.com>
To: "Hudson, Nick" <nhudson@...mai.com>
Cc: Jason Wang <jasowang@...hat.com>, Willem de Bruijn <willemdebruijn.kernel@...il.com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tun: use skb_attempt_defer_free in tun_do_read
On Fri, Nov 7, 2025 at 1:16 AM Hudson, Nick <nhudson@...mai.com> wrote:
>
>
>
> > On 7 Nov 2025, at 09:11, Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > !-------------------------------------------------------------------|
> > This Message Is From an External Sender
> > This message came from outside your organization.
> > |-------------------------------------------------------------------!
> >
> > On Fri, Nov 7, 2025 at 12:41 AM Hudson, Nick <nhudson@...mai.com> wrote:
> >>
> >>
> >>
> >>> On 7 Nov 2025, at 02:21, Jason Wang <jasowang@...hat.com> wrote:
> >>>
> >>> !-------------------------------------------------------------------|
> >>> This Message Is From an External Sender
> >>> This message came from outside your organization.
> >>> |-------------------------------------------------------------------!
> >>>
> >>> On Thu, Nov 6, 2025 at 11:51 PM Nick Hudson <nhudson@...mai.com> wrote:
> >>>>
> >>>> On a 640 CPU system running virtio-net VMs with the vhost-net driver, and
> >>>> multiqueue (64) tap devices testing has shown contention on the zone lock
> >>>> of the page allocator.
> >>>>
> >>>> A 'perf record -F99 -g sleep 5' of the CPUs where the vhost worker threads run shows
> >>>>
> >>>> # perf report -i perf.data.vhost --stdio --sort overhead --no-children | head -22
> >>>> ...
> >>>> #
> >>>> 100.00%
> >>>> |
> >>>> |--9.47%--queued_spin_lock_slowpath
> >>>> | |
> >>>> | --9.37%--_raw_spin_lock_irqsave
> >>>> | |
> >>>> | |--5.00%--__rmqueue_pcplist
> >>>> | | get_page_from_freelist
> >>>> | | __alloc_pages_noprof
> >>>> | | |
> >>>> | | |--3.34%--napi_alloc_skb
> >>>> #
> >>>>
> >>>> That is, for Rx packets
> >>>> - ksoftirqd threads pinned 1:1 to CPUs do SKB allocation.
> >>>> - vhost-net threads float across CPUs do SKB free.
> >>>>
> >>>> One method to avoid this contention is to free SKB allocations on the same
> >>>> CPU as they were allocated on. This allows freed pages to be placed on the
> >>>> per-cpu page (PCP) lists so that any new allocations can be taken directly
> >>>> from the PCP list rather than having to request new pages from the page
> >>>> allocator (and taking the zone lock).
> >>>>
> >>>> Fortunately, previous work has provided all the infrastructure to do this
> >>>> via the skb_attempt_defer_free call which this change uses instead of
> >>>> consume_skb in tun_do_read.
> >>>>
> >>>> Testing done with a 6.12 based kernel and the patch ported forward.
> >>>>
> >>>> Server is Dual Socket AMD SP5 - 2x AMD SP5 9845 (Turin) with 2 VMs
> >>>> Load generator: iPerf2 x 1200 clients MSS=400
> >>>>
> >>>> Before:
> >>>> Maximum traffic rate: 55Gbps
> >>>>
> >>>> After:
> >>>> Maximum traffic rate 110Gbps
> >>>> ---
> >>>> drivers/net/tun.c | 2 +-
> >>>> net/core/skbuff.c | 2 ++
> >>>> 2 files changed, 3 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>> index 8192740357a0..388f3ffc6657 100644
> >>>> --- a/drivers/net/tun.c
> >>>> +++ b/drivers/net/tun.c
> >>>> @@ -2185,7 +2185,7 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
> >>>> if (unlikely(ret < 0))
> >>>> kfree_skb(skb);
> >>>> else
> >>>> - consume_skb(skb);
> >>>> + skb_attempt_defer_free(skb);
> >>>> }
> >>>>
> >>>> return ret;
> >>>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> >>>> index 6be01454f262..89217c43c639 100644
> >>>> --- a/net/core/skbuff.c
> >>>> +++ b/net/core/skbuff.c
> >>>> @@ -7201,6 +7201,7 @@ nodefer: kfree_skb_napi_cache(skb);
> >>>> DEBUG_NET_WARN_ON_ONCE(skb_dst(skb));
> >>>> DEBUG_NET_WARN_ON_ONCE(skb->destructor);
> >>>> DEBUG_NET_WARN_ON_ONCE(skb_nfct(skb));
> >>>> + DEBUG_NET_WARN_ON_ONCE(skb_shared(skb));
> >>>
> >>> I may miss something but it looks there's no guarantee that the packet
> >>> sent to TAP is not shared.
> >>
> >> Yes, I did wonder.
> >>
> >> How about something like
> >>
> >> /**
> >> * consume_skb_attempt_defer - free an skbuff
> >> * @skb: buffer to free
> >> *
> >> * Drop a ref to the buffer and attempt to defer free it if the usage count
> >> * has hit zero.
> >> */
> >> void consume_skb_attempt_defer(struct sk_buff *skb)
> >> {
> >> if (!skb_unref(skb))
> >> return;
> >>
> >> trace_consume_skb(skb, __builtin_return_address(0));
> >>
> >> skb_attempt_defer_free(skb);
> >> }
> >> EXPORT_SYMBOL(consume_skb_attempt_defer);
> >>
> >> and an inline version for the !CONFIG_TRACEPOINTS case
> >
> > I will take care of the changes, have you seen my recent series ?
>
> Great, thanks. I did see your series and will evaluate the improvement in our test setup.
>
> >
> >
> > I think you are missing a few points….
>
> Sure, still learning.
Sure !
Make sure to add in your dev .config : CONFIG_DEBUG_NET=y
Powered by blists - more mailing lists