From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <zhihong.wang@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
by dpdk.org (Postfix) with ESMTP id 540F62BA9
for <dev@dpdk.org>; Sun, 18 Sep 2016 04:57:45 +0200 (CEST)
Received: from orsmga004.jf.intel.com ([10.7.209.38])
by fmsmga105.fm.intel.com with ESMTP; 17 Sep 2016 19:57:44 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.30,354,1470726000"; d="scan'208";a="10759566"
Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205])
by orsmga004.jf.intel.com with ESMTP; 17 Sep 2016 19:57:44 -0700
Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by
fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS)
id 14.3.248.2; Sat, 17 Sep 2016 19:57:43 -0700
Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by
FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS)
id 14.3.248.2; Sat, 17 Sep 2016 19:57:43 -0700
Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.209]) by
SHSMSX101.ccr.corp.intel.com ([169.254.1.91]) with mapi id 14.03.0248.002;
Sun, 18 Sep 2016 10:57:41 +0800
From: "Wang, Zhihong" <zhihong.wang@intel.com>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>, Maxime Coquelin
<maxime.coquelin@redhat.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "thomas.monjalon@6wind.com"
<thomas.monjalon@6wind.com>
Thread-Topic: [PATCH v5 5/6] vhost: batch update used ring
Thread-Index: AQHSCoeQzsSjk8GNGUa+xR0CoUAoi6B1fkEAgAM0D/CAAZGWAIAD0TgAgACGaaA=
Date: Sun, 18 Sep 2016 02:57:40 +0000
Message-ID: <8F6C2BD409508844A0EFC19955BE09414E79A025@SHSMSX103.ccr.corp.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>
<20160918025542.GC23158@yliu-dev.sh.intel.com>
In-Reply-To: <20160918025542.GC23158@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
Subject: Re: [dpdk-dev] [PATCH v5 5/6] vhost: batch update used ring
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
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>
X-List-Received-Date: Sun, 18 Sep 2016 02:57:45 -0000
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Sunday, September 18, 2016 10:56 AM
> To: Maxime Coquelin <maxime.coquelin@redhat.com>
> Cc: Wang, Zhihong <zhihong.wang@intel.com>; dev@dpdk.org;
> thomas.monjalon@6wind.com
> Subject: Re: [PATCH v5 5/6] vhost: batch update used ring
>=20
> 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 =3D vq->size - used_idx_start;
> > >>>+ uint32_t part_2 =3D 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.
>=20
> Agreed, that will be better; it could avoid repeating similar code
> block 3 times.
Okay. Thanks for the suggestion, Maxime and Yuanhan.
>=20
> > I don't have a strong opinion on this, if Yuanhan is fine
> > with current code, that's ok for me.
>=20
> From what I know, that's kind of DPDK prefered way, to expand code
> when necessary. For example, 9ec201f5d6e7 ("mbuf: provide bulk
> allocation").
>=20
> So I'm fine with it.
>=20
> --yliu