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 3E777370 for ; Tue, 22 Dec 2015 03:39:49 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 21 Dec 2015 18:39:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,462,1444719600"; d="scan'208";a="621945565" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by FMSMGA003.fm.intel.com with ESMTP; 21 Dec 2015 18:39:46 -0800 Date: Tue, 22 Dec 2015 10:40:58 +0800 From: Yuanhan Liu To: "Xie, Huawei" Message-ID: <20151222024058.GE18863@yliu-dev.sh.intel.com> References: <1449027793-30975-1-git-send-email-yuanhan.liu@linux.intel.com> <1450321921-27799-1-git-send-email-yuanhan.liu@linux.intel.com> <1450321921-27799-3-git-send-email-yuanhan.liu@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Cc: "Michael S. Tsirkin" , "dev@dpdk.org" , Victor Kaplansky Subject: Re: [dpdk-dev] [PATCH v2 2/6] vhost: introduce vhost_log_write 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, 22 Dec 2015 02:39:49 -0000 On Mon, Dec 21, 2015 at 03:06:43PM +0000, Xie, Huawei wrote: > On 12/17/2015 11:11 AM, Yuanhan Liu wrote: > > Introduce vhost_log_write() helper function to log the dirty pages we > > touched. Page size is harded code to 4096 (VHOST_LOG_PAGE), and each > > log is presented by 1 bit. > > > > Therefore, vhost_log_write() simply finds the right bit for related > > page we are gonna change, and set it to 1. dev->log_base denotes the > > start of the dirty page bitmap. > > > > Signed-off-by: Yuanhan Liu > > Signed-off-by: Victor Kaplansky > --- > > lib/librte_vhost/rte_virtio_net.h | 29 +++++++++++++++++++++++++++++ > > 1 file changed, 29 insertions(+) > > > > diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h > > index 8acee02..5726683 100644 > > --- a/lib/librte_vhost/rte_virtio_net.h > > +++ b/lib/librte_vhost/rte_virtio_net.h > > @@ -40,6 +40,7 @@ > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -59,6 +60,8 @@ struct rte_mbuf; > > /* Backend value set by guest. */ > > #define VIRTIO_DEV_STOPPED -1 > > > > +#define VHOST_LOG_PAGE 4096 > > + > > > > /* Enum for virtqueue management. */ > > enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM}; > > @@ -205,6 +208,32 @@ gpa_to_vva(struct virtio_net *dev, uint64_t guest_pa) > > return vhost_va; > > } > > > > +static inline void __attribute__((always_inline)) > > +vhost_log_page(uint8_t *log_base, uint64_t page) > > +{ > > + log_base[page / 8] |= 1 << (page % 8); > > +} > > + > Those logging functions are not supposed to be API. Could we move them > into an internal header file? Agreed. I should have put them into vhost_rxtx.c > > +static inline void __attribute__((always_inline)) > > +vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len) > > +{ > > + uint64_t page; > > + > Before we log, we need memory barrier to make sure updates are in place. > > + if (likely(((dev->features & (1ULL << VHOST_F_LOG_ALL)) == 0) || > > + !dev->log_base || !len)) > > + return; Put a memory barrier inside set_features()? I see no var dependence here, why putting a barrier then? We are accessing and modifying same var, doesn't the cache MESI protocol will get rid of your concerns? > > + > > + if (unlikely(dev->log_size < ((addr + len - 1) / VHOST_LOG_PAGE / 8))) > > + return; > > + > > + page = addr / VHOST_LOG_PAGE; > > + while (page * VHOST_LOG_PAGE < addr + len) { > Let us have a page_end var to make the code simpler? Could do that. > > + vhost_log_page((uint8_t *)(uintptr_t)dev->log_base, page); > > + page += VHOST_LOG_PAGE; > page += 1? Oops, right. --yliu > > + } > > +} > > + > > + > > /** > > * Disable features in feature_mask. Returns 0 on success. > > */ >