> -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan.liu at linux.intel.com] > Sent: Sunday, September 18, 2016 10:19 PM > To: Wang, Zhihong <zhihong.wang at intel.com> > Cc: dev at dpdk.org; maxime.coquelin at redhat.com; > thomas.monjalon at 6wind.com > Subject: Re: [PATCH v5 2/6] vhost: rewrite enqueue > > 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.
Okay. I'll add more detailed commit log. > > 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 <zhihong.wang at intel.com> > > ---
