From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-000f0801.pphosted.com (mx0b-000f0801.pphosted.com [67.231.152.113]) by dpdk.org (Postfix) with ESMTP id 6EB672C18 for ; Mon, 7 Aug 2017 18:11:33 +0200 (CEST) Received: from pps.filterd (m0048192.ppops.net [127.0.0.1]) by mx0b-000f0801.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v77G30Gg027938; Mon, 7 Aug 2017 09:11:31 -0700 Received: from brmwp-exmb11.corp.brocade.com ([208.47.132.227]) by mx0b-000f0801.pphosted.com with ESMTP id 2c5da14tux-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 07 Aug 2017 09:11:31 -0700 Received: from confsjhq2-2-001.brocade.com (10.252.68.2) by BRMWP-EXMB11.corp.brocade.com (172.16.59.77) with Microsoft SMTP Server (TLS) id 15.0.1293.2; Mon, 7 Aug 2017 10:11:27 -0600 From: "Charles (Chas) Williams" To: CC: , "Charles (Chas) Williams" Date: Mon, 7 Aug 2017 12:11:14 -0400 Message-ID: <1502122274-15657-1-git-send-email-ciwillia@brocade.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1502120243-8902-1-git-send-email-ciwillia@brocade.com> References: <1502120243-8902-1-git-send-email-ciwillia@brocade.com> MIME-Version: 1.0 Content-Type: text/plain X-ClientProxiedBy: hq1wp-excas13.corp.brocade.com (10.70.36.103) To BRMWP-EXMB11.corp.brocade.com (172.16.59.77) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-08-07_11:, , signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=3 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1708070267 Subject: [dpdk-dev] [PATCH v2] mbuf: use refcnt = 0 when debugging X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Aug 2017 16:11:33 -0000 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 --- lib/librte_mbuf/rte_mbuf.c | 2 +- lib/librte_mbuf/rte_mbuf.h | 42 +++++++++++++++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 8 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..3e00373 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,10 +1329,13 @@ 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; - } 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)) @@ -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