[dpdk-dev] [PATCH 3/9] mbuf: set mbuf fields while in pool

[Ananyev, Konstantin]

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, March 31, 2017 12:22 PM
> To: Olivier Matz <olivier.matz at 6wind.com>
> Cc: dev at dpdk.org; Ananyev, Konstantin <konstantin.ananyev at intel.com>; mb at smartsharesystems.com; Chilikin, Andrey
> <andrey.chilikin at intel.com>; jblunck at infradead.org; nelio.laranjeiro at 6wind.com; arybchenko at solarflare.com
> Subject: Re: [PATCH 3/9] mbuf: set mbuf fields while in pool
> 
> On Wed, Mar 08, 2017 at 10:41:55AM +0100, Olivier Matz wrote:
> > 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.
> >
> > Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
> > ---
> >  drivers/net/mlx5/mlx5_rxtx.c     |  5 ++---
> >  drivers/net/mpipe/mpipe_tilegx.c |  1 +
> >  lib/librte_mbuf/rte_mbuf.c       |  2 ++
> >  lib/librte_mbuf/rte_mbuf.h       | 42 +++++++++++++++++++++++++++++-----------
> >  4 files changed, 36 insertions(+), 14 deletions(-)
> >
> <snip>
> >  /**
> > @@ -1244,9 +1262,13 @@ 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 (RTE_MBUF_INDIRECT(m))
> >  			rte_pktmbuf_detach(m);
> > +
> > +		m->next = NULL;
> > +		m->nb_segs = 1;
> > +		rte_mbuf_refcnt_set(m, 1);
> > +
> >  		return m;
> >  	}
> >  	return NULL;
> 
> Do we need to make this change to prefree_seg? If we update the detach
> function to set the next point to null on detaching a segment, and if we
> change the "free" function which frees a whole chain of mbufs, we should
> be covered, should we not? If we are freeing a standalone segment, that
> segment should already have it's nb_segs and next pointers correct.

detach() is invoked only for indirect mbufs.
We can have a chain of direct mbufs too.
About free() - most PMD use either rte_pktmbuf_free_seg()
or rte_pktmbuf_prefree_seg();rte_mempool_put_bulk(); directly.
Konstantin