From: "Charles (Chas) Williams" <ciwillia@brocade.com>
To: <dev@dpdk.org>
Cc: <olivier.matz@6wind.com>,
"Charles (Chas) Williams" <ciwillia@brocade.com>
Subject: [dpdk-dev] [PATCH v3] mbuf: use refcnt = 0 when debugging
Date: Thu, 7 Sep 2017 17:21:51 -0400 [thread overview]
Message-ID: <1504819311-8441-1-git-send-email-ciwillia@brocade.com> (raw)
In-Reply-To: <1502122274-15657-1-git-send-email-ciwillia@brocade.com>
After commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool") is it
much harder to detect a "double free". If the developer makes a copy
of an mbuf pointer and frees it twice, this condition is never detected
and the mbuf gets returned to the pool twice.
Since this requires extra work to track, make this behavior conditional
on CONFIG_RTE_LIBRTE_MBUF_DEBUG.
Signed-off-by: Chas Williams <ciwillia@brocade.com>
---
lib/librte_mbuf/rte_mbuf.c | 2 +-
lib/librte_mbuf/rte_mbuf.h | 40 ++++++++++++++++++++++++++++++++++------
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 26a62b8..b0d222c 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -145,7 +145,7 @@ rte_pktmbuf_init(struct rte_mempool *mp,
m->pool = mp;
m->nb_segs = 1;
m->port = 0xff;
- rte_mbuf_refcnt_set(m, 1);
+ rte_mbuf_refcnt_set(m, RTE_MBUF_UNUSED_CNT);
m->next = NULL;
}
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index eaed7ee..1400b35 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -671,11 +671,15 @@ struct rte_pktmbuf_pool_private {
#ifdef RTE_LIBRTE_MBUF_DEBUG
+#define RTE_MBUF_UNUSED_CNT 0
+
/** check mbuf type in debug mode */
#define __rte_mbuf_sanity_check(m, is_h) rte_mbuf_sanity_check(m, is_h)
#else /* RTE_LIBRTE_MBUF_DEBUG */
+#define RTE_MBUF_UNUSED_CNT 1
+
/** check mbuf type in debug mode */
#define __rte_mbuf_sanity_check(m, is_h) do { } while (0)
@@ -721,6 +725,9 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
static inline uint16_t
rte_mbuf_refcnt_update(struct rte_mbuf *m, int16_t value)
{
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ RTE_ASSERT(rte_mbuf_refcnt_read(m) != 0);
+#endif
/*
* 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
@@ -791,10 +798,9 @@ void
rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header);
#define MBUF_RAW_ALLOC_CHECK(m) do { \
- RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1); \
+ RTE_ASSERT(rte_mbuf_refcnt_read(m) == RTE_MBUF_UNUSED_CNT); \
RTE_ASSERT((m)->next == NULL); \
RTE_ASSERT((m)->nb_segs == 1); \
- __rte_mbuf_sanity_check(m, 0); \
} while (0)
/**
@@ -825,6 +831,10 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp)
return NULL;
m = (struct rte_mbuf *)mb;
MBUF_RAW_ALLOC_CHECK(m);
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ rte_mbuf_refcnt_set(m, 1);
+#endif
+ __rte_mbuf_sanity_check(m, 0);
return m;
}
@@ -846,10 +856,9 @@ static __rte_always_inline void
rte_mbuf_raw_free(struct rte_mbuf *m)
{
RTE_ASSERT(RTE_MBUF_DIRECT(m));
- RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1);
+ RTE_ASSERT(rte_mbuf_refcnt_read(m) == RTE_MBUF_UNUSED_CNT);
RTE_ASSERT(m->next == NULL);
RTE_ASSERT(m->nb_segs == 1);
- __rte_mbuf_sanity_check(m, 0);
rte_mempool_put(m->pool, m);
}
@@ -1159,21 +1168,37 @@ static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool,
case 0:
while (idx != count) {
MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ rte_mbuf_refcnt_set(mbufs[idx], 1);
+#endif
+ __rte_mbuf_sanity_check(mbufs[idx], 0);
rte_pktmbuf_reset(mbufs[idx]);
idx++;
/* fall-through */
case 3:
MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ rte_mbuf_refcnt_set(mbufs[idx], 1);
+#endif
+ __rte_mbuf_sanity_check(mbufs[idx], 0);
rte_pktmbuf_reset(mbufs[idx]);
idx++;
/* fall-through */
case 2:
MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ rte_mbuf_refcnt_set(mbufs[idx], 1);
+#endif
+ __rte_mbuf_sanity_check(mbufs[idx], 0);
rte_pktmbuf_reset(mbufs[idx]);
idx++;
/* fall-through */
case 1:
MBUF_RAW_ALLOC_CHECK(mbufs[idx]);
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ rte_mbuf_refcnt_set(mbufs[idx], 1);
+#endif
+ __rte_mbuf_sanity_check(mbufs[idx], 0);
rte_pktmbuf_reset(mbufs[idx]);
idx++;
/* fall-through */
@@ -1271,7 +1296,7 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
if (rte_mbuf_refcnt_update(md, -1) == 0) {
md->next = NULL;
md->nb_segs = 1;
- rte_mbuf_refcnt_set(md, 1);
+ rte_mbuf_refcnt_set(md, RTE_MBUF_UNUSED_CNT);
rte_mbuf_raw_free(md);
}
}
@@ -1304,6 +1329,9 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
m->next = NULL;
m->nb_segs = 1;
}
+#ifdef RTE_LIBRTE_MBUF_DEBUG
+ rte_mbuf_refcnt_set(m, RTE_MBUF_UNUSED_CNT);
+#endif
return m;
@@ -1317,7 +1345,7 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
m->next = NULL;
m->nb_segs = 1;
}
- rte_mbuf_refcnt_set(m, 1);
+ rte_mbuf_refcnt_set(m, RTE_MBUF_UNUSED_CNT);
return m;
}
--
2.1.4
next prev parent reply other threads:[~2017-09-07 21:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-07 15:37 [dpdk-dev] [PATCH] " Charles (Chas) Williams
2017-08-07 16:11 ` [dpdk-dev] [PATCH v2] " Charles (Chas) Williams
2017-09-04 14:27 ` Radu Nicolau
2017-09-06 10:46 ` Chas Williams
2017-09-06 11:58 ` Ananyev, Konstantin
2017-09-06 13:55 ` Chas Williams
2017-09-06 14:53 ` Ananyev, Konstantin
2017-09-07 15:55 ` Chas Williams
2017-09-07 21:21 ` Charles (Chas) Williams [this message]
2017-09-20 11:23 ` [dpdk-dev] [PATCH v3] " Olivier MATZ
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=1504819311-8441-1-git-send-email-ciwillia@brocade.com \
--to=ciwillia@brocade.com \
--cc=dev@dpdk.org \
--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).