From: Jesper Dangaard Brouer <brouer@redhat.com>
To: Felix Fietkau <nbd@nbd.name>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
netdev@vger.kernel.org, davem@davemloft.net, fw@strlen.de,
brouer@redhat.com
Subject: Re: [PATCH v2] net: use bulk free in kfree_skb_list
Date: Mon, 25 Mar 2019 17:03:37 +0100 [thread overview]
Message-ID: <20190325170337.145026cb@carbon> (raw)
In-Reply-To: <cb18c78c-6ca6-7a14-ee1d-8bf5ad123e24@nbd.name>
On Mon, 25 Mar 2019 10:35:35 +0100 Felix Fietkau <nbd@nbd.name> wrote:
> On 2019-03-25 10:31, Eric Dumazet wrote:
> > On 03/25/2019 02:09 AM, Felix Fietkau wrote:
> >> On 2019-03-25 09:51, Eric Dumazet wrote:
> >>> On 03/24/2019 09:56 AM, Felix Fietkau wrote:
> >>>>
> >>>> Since we're freeing multiple skbs, we might as well use bulk
> >>>> free to save a few cycles. Use the same conditions for bulk free
> >>>> as in napi_consume_skb.
> >>>
> >>> I do not believe kfree_skb_list() is used in the fast path, so do
> >>> we really need to make it so complex ?
> >>
> >> mac80211 uses it to free the fraglist from A-MSDU aggregated
> >> packets in the tx status path. That's one fast path where it gets
> >> used right now and the reason it was showing up in my perf
> >> traces.
Notice that kfree_skb_list(to_free) is used in (what I consider)
"fast-path" when the qdisc system drops packets, happens via to_free in
bottom of __dev_xmit_skb(). And fq_codel (which is default in most
distro's today) have an optimization (in fq_codel_drop()) for bulk
freeing SKBs, which will also benefit from this.
Thus, I still think it will be valuable to optimize kfree_skb_list().
> > This is not drop monitor friendly then....
> >
> > TX completion should use consume_skb() or dev_kfree_skb()
> >
> > BTW, I wonder what drop-monitor signal bulk free is sending ?
>
> Good point about the drop monitor. Would you prefer that I replace
> this patch with one that adds a consume_skb_list function and another
> one that makes mac80211 use it?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
next prev parent reply other threads:[~2019-03-25 16:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-24 16:56 [PATCH v2] net: use bulk free in kfree_skb_list Felix Fietkau
2019-03-24 18:23 ` Jesper Dangaard Brouer
2019-03-25 8:51 ` Eric Dumazet
2019-03-25 9:09 ` Felix Fietkau
2019-03-25 9:31 ` Eric Dumazet
2019-03-25 9:35 ` Felix Fietkau
2019-03-25 16:03 ` Jesper Dangaard Brouer [this message]
2019-03-25 8:54 ` Paolo Abeni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190325170337.145026cb@carbon \
--to=brouer@redhat.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=fw@strlen.de \
--cc=nbd@nbd.name \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.