From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 9DB78C350 for ; Tue, 4 Aug 2015 12:50:26 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 04 Aug 2015 03:50:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,608,1432623600"; d="scan'208";a="535731443" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.208.63]) by FMSMGA003.fm.intel.com with SMTP; 04 Aug 2015 03:50:23 -0700 Received: by (sSMTP sendmail emulation); Tue, 04 Aug 2015 11:50:22 +0025 Date: Tue, 4 Aug 2015 11:50:22 +0100 From: Bruce Richardson To: hepeng 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 Content-Disposition: inline In-Reply-To: <1438392394-19653-1-git-send-email-xnhp0320@icloud.com> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] [new]ixgbe:set txep.mbuf to NULL when calling ixgbe_tx_free_bufs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 04 Aug 2015 10:50:27 -0000 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 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 >