From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [PATCH v5 5/6] vhost: batch update used ring
Date: Sun, 18 Sep 2016 10:55:42 +0800
Message-ID: <20160918025542.GC23158@yliu-dev.sh.intel.com>
References: <1471319402-112998-1-git-send-email-zhihong.wang@intel.com>
 <1473392368-84903-1-git-send-email-zhihong.wang@intel.com>
 <1473392368-84903-6-git-send-email-zhihong.wang@intel.com>
 <473ef253-86bf-9a7a-d028-21c27690a421@redhat.com>
 <8F6C2BD409508844A0EFC19955BE09414E70FB6A@SHSMSX103.ccr.corp.intel.com>
 <52dba6dc-ca0d-1aeb-cf18-89470450a183@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "Wang, Zhihong" <zhihong.wang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
 by dpdk.org (Postfix) with ESMTP id E475D2BB5
 for <dev@dpdk.org>; Sun, 18 Sep 2016 04:55:16 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <52dba6dc-ca0d-1aeb-cf18-89470450a183@redhat.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 Thu, Sep 15, 2016 at 06:38:06PM +0200, Maxime Coquelin wrote:
> >>>+static inline void __attribute__((always_inline))
> >>>+flush_used_ring(struct virtio_net *dev, struct vhost_virtqueue *vq,
> >>>+		uint32_t used_idx_start)
> >>>+{
> >>>+	if (used_idx_start + vq->shadow_used_idx < vq->size) {
> >>>+		rte_memcpy(&vq->used->ring[used_idx_start],
> >>>+				&vq->shadow_used_ring[0],
> >>>+				vq->shadow_used_idx *
> >>>+				sizeof(struct vring_used_elem));
> >>>+		vhost_log_used_vring(dev, vq,
> >>>+				offsetof(struct vring_used,
> >>>+					ring[used_idx_start]),
> >>>+				vq->shadow_used_idx *
> >>>+				sizeof(struct vring_used_elem));
> >>>+	} else {
> >>>+		uint32_t part_1 = vq->size - used_idx_start;
> >>>+		uint32_t part_2 = vq->shadow_used_idx - part_1;
> >>>+
> >>>+		rte_memcpy(&vq->used->ring[used_idx_start],
> >>>+				&vq->shadow_used_ring[0],
> >>>+				part_1 *
> >>>+				sizeof(struct vring_used_elem));
> >>>+		vhost_log_used_vring(dev, vq,
> >>>+				offsetof(struct vring_used,
> >>>+					ring[used_idx_start]),
> >>>+				part_1 *
> >>>+				sizeof(struct vring_used_elem));
> >>>+		rte_memcpy(&vq->used->ring[0],
> >>>+				&vq->shadow_used_ring[part_1],
> >>>+				part_2 *
> >>>+				sizeof(struct vring_used_elem));
> >>>+		vhost_log_used_vring(dev, vq,
> >>>+				offsetof(struct vring_used,
> >>>+					ring[0]),
> >>>+				part_2 *
> >>>+				sizeof(struct vring_used_elem));
> >>>+	}
> >>> }
> >>Is expanding the code done for performance purpose?
> >
> >Hi Maxime,
> >
> >Yes theoretically this has the least branch number.
> >And I think the logic is simpler this way.
> Ok, in that case, maybe you could create a function to
> do the rte_memcpy and the vhost_log_used on a given range.

Agreed, that will be better; it could avoid repeating similar code
block 3 times.

> I don't have a strong opinion on this, if Yuanhan is fine
> with current code, that's ok for me.

>>From what I know, that's kind of DPDK prefered way, to expand code
when necessary. For example, 9ec201f5d6e7 ("mbuf: provide bulk
allocation").

So I'm fine with it.

	--yliu