From: "Hanoch Haim (hhaim)" <hhaim@cisco.com>
To: Ilya Matveychikov <matvejchikov@gmail.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Olivier MATZ <olivier.matz@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v3] mbuf: cleanup rte_pktmbuf_lastseg(), fix atomic usage
Date: Wed, 15 Nov 2017 12:46:15 +0000 [thread overview]
Message-ID: <ad03f3a3142141afb9dc565500429df9@XCH-RTP-017.cisco.com> (raw)
In-Reply-To: <1D98684F-B8A9-4037-8534-0D4E3A1FD34C@gmail.com>
+Oliver,
Ilia, I assume there is a reason why Oliver did that, I just consolidate the code.
He didn't want to *write* the same value to mbuf field.
In the common case that pointer was already cleared by the driver, it is better to just read it. From cache synchronization perspective it will run faster.
Thanks,
Hanoh
-----Original Message-----
From: Ilya Matveychikov [mailto:matvejchikov@gmail.com]
Sent: Wednesday, November 15, 2017 1:14 PM
To: Hanoch Haim (hhaim)
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3] mbuf: cleanup rte_pktmbuf_lastseg(), fix atomic usage
> On Nov 15, 2017, at 1:14 PM, Hanoh Haim <hhaim@cisco.com> wrote:
>
> Signed-off-by: Hanoh Haim <hhaim@cisco.com>
> ---
> lib/librte_mbuf/rte_mbuf.h | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 7e326bb..ab110f8 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -1159,6 +1159,15 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
> __rte_mbuf_sanity_check(m, 1);
> }
>
> +
> +static __rte_always_inline void rte_pktmbuf_reset_before_free(struct
> +rte_mbuf *m) {
> + if (m->next != NULL) {
> + m->next = NULL;
> + m->nb_segs = 1;
> + }
> +}
> +
Probably it will be more clean to add something __te_pktmbuf_reset_nb_segs() without check for (m->next != NULL) and use it everywhere in mbuf’s the code, not only in
rte_pktmbuf_prefree_seg() function. And I think it will be better to have separate patch for that.
> /**
> * Allocate a new mbuf from a mempool.
> *
> @@ -1323,8 +1332,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
> m->ol_flags = 0;
>
> if (rte_mbuf_refcnt_update(md, -1) == 0) {
> - md->next = NULL;
> - md->nb_segs = 1;
Using rte_pktmbuf_reset_before_free() here adds unnecessary check for m->next in that path.
> + rte_pktmbuf_reset_before_free(md);
> rte_mbuf_refcnt_set(md, 1);
> rte_mbuf_raw_free(md);
> }
> @@ -1354,25 +1362,16 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
> if (RTE_MBUF_INDIRECT(m))
> rte_pktmbuf_detach(m);
>
> - if (m->next != NULL) {
> - m->next = NULL;
> - m->nb_segs = 1;
> - }
> -
> + rte_pktmbuf_reset_before_free(m);
> return m;
>
> - } else if (rte_atomic16_add_return(&m->refcnt_atomic, -1) == 0) {
> -
> + } else if (rte_mbuf_refcnt_update(m, -1) == 0) {
>
> if (RTE_MBUF_INDIRECT(m))
> rte_pktmbuf_detach(m);
>
> - if (m->next != NULL) {
> - m->next = NULL;
> - m->nb_segs = 1;
> - }
> + rte_pktmbuf_reset_before_free(m);
> rte_mbuf_refcnt_set(m, 1);
> -
> return m;
> }
> return NULL;
> --
> 2.9.3
>
next prev parent reply other threads:[~2017-11-15 12:46 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-15 9:14 Hanoh Haim
2017-11-15 11:13 ` Ilya Matveychikov
2017-11-15 12:46 ` Hanoch Haim (hhaim) [this message]
2017-11-15 17:30 ` Olivier MATZ
2017-11-16 7:16 ` Hanoch Haim (hhaim)
2017-11-16 8:07 ` Ilya Matveychikov
2017-11-16 8:42 ` Olivier MATZ
2017-11-16 9:06 ` Hanoch Haim (hhaim)
2017-11-16 9:32 ` Ilya Matveychikov
2017-11-16 9:37 ` Olivier MATZ
2017-11-16 9:44 ` Ilya Matveychikov
2017-11-16 10:54 ` Ananyev, Konstantin
2017-12-08 15:46 ` [dpdk-dev] [PATCH v4] mbuf: fix mbuf free performance with non atomic refcnt Olivier Matz
2017-12-08 16:04 ` Ilya Matveychikov
2017-12-08 16:19 ` Olivier MATZ
2017-12-08 16:37 ` Stephen Hemminger
2017-12-10 8:37 ` Hanoch Haim (hhaim)
2017-12-11 10:28 ` Olivier MATZ
2018-01-18 23:23 ` Thomas Monjalon
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=ad03f3a3142141afb9dc565500429df9@XCH-RTP-017.cisco.com \
--to=hhaim@cisco.com \
--cc=dev@dpdk.org \
--cc=matvejchikov@gmail.com \
--cc=olivier.matz@6wind.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).