From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [PATCH v2 2/6] vhost: introduce vhost_log_write
Date: Tue, 22 Dec 2015 11:04:32 +0800
Message-ID: <20151222030432.GG18863@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>
 <C37D651A908B024F974696C65296B57B4C54DD5F@SHSMSX101.ccr.corp.intel.com>
 <20151222024058.GE18863@yliu-dev.sh.intel.com>
 <C37D651A908B024F974696C65296B57B4C54FE79@SHSMSX101.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "Michael S. Tsirkin" <mst@redhat.com>, "dev@dpdk.org" <dev@dpdk.org>,
 Victor Kaplansky <vkaplans@redhat.com>
To: "Xie, Huawei" <huawei.xie@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 2E6D85A0A
 for <dev@dpdk.org>; Tue, 22 Dec 2015 04:03:23 +0100 (CET)
Content-Disposition: inline
In-Reply-To: <C37D651A908B024F974696C65296B57B4C54FE79@SHSMSX101.ccr.corp.intel.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Tue, Dec 22, 2015 at 02:45:52AM +0000, Xie, Huawei wrote:
> >>> +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?
> This fence isn't about feature var. It is to ensure that updates to the
> guest buffer are committed before the logging.

Oh.., I was thinking you were talking about the "dev->features" field
concurrent access and modify you mentioned from V1.

> For IA strong memory model, compiler barrier is enough. For other weak
> memory model, fence is required.
> >>> +
> >>> +	if (unlikely(dev->log_size < ((addr + len - 1) / VHOST_LOG_PAGE / 8)))
> >>> +		return;

So that I should put a "rte_mb()" __here__?

	--yliu
> >>> +
> >>> +	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.
> >>>   */
>