PING for review. Only received feedback from Stephen about the GCC optimized RTE_MBUF_DIRECT() macro being complex, and could be an inline function instead.
Venlig hilsen / Kind regards, -Morten Brørup > -----Original Message----- > From: Morten Brørup [mailto:[email protected]] > Sent: Wednesday, 27 August 2025 23.36 > To: [email protected] > Cc: Morten Brørup > Subject: [PATCH] mbuf: optimize segment prefree > > Eefactored rte_pktmbuf_prefree_seg() for both performance and > readability. > > With the optimized RTE_MBUF_DIRECT() macro, the common likely code path > now fits within one instruction cache line on x86-64 when built with > GCC. > > Signed-off-by: Morten Brørup <[email protected]> > --- > lib/mbuf/rte_mbuf.h | 52 ++++++++++++++++------------------------ > lib/mbuf/rte_mbuf_core.h | 8 +++++++ > 2 files changed, 28 insertions(+), 32 deletions(-) > > diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h > index 06ab7502a5..f4a348597f 100644 > --- a/lib/mbuf/rte_mbuf.h > +++ b/lib/mbuf/rte_mbuf.h > @@ -31,6 +31,7 @@ > * http://www.kohala.com/start/tcpipiv2.html > */ > > +#include <stdbool.h> > #include <stdint.h> > > #include <rte_common.h> > @@ -1423,44 +1424,31 @@ static inline int > __rte_pktmbuf_pinned_extbuf_decref(struct rte_mbuf *m) > static __rte_always_inline struct rte_mbuf * > rte_pktmbuf_prefree_seg(struct rte_mbuf *m) > { > - __rte_mbuf_sanity_check(m, 0); > - > - if (likely(rte_mbuf_refcnt_read(m) == 1)) { > - > - if (!RTE_MBUF_DIRECT(m)) { > - rte_pktmbuf_detach(m); > - if (RTE_MBUF_HAS_EXTBUF(m) && > - RTE_MBUF_HAS_PINNED_EXTBUF(m) && > - __rte_pktmbuf_pinned_extbuf_decref(m)) > - return NULL; > - } > - > - if (m->next != NULL) > - m->next = NULL; > - if (m->nb_segs != 1) > - m->nb_segs = 1; > + bool refcnt_not_one; > > - return m; > + __rte_mbuf_sanity_check(m, 0); > > - } else if (__rte_mbuf_refcnt_update(m, -1) == 0) { > + refcnt_not_one = unlikely(rte_mbuf_refcnt_read(m) != 1); > + if (refcnt_not_one && > + __rte_mbuf_refcnt_update(m, -1) != 0) > + return NULL; > > - if (!RTE_MBUF_DIRECT(m)) { > - rte_pktmbuf_detach(m); > - if (RTE_MBUF_HAS_EXTBUF(m) && > - RTE_MBUF_HAS_PINNED_EXTBUF(m) && > - __rte_pktmbuf_pinned_extbuf_decref(m)) > - return NULL; > - } > + if (unlikely(!RTE_MBUF_DIRECT(m))) { > + rte_pktmbuf_detach(m); > + if (RTE_MBUF_HAS_EXTBUF(m) && > + RTE_MBUF_HAS_PINNED_EXTBUF(m) && > + __rte_pktmbuf_pinned_extbuf_decref(m)) > + return NULL; > + } > > - if (m->next != NULL) > - m->next = NULL; > - if (m->nb_segs != 1) > - m->nb_segs = 1; > + if (refcnt_not_one) > rte_mbuf_refcnt_set(m, 1); > + if (m->nb_segs != 1) > + m->nb_segs = 1; > + if (m->next != NULL) > + m->next = NULL; > > - return m; > - } > - return NULL; > + return m; > } > > /** > diff --git a/lib/mbuf/rte_mbuf_core.h b/lib/mbuf/rte_mbuf_core.h > index a0df265b5d..a5242274d7 100644 > --- a/lib/mbuf/rte_mbuf_core.h > +++ b/lib/mbuf/rte_mbuf_core.h > @@ -715,6 +715,14 @@ struct rte_mbuf_ext_shared_info { > #define RTE_MBUF_DIRECT(mb) \ > (!((mb)->ol_flags & (RTE_MBUF_F_INDIRECT | RTE_MBUF_F_EXTERNAL))) > > +/* GCC only optimizes single-bit MSB tests this way, so do it by hand > with multi-bit. */ > +#if defined(RTE_TOOLCHAIN_GCC) && defined(RTE_ARCH_X86) > +#undef RTE_MBUF_DIRECT > +#define RTE_MBUF_DIRECT(mb) \ > + (!(((const uint8_t *)(mb))[offsetof(struct rte_mbuf, ol_flags) + > 7] & \ > + (uint8_t)((RTE_MBUF_F_INDIRECT | RTE_MBUF_F_EXTERNAL) >> 56))) > +#endif > + > /** Uninitialized or unspecified port. */ > #define RTE_MBUF_PORT_INVALID UINT16_MAX > /** For backwards compatibility. */ > -- > 2.43.0
