> -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev, Konstantin > Sent: Wednesday, September 11, 2019 1:42 PM > To: Van Haaren, Harry; Stephen Hemminger; Morten Brørup > Cc: olivier.m...@6wind.com; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] mbuf: add bulk free function > > > > > -----Original Message----- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Van Haaren, Harry > > Sent: Wednesday, September 11, 2019 12:30 PM > > To: Stephen Hemminger <step...@networkplumber.org>; Morten Brørup > <m...@smartsharesystems.com> > > Cc: olivier.m...@6wind.com; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] mbuf: add bulk free function > > > > > -----Original Message----- > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Stephen Hemminger > > > Sent: Wednesday, September 11, 2019 12:19 PM > > > To: Morten Brørup <m...@smartsharesystems.com> > > > Cc: olivier.m...@6wind.com; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH] mbuf: add bulk free function > > > > > > On Wed, 11 Sep 2019 09:19:08 +0000 > > > Morten Brørup <m...@smartsharesystems.com> wrote: > > > > > > > Add function for freeing a bulk of mbufs. > > > > > > > > Signed-off-by: Morten Brørup <m...@smartsharesystems.com> > > > > --- > > > > lib/librte_mbuf/rte_mbuf.h | 17 +++++++++++++++++ > > > > 1 file changed, 17 insertions(+) > > > > > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > > > index 98225ec80..f2e174da1 100644 > > > > --- a/lib/librte_mbuf/rte_mbuf.h > > > > +++ b/lib/librte_mbuf/rte_mbuf.h > > > > @@ -1907,6 +1907,23 @@ static inline void rte_pktmbuf_free(struct > rte_mbuf > > > *m) > > > > } > > > > } > > > > > > > > +/** > > > > + * Free a bulk of mbufs back into their original mempool. > > > > + * > > > > + * @param mbufs > > > > + * Array of pointers to mbufs > > > > + * @param count > > > > + * Array size > > > > + */ > > > > +static inline void > > > > +rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned count) > > > > +{ > > > > + unsigned idx = 0; > > > > + > > > > + for (idx = 0; idx < count; idx++) > > > > + rte_pktmbuf_free(mbufs[idx]); > > > > +} > > > > + > > > > > > You can optimize this to use mempool bulk put operation. > > > > I believe there's a nuance here - not all mbufs may come from the same > mempool. > > The for() approach will free each to its "home" mempool. > > The bulk() approach may return mbufs to pools they didn't originate from. > > > > For performance reasons it would be nice if they did, but we (in the DPDK > library) > > should not blindly assume that. > > I suppose Stephen is aware of that and suggests something similar to > What many PMDs are already doing. Let say in ixgbe: > static __rte_always_inline int > ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq) > { > .... > for (i = 0; i < txq->tx_rs_thresh; ++i, ++txep) { > /* free buffers one at a time */ > m = rte_pktmbuf_prefree_seg(txep->mbuf); > txep->mbuf = NULL; > > if (unlikely(m == NULL)) > continue; > > if (nb_free >= RTE_IXGBE_TX_MAX_FREE_BUF_SZ || > (nb_free > 0 && m->pool != free[0]->pool)) { > rte_mempool_put_bulk(free[0]->pool, > (void **)free, nb_free); > nb_free = 0; > } > > free[nb_free++] = m; > } > } > > Of course generic function will also need to go through all segments in > each packet. >
Thank you for the clarifying example! It looks like this optimization behaves as if RTE_LIBRTE_MBUF_DEBUG is not set. In that case, the existing rte_pktmbuf_free() could be optimized similarly to free multiple segments in bulk. (Except that the use of likely/unlikely in the mbuf library heavily favors single-segment mbufs, so such an optimization would go against this favoritism.) And yes, I see how rte_pktmbuf_free_bulk() could be optimized similarly. But I don't think it's appropriate for mbuf library functions to assume that RTE_LIBRTE_MBUF_DEBUG is not set. Although it could be implemented both ways, controlled by #ifdef RTE_LIBRTE_MBUF_DEBUG / #else / #endif. > > We could consider adding a 2nd functions, > rte_pktmbuf_free_bulk_to_single_mempool() > > or some better descriptive name. > > Probably a good idea too. > Konstantin > Feature creep. I prefer the generic function only. Med venlig hilsen / kind regards - Morten Brørup
