From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 3E2DE2FDD for ; Tue, 4 Apr 2017 18:32:28 +0200 (CEST) Received: from glumotte.dev.6wind.com (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id 1DFCE258BF; Tue, 4 Apr 2017 18:32:22 +0200 (CEST) From: Olivier Matz To: dev@dpdk.org Cc: konstantin.ananyev@intel.com, bruce.richardson@intel.com, mb@smartsharesystems.com, andrey.chilikin@intel.com, jblunck@infradead.org, nelio.laranjeiro@6wind.com, arybchenko@solarflare.com, thomas.monjalon@6wind.com, jerin.jacob@caviumnetworks.com Date: Tue, 4 Apr 2017 18:28:02 +0200 Message-Id: <20170404162807.20157-4-olivier.matz@6wind.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170404162807.20157-1-olivier.matz@6wind.com> References: <1488966121-22853-1-git-send-email-olivier.matz@6wind.com> <20170404162807.20157-1-olivier.matz@6wind.com> Subject: [dpdk-dev] [PATCH v2 3/8] mbuf: set mbuf fields while in pool 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: Tue, 04 Apr 2017 16:32:28 -0000 Set the value of m->refcnt to 1, m->nb_segs to 1 and m->next to NULL when the mbuf is stored inside the mempool (unused). This is done in rte_pktmbuf_prefree_seg(), before freeing or recycling a mbuf. Before this patch, the value of m->refcnt was expected to be 0 while in pool. The objectives are: - to avoid drivers to set m->next to NULL in the early Rx path, since this field is in the second 64B of the mbuf and its access could trigger a cache miss - rationalize the behavior of raw_alloc/raw_free: one is now the symmetric of the other, and refcnt is never changed in these functions. To optimize the freeing of the segments, we try try to only update m->refcnt, m->next, and m->nb_segs when it's required (idea from Konstantin Ananyev ). Signed-off-by: Olivier Matz --- drivers/net/mlx5/mlx5_rxtx.c | 5 ++-- lib/librte_mbuf/rte_mbuf.c | 2 ++ lib/librte_mbuf/rte_mbuf.h | 60 +++++++++++++++++++++++++++++++++++--------- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 0cbf98ffd..e048b8d0e 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1474,7 +1474,8 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) while (pkt != seg) { assert(pkt != (*rxq->elts)[idx]); rep = NEXT(pkt); - rte_mbuf_refcnt_set(pkt, 0); + NEXT(pkt) = NULL; + NB_SEGS(pkt) = 1; rte_mbuf_raw_free(pkt); pkt = rep; } @@ -1485,13 +1486,11 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) len = mlx5_rx_poll_len(rxq, cqe, cqe_cnt, &rss_hash_res); if (!len) { - rte_mbuf_refcnt_set(rep, 0); rte_mbuf_raw_free(rep); break; } if (unlikely(len == -1)) { /* RX error, packet is likely too large. */ - rte_mbuf_refcnt_set(rep, 0); rte_mbuf_raw_free(rep); ++rxq->stats.idropped; goto skip; diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index 3fb2700ba..207bf3dd3 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -146,6 +146,8 @@ rte_pktmbuf_init(struct rte_mempool *mp, m->pool = mp; m->nb_segs = 1; m->port = 0xff; + rte_mbuf_refcnt_set(m, 1); + m->next = NULL; } /* helper to create a mbuf pool */ diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 2dc4d8b98..1efebec7c 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -775,6 +775,11 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header); * initializing all the required fields. See rte_pktmbuf_reset(). * For standard needs, prefer rte_pktmbuf_alloc(). * + * The caller can expect that the following fields of the mbuf structure + * are initialized: buf_addr, buf_physaddr, buf_len, refcnt=1, nb_segs=1, + * next=NULL, pool, priv_size. The other fields must be initialized + * by the caller. + * * @param mp * The mempool from which mbuf is allocated. * @return @@ -789,8 +794,9 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp) if (rte_mempool_get(mp, &mb) < 0) return NULL; m = (struct rte_mbuf *)mb; - RTE_ASSERT(rte_mbuf_refcnt_read(m) == 0); - rte_mbuf_refcnt_set(m, 1); + RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1); + RTE_ASSERT(m->next == NULL); + RTE_ASSERT(m->nb_segs == 1); __rte_mbuf_sanity_check(m, 0); return m; @@ -799,8 +805,13 @@ static inline struct rte_mbuf *rte_mbuf_raw_alloc(struct rte_mempool *mp) /** * Put mbuf back into its original mempool. * - * The caller must ensure that the mbuf is direct and that the - * reference counter is 0. + * The caller must ensure that the mbuf is direct and properly + * reinitialized (refcnt=1, next=NULL, nb_segs=1), as done by + * rte_pktmbuf_prefree_seg(). + * + * This function should be used with care, when optimization is + * required. For standard needs, prefer rte_pktmbuf_free() or + * rte_pktmbuf_free_seg(). * * @param m * The mbuf to be freed. @@ -809,13 +820,16 @@ static inline void __attribute__((always_inline)) rte_mbuf_raw_free(struct rte_mbuf *m) { RTE_ASSERT(RTE_MBUF_DIRECT(m)); - RTE_ASSERT(rte_mbuf_refcnt_read(m) == 0); + RTE_ASSERT(rte_mbuf_refcnt_read(m) == 1); + RTE_ASSERT(m->next == NULL); + RTE_ASSERT(m->nb_segs == 1); + __rte_mbuf_sanity_check(m, 0); rte_mempool_put(m->pool, m); } /* compat with older versions */ __rte_deprecated -static inline void __attribute__((always_inline)) +static inline void __rte_mbuf_raw_free(struct rte_mbuf *m) { rte_mbuf_raw_free(m); @@ -1226,8 +1240,12 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m) m->data_len = 0; m->ol_flags = 0; - if (rte_mbuf_refcnt_update(md, -1) == 0) + if (rte_mbuf_refcnt_update(md, -1) == 0) { + md->next = NULL; + md->nb_segs = 1; + rte_mbuf_refcnt_set(md, 1); rte_mbuf_raw_free(md); + } } /** @@ -1250,10 +1268,30 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m) { __rte_mbuf_sanity_check(m, 0); - if (likely(rte_mbuf_refcnt_update(m, -1) == 0)) { - /* if this is an indirect mbuf, it is detached. */ + if (likely(rte_mbuf_refcnt_read(m) == 1)) { + if (RTE_MBUF_INDIRECT(m)) rte_pktmbuf_detach(m); + + if (m->next != NULL) { + m->next = NULL; + m->nb_segs = 1; + } + + return m; + + } else if (rte_atomic16_add_return(&m->refcnt_atomic, -1) == 0) { + + + if (RTE_MBUF_INDIRECT(m)) + rte_pktmbuf_detach(m); + + if (m->next != NULL) { + m->next = NULL; + m->nb_segs = 1; + } + rte_mbuf_refcnt_set(m, 1); + return m; } return NULL; @@ -1280,10 +1318,8 @@ static inline void __attribute__((always_inline)) rte_pktmbuf_free_seg(struct rte_mbuf *m) { m = rte_pktmbuf_prefree_seg(m); - if (likely(m != NULL)) { - m->next = NULL; + if (likely(m != NULL)) rte_mbuf_raw_free(m); - } } /** -- 2.11.0