From 8f094a9ac5d7424e460bc85708e99aa255432a87 Mon Sep 17 00:00:00 2001 From: Olivier Matz Date: Tue, 4 Apr 2017 18:28:02 +0200 Subject: [PATCH] mbuf: set mbuf fields while in pool 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 3cbf63f79f..06b699afa0 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1999,7 +1999,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; } @@ -2010,13 +2011,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 3fb2700ba6..207bf3dd32 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 2dc4d8b988..1efebec7c5 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); - } } /**