From: Olivier Matz <olivier.matz@6wind.com>
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2] mbuf: optimize rte_mbuf_refcnt_update
Date: Mon, 8 Jun 2015 16:57:22 +0200 [thread overview]
Message-ID: <1433775442-31438-1-git-send-email-olivier.matz@6wind.com> (raw)
In-Reply-To: <1433151145-8176-1-git-send-email-olivier.matz@6wind.com>
In __rte_pktmbuf_prefree_seg(), there was an optimization to avoid using
a costly atomic operation when updating the mbuf reference counter if
its value is 1. Indeed, it means that we are the only owner of the mbuf,
and therefore nobody can change it at the same time.
We can generalize this optimization directly in rte_mbuf_refcnt_update()
so the other callers of this function, like rte_pktmbuf_attach(), can
also take advantage of this optimization.
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
lib/librte_mbuf/rte_mbuf.h | 57 +++++++++++++++++++++++-----------------------
1 file changed, 28 insertions(+), 29 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ab6de67..6c9cfd6 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -426,21 +426,6 @@ if (!(exp)) { \
#ifdef RTE_MBUF_REFCNT_ATOMIC
/**
- * Adds given value to an mbuf's refcnt and returns its new value.
- * @param m
- * Mbuf to update
- * @param value
- * Value to add/subtract
- * @return
- * Updated value
- */
-static inline uint16_t
-rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
-{
- return (uint16_t)(rte_atomic16_add_return(&m->refcnt_atomic, value));
-}
-
-/**
* Reads the value of an mbuf's refcnt.
* @param m
* Mbuf to read
@@ -466,6 +451,33 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
rte_atomic16_set(&m->refcnt_atomic, new_value);
}
+/**
+ * Adds given value to an mbuf's refcnt and returns its new value.
+ * @param m
+ * Mbuf to update
+ * @param value
+ * Value to add/subtract
+ * @return
+ * Updated value
+ */
+static inline uint16_t
+rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
+{
+ /*
+ * The atomic_add is an expensive operation, so we don't want to
+ * call it in the case where we know we are the uniq holder of
+ * this mbuf (i.e. ref_cnt == 1). Otherwise, an atomic
+ * operation has to be used because concurrent accesses on the
+ * reference counter can occur.
+ */
+ if (likely(rte_mbuf_refcnt_read(m) == 1)) {
+ rte_mbuf_refcnt_set(m, 1 + value);
+ return 1 + value;
+ }
+
+ return (uint16_t)(rte_atomic16_add_return(&m->refcnt_atomic, value));
+}
+
#else /* ! RTE_MBUF_REFCNT_ATOMIC */
/**
@@ -895,20 +907,7 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
{
__rte_mbuf_sanity_check(m, 0);
- /*
- * Check to see if this is the last reference to the mbuf.
- * Note: the double check here is deliberate. If the ref_cnt is "atomic"
- * the call to "refcnt_update" is a very expensive operation, so we
- * don't want to call it in the case where we know we are the holder
- * of the last reference to this mbuf i.e. ref_cnt == 1.
- * If however, ref_cnt != 1, it's still possible that we may still be
- * the final decrementer of the count, so we need to check that
- * result also, to make sure the mbuf is freed properly.
- */
- if (likely (rte_mbuf_refcnt_read(m) == 1) ||
- likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
-
- rte_mbuf_refcnt_set(m, 0);
+ if (likely(rte_mbuf_refcnt_update(m, -1) == 0)) {
/* if this is an indirect mbuf, then
* - detach mbuf
--
2.1.4
next prev parent reply other threads:[~2015-06-08 14:57 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-01 9:32 [dpdk-dev] [PATCH] mbuf: optimize first reference increment in rte_pktmbuf_attach Olivier Matz
2015-06-03 10:59 ` Bruce Richardson
2015-06-05 9:07 ` Olivier MATZ
2015-06-08 14:57 ` Olivier Matz [this message]
2015-06-09 12:57 ` [dpdk-dev] [PATCH v2] mbuf: optimize rte_mbuf_refcnt_update Bruce Richardson
2015-06-12 14:10 ` Thomas Monjalon
2015-12-27 9:39 Hanoch Haim (hhaim)
2016-01-04 13:53 ` Olivier MATZ
2016-01-04 14:43 ` Hanoch Haim (hhaim)
2016-01-05 10:57 ` Olivier MATZ
2016-01-05 11:11 ` Hanoch Haim (hhaim)
2016-01-05 12:12 ` Olivier MATZ
2016-01-13 11:48 ` Bruce Richardson
2016-01-13 16:28 ` Hanoch Haim (hhaim)
2016-01-13 16:40 ` Bruce Richardson
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=1433775442-31438-1-git-send-email-olivier.matz@6wind.com \
--to=olivier.matz@6wind.com \
--cc=dev@dpdk.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 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).