From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [PATCH 3/9] mbuf: set mbuf fields while in pool
Date: Fri, 31 Mar 2017 12:21:38 +0100
Message-ID: <20170331112138.GA12052@bricha3-MOBL3.ger.corp.intel.com>
References: <1485271173-13408-1-git-send-email-olivier.matz@6wind.com>
 <1488966121-22853-1-git-send-email-olivier.matz@6wind.com>
 <1488966121-22853-4-git-send-email-olivier.matz@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org, konstantin.ananyev@intel.com, mb@smartsharesystems.com,
 andrey.chilikin@intel.com, jblunck@infradead.org,
 nelio.laranjeiro@6wind.com, arybchenko@solarflare.com
To: Olivier Matz <olivier.matz@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id B68B5108D
 for <dev@dpdk.org>; Fri, 31 Mar 2017 13:21:43 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1488966121-22853-4-git-send-email-olivier.matz@6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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@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.

/Bruce