From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id B66061BA03 for ; Wed, 25 Oct 2017 18:50:04 +0200 (CEST) Received: by mail-wm0-f68.google.com with SMTP id q124so3126616wmb.0 for ; Wed, 25 Oct 2017 09:50:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=P9+M1+L12Vb7miM9vfU829ZXxTk8xdKqSduaC8hB0gU=; b=ifocQE1s3piUMFbE9z4EhRvS+zwBt4Uj1Q8vjSDUA2R0Vz+hVLNMVYB8a/dWgei1+3 7uHKJBlVuIiGb0/rborbQc15atqWdGiW3dRBN0ys4x5L/85/YWN9MBqjWoQU56mSzwdw f0w16K27GJbylWeoaEapY/R5oa1/v7OPj5hEvKDGd1hBQG95HFKgV9lwKMFAue0SgK7K EfWXj6edIwCjMSZ36T04MrQUbuLVzzYXFhZEA+vJnWHgjmYrPWeJhDO+4W1TnGlvR5tL +A94aenjW6mA4aRJ7hSGeAIefMv0dj66x6cUiKiu4HJAOKalmOcRiGX6h/evKLKecFVC BoGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=P9+M1+L12Vb7miM9vfU829ZXxTk8xdKqSduaC8hB0gU=; b=nfnKm7qYsmzmyXpo148JL6zmajKiwpgN6Q35gbPKI26s0bhcJpCE31Z2orK/TE4n76 tkCSFaihlQeXvuUk4OepZjwEPSwmT49G3aYMVL8YOui6rPpGxzZJ2b8VurqyaHVBelil MSG268Y5/F/WTU4eiwlWsIK9ctltSraydo4oxy2uuGO7lmK/JtbXp5pp7ifooH236YOG Li+I3PjjAnnhFzZeZ0qY+bR9uWUxZAE8MCKu5c6d0b1gSrzM3wXUKH969TNLRI4cISY2 YJsqs0thVk9NNIXLQdyZmJ0WLMAjiZj3XswKmj7UTm2S1eOm+rv8fgFz9nPZyzXwLAhO v1RQ== X-Gm-Message-State: AMCzsaXi1qIjwuclSwDMfuWX6TBtloIqh1aPnzvSS4vQsk02b0eye4vm 7aEd20H/ERJevShhkNe+D4hMjA== X-Google-Smtp-Source: ABhQp+Ri4q+mm4xxLbk9MVLLYHTA84BtLvUm1vtmjZww2jXu8flxnfbe4bvpJ4nvklNvg93ugYVyFw== X-Received: by 10.28.111.206 with SMTP id c75mr2465590wmi.123.1508950204459; Wed, 25 Oct 2017 09:50:04 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id v7sm2566360wrd.56.2017.10.25.09.50.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Oct 2017 09:50:03 -0700 (PDT) Date: Wed, 25 Oct 2017 18:49:52 +0200 From: Adrien Mazarguil To: Ophir Munk Cc: dev@dpdk.org, Thomas Monjalon , Olga Shern , Matan Azrad Message-ID: <20171025164952.GI26782@6wind.com> References: <1508752838-30408-1-git-send-email-ophirmu@mellanox.com> <1508768520-4810-1-git-send-email-ophirmu@mellanox.com> <1508768520-4810-6-git-send-email-ophirmu@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1508768520-4810-6-git-send-email-ophirmu@mellanox.com> Subject: Re: [dpdk-dev] [PATCH v2 5/7] net/mlx4: remove unnecessary variables in Tx burst X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Oct 2017 16:50:04 -0000 Hi Ophir/Matan, On Mon, Oct 23, 2017 at 02:21:58PM +0000, Ophir Munk wrote: > From: Matan Azrad > > Remove usage of variables which doesn't add new information for > performance improvement. > > Signed-off-by: Matan Azrad I'm almost 100% sure this commit wasn't validated for performance on its own. Don't mention "performance improvement" in that case. If you're removing a couple of local variables for readability, just say so. More below. > --- > drivers/net/mlx4/mlx4_rxtx.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/mlx4/mlx4_rxtx.c b/drivers/net/mlx4/mlx4_rxtx.c > index 014a6d3..e8d9a35 100644 > --- a/drivers/net/mlx4/mlx4_rxtx.c > +++ b/drivers/net/mlx4/mlx4_rxtx.c > @@ -285,8 +285,6 @@ mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n) > struct txq *txq = (struct txq *)dpdk_txq; > unsigned int elts_head = txq->elts_head; > const unsigned int elts_n = txq->elts_n; > - unsigned int elts_comp = 0; > - unsigned int bytes_sent = 0; > unsigned int i; > unsigned int max; > struct mlx4_sq *sq = &txq->msq; > @@ -498,8 +496,7 @@ mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n) > MLX4_BIT_WQE_OWN : 0)); > sq->head += nr_txbbs; > elt->buf = buf; > - bytes_sent += buf->pkt_len; > - ++elts_comp; > + txq->stats.obytes += buf->pkt_len; > elts_head = elts_head_next; > } > /* Take a shortcut if nothing must be sent. */ > @@ -507,13 +504,12 @@ mlx4_tx_burst(void *dpdk_txq, struct rte_mbuf **pkts, uint16_t pkts_n) > return 0; > /* Increment send statistics counters. */ > txq->stats.opackets += i; > - txq->stats.obytes += bytes_sent; > /* Make sure that descriptors are written before doorbell record. */ > rte_wmb(); > /* Ring QP doorbell. */ > rte_write32(txq->msq.doorbell_qpn, txq->msq.db); > txq->elts_head = elts_head; > - txq->elts_comp += elts_comp; > + txq->elts_comp += i; > return i; > } For bytes_sent, reading these changes and assuming -O0 with the compiler attempting to convert the code without reordering/improving things, this replaces register variables used in a loop with memory operations on a large structure through a pointer (txq->stats update for every iteration instead of once at the end). Are you really sure this is more optimized that way? Although the compiler likely does it already with -O3, helping it avoid unnecessary memory writes is good in my opinion. OK for the removal of redundant elts_comp though. Although I'm pretty sure once again the compiler didn't wait for this patch to optimize it away. -- Adrien Mazarguil 6WIND