From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 444AB379E for ; Tue, 22 Dec 2015 03:45:57 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga102.fm.intel.com with ESMTP; 21 Dec 2015 18:45:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,462,1444719600"; d="scan'208";a="712451921" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga003.jf.intel.com with ESMTP; 21 Dec 2015 18:45:56 -0800 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Dec 2015 18:45:55 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 21 Dec 2015 18:45:55 -0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.190]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.105]) with mapi id 14.03.0248.002; Tue, 22 Dec 2015 10:45:53 +0800 From: "Xie, Huawei" To: Yuanhan Liu Thread-Topic: [PATCH v2 2/6] vhost: introduce vhost_log_write Thread-Index: AQHRPAEzuIYE9Vl6N0Oddxfj/raOrQ== Date: Tue, 22 Dec 2015 02:45:52 +0000 Message-ID: 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> <20151222024058.GE18863@yliu-dev.sh.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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:45:58 -0000 On 12/22/2015 10:40 AM, Yuanhan Liu wrote:=0A= > On Mon, Dec 21, 2015 at 03:06:43PM +0000, Xie, Huawei wrote:=0A= >> On 12/17/2015 11:11 AM, Yuanhan Liu wrote:=0A= >>> Introduce vhost_log_write() helper function to log the dirty pages we= =0A= >>> touched. Page size is harded code to 4096 (VHOST_LOG_PAGE), and each=0A= >>> log is presented by 1 bit.=0A= >>>=0A= >>> Therefore, vhost_log_write() simply finds the right bit for related=0A= >>> page we are gonna change, and set it to 1. dev->log_base denotes the=0A= >>> start of the dirty page bitmap.=0A= >>>=0A= >>> Signed-off-by: Yuanhan Liu =0A= >>> Signed-off-by: Victor Kaplansky >> ---=0A= >>> lib/librte_vhost/rte_virtio_net.h | 29 +++++++++++++++++++++++++++++= =0A= >>> 1 file changed, 29 insertions(+)=0A= >>>=0A= >>> diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_v= irtio_net.h=0A= >>> index 8acee02..5726683 100644=0A= >>> --- a/lib/librte_vhost/rte_virtio_net.h=0A= >>> +++ b/lib/librte_vhost/rte_virtio_net.h=0A= >>> @@ -40,6 +40,7 @@=0A= >>> */=0A= >>> =0A= >>> #include =0A= >>> +#include =0A= >>> #include =0A= >>> #include =0A= >>> #include =0A= >>> @@ -59,6 +60,8 @@ struct rte_mbuf;=0A= >>> /* Backend value set by guest. */=0A= >>> #define VIRTIO_DEV_STOPPED -1=0A= >>> =0A= >>> +#define VHOST_LOG_PAGE 4096=0A= >>> +=0A= >>> =0A= >>> /* Enum for virtqueue management. */=0A= >>> enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};=0A= >>> @@ -205,6 +208,32 @@ gpa_to_vva(struct virtio_net *dev, uint64_t guest_= pa)=0A= >>> return vhost_va;=0A= >>> }=0A= >>> =0A= >>> +static inline void __attribute__((always_inline))=0A= >>> +vhost_log_page(uint8_t *log_base, uint64_t page)=0A= >>> +{=0A= >>> + log_base[page / 8] |=3D 1 << (page % 8);=0A= >>> +}=0A= >>> +=0A= >> Those logging functions are not supposed to be API. Could we move them= =0A= >> into an internal header file?=0A= > Agreed. I should have put them into vhost_rxtx.c=0A= >=0A= >>> +static inline void __attribute__((always_inline))=0A= >>> +vhost_log_write(struct virtio_net *dev, uint64_t addr, uint64_t len)= =0A= >>> +{=0A= >>> + uint64_t page;=0A= >>> +=0A= >> Before we log, we need memory barrier to make sure updates are in place.= =0A= >>> + if (likely(((dev->features & (1ULL << VHOST_F_LOG_ALL)) =3D=3D 0) ||= =0A= >>> + !dev->log_base || !len))=0A= >>> + return;=0A= > Put a memory barrier inside set_features()?=0A= >=0A= > I see no var dependence here, why putting a barrier then? We are=0A= > accessing and modifying same var, doesn't the cache MESI protocol=0A= > will get rid of your concerns?=0A= This fence isn't about feature var. It is to ensure that updates to the=0A= guest buffer are committed before the logging.=0A= For IA strong memory model, compiler barrier is enough. For other weak=0A= memory model, fence is required.=0A= >>> +=0A= >>> + if (unlikely(dev->log_size < ((addr + len - 1) / VHOST_LOG_PAGE / 8))= )=0A= >>> + return;=0A= >>> +=0A= >>> + page =3D addr / VHOST_LOG_PAGE;=0A= >>> + while (page * VHOST_LOG_PAGE < addr + len) {=0A= >> Let us have a page_end var to make the code simpler?=0A= > Could do that.=0A= >=0A= >=0A= >>> + vhost_log_page((uint8_t *)(uintptr_t)dev->log_base, page);=0A= >>> + page +=3D VHOST_LOG_PAGE;=0A= >> page +=3D 1?=0A= > Oops, right.=0A= >=0A= > --yliu=0A= >=0A= >>> + }=0A= >>> +}=0A= >>> +=0A= >>> +=0A= >>> /**=0A= >>> * Disable features in feature_mask. Returns 0 on success.=0A= >>> */=0A= =0A=