From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 3A3C02BF5 for ; Sun, 18 Sep 2016 16:18:44 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga105.jf.intel.com with ESMTP; 18 Sep 2016 07:18:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,357,1470726000"; d="scan'208";a="1058479418" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by fmsmga002.fm.intel.com with ESMTP; 18 Sep 2016 07:18:41 -0700 Date: Sun, 18 Sep 2016 22:19:14 +0800 From: Yuanhan Liu To: Zhihong Wang Cc: dev@dpdk.org, maxime.coquelin@redhat.com, thomas.monjalon@6wind.com Message-ID: <20160918141914.GI23158@yliu-dev.sh.intel.com> References: <1473392368-84903-1-git-send-email-zhihong.wang@intel.com> <1473392368-84903-3-git-send-email-zhihong.wang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1473392368-84903-3-git-send-email-zhihong.wang@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v5 2/6] vhost: rewrite enqueue 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: Sun, 18 Sep 2016 14:18:44 -0000 On Thu, Sep 08, 2016 at 11:39:24PM -0400, Zhihong Wang wrote: > This patch implements the vhost logic from scratch into a single function > designed for high performance and better maintainability. As always, your commit log just states what have been done, but doesn't tell why such changes have been made. For example, you said "it's designed for high performance", then you'd better explain why your version would introduce high performance. You need a reason, as well as some numbers (percent change) to prove it: it's not that right to keep the numbers inside: I'm sure people outside intel are also willing and happy to know those numbers. For this patch, I think it's more about the maintainability improvement but not performance: the performance tunning patches are done later after all. Another example is, in patch 6, you said "It reduces CPU pipeline stall cycles significantly", but you didn't say why there is pipeline stall before and why your patch reduces it. All those are important things that deserves some explanation. So, I'd ask you to re-visit all your patches in this set, to think what you could add to make the commit better and more informative. Besides that, I think this patchset looks fine to me. I may just need another time to look it more carefully, then I think I can merge (v6). BTW, thanks for the great work! --yliu > This is the baseline version of the new code, more optimization will be > added in the following patches in this patch set. > > Signed-off-by: Zhihong Wang > ---