From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [PATCH] [new]ixgbe:set txep.mbuf to NULL when
 calling ixgbe_tx_free_bufs
Date: Tue, 4 Aug 2015 11:50:22 +0100
Message-ID: <20150804105022.GA8948@bricha3-MOBL3>
References: <1438392394-19653-1-git-send-email-xnhp0320@icloud.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org
To: hepeng <xnhp0320@icloud.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 9DB78C350
 for <dev@dpdk.org>; Tue,  4 Aug 2015 12:50:26 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <1438392394-19653-1-git-send-email-xnhp0320@icloud.com>
List-Id: patches and discussions about DPDK <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 Sat, Aug 01, 2015 at 09:26:34AM +0800, hepeng wrote:
> In *ixgbe_tx_free_bufs*, after recycling some tx entries, one should set their mbuf pointers to NULL.
> 
> The first path is not correct, the txep->mbuf should be set to NULL no matter if it is recycled into mempool
> Signed-off-by: hepeng <xnhp0320@icloud.com>

Why is this necessary? Setting the mbuf pointer to null introduces an extra write
to the descriptor ring for every packet. Right now the free entries are simply
tracking using the indexes in the tx_queue structure, there is no need to set
the mbufs to NULL as well, as far as I can see.

/Bruce

> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> index 1c16dec..e7ce740 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> @@ -612,6 +612,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
>  	 */
>  	txep = &txq->sw_ring_v[txq->tx_next_dd - (n - 1)];
>  	m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
> +    txep[0].mbuf = NULL;
>  	if (likely(m != NULL)) {
>  		free[0] = m;
>  		nb_free = 1;
> @@ -632,11 +633,21 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
>  	} else {
>  		for (i = 1; i < n; i++) {
>  			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
> -			if (m != NULL)
> +			if (m != NULL) {
>  				rte_mempool_put(m->pool, m);
> +            }
>  		}
>  	}
>  
> +    /*
> +     * No matter the mbufs have been put back to mempool or not,
> +     * we should set the txep[i].mbuf to NULL
> +     */
> +
> +    for( i = 1; i < n; i++) {
> +        txep[i].mbuf = NULL;
> +    }
> +
>  	/* buffers were freed, update counters */
>  	txq->nb_tx_free = (uint16_t)(txq->nb_tx_free + txq->tx_rs_thresh);
>  	txq->tx_next_dd = (uint16_t)(txq->tx_next_dd + txq->tx_rs_thresh);
> -- 
> 1.9.1
>