From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <i.maximets@samsung.com>
Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com
 [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id 9FE621B122
 for <dev@dpdk.org>; Wed,  5 Dec 2018 17:01:26 +0100 (CET)
Received: from eucas1p1.samsung.com (unknown [182.198.249.206])
 by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id
 20181205160125euoutp018f1d0cbb20184d4befc5fcda8084dd6d~tey400-341482014820euoutp01A
 for <dev@dpdk.org>; Wed,  5 Dec 2018 16:01:25 +0000 (GMT)
DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com
 20181205160125euoutp018f1d0cbb20184d4befc5fcda8084dd6d~tey400-341482014820euoutp01A
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com;
 s=mail20170921; t=1544025685;
 bh=phY+bLddNhRE7/O/Knt5EqJ9QAX0COx+1x+Ju/0t7Uw=;
 h=Subject:To:From:Cc:Date:In-Reply-To:References:From;
 b=tXp521U57JolSU7cX20IXdFGt9byIxuiAamPnmXo3yt2kIPdq1GljdQE0hsjxg/t7
 IyJ2nWMiz05NtDTB95D0Z4he4ZiYnNodhd0pP8RWp1OdU3IShfQ0p79eDupLnShu2b
 /ytBAdWM64/CVb6RdVPUGJI+SKOXpqJDHdGSEB0w=
Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by
 eucas1p2.samsung.com (KnoxPortal) with ESMTP id
 20181205160125eucas1p224569f022813cd6d7aad63a701b9a649~tey4YplpH2605126051eucas1p23;
 Wed,  5 Dec 2018 16:01:25 +0000 (GMT)
Received: from eucas1p2.samsung.com ( [182.198.249.207]) by
 eusmges3new.samsung.com (EUCPMTA) with SMTP id 7B.37.04806.456F70C5; Wed,  5
 Dec 2018 16:01:24 +0000 (GMT)
Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by
 eucas1p1.samsung.com (KnoxPortal) with ESMTPA id
 20181205160124eucas1p1470e3dc9afe8e59ceab54a58140cf400~tey3pvSfk2135821358eucas1p11;
 Wed,  5 Dec 2018 16:01:24 +0000 (GMT)
Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by
 eusmtrp2.samsung.com (KnoxPortal) with ESMTP id
 20181205160124eusmtrp25047b842c04f7c870fb1ecec5680efce~tey3bVGcI1036310363eusmtrp2f;
 Wed,  5 Dec 2018 16:01:24 +0000 (GMT)
X-AuditID: cbfec7f5-367ff700000012c6-e8-5c07f654dee0
Received: from eusmtip2.samsung.com ( [203.254.199.222]) by
 eusmgms1.samsung.com (EUCPMTA) with SMTP id 0E.36.04284.456F70C5; Wed,  5
 Dec 2018 16:01:24 +0000 (GMT)
Received: from [106.109.129.180] (unknown [106.109.129.180]) by
 eusmtip2.samsung.com (KnoxPortal) with ESMTPA id
 20181205160123eusmtip262ea23ea6a661e5668d460ed31b58835~tey246bOc0683506835eusmtip2v;
 Wed,  5 Dec 2018 16:01:23 +0000 (GMT)
To: Maxime Coquelin <maxime.coquelin@redhat.com>, dev@dpdk.org,
 tiwei.bie@intel.com, zhihong.wang@intel.com, jfreimann@redhat.com
From: Ilya Maximets <i.maximets@samsung.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Message-ID: <7fbcfcea-3c81-d5d1-86bf-8fe8e63d4468@samsung.com>
Date: Wed, 5 Dec 2018 19:01:23 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181128094700.14598-1-maxime.coquelin@redhat.com>
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrMKsWRmVeSWpSXmKPExsWy7djP87oh39hjDJ7/M7d492k7k8WV9p/s
 FufWLGWxONa5h8Xi/69XrBZbG/4zWWy+OInJgd3j14KlrB6L97xk8ni/7yqbR9+WVYwBLFFc
 NimpOZllqUX6dglcGUtf+BRclqy48XYdawPjRZEuRk4OCQETiX/n57B0MXJxCAmsYJTY8+cz
 E4TzhVFi0vezUM5nRomVe6axwrQ0NSyAalnOKLHtzyywhJDAR0aJ25P4uhg5OIQF/CUufg0G
 CYsINDNK3NzAAmKzCehInFp9hBGkhFlAU+L+NlWQMK+AncSvtzeYQGwWARWJRb+ngZWLCkRI
 dNxfzQZRIyhxcuYTsDingIPE+f1djCA2s4C4RNOXlawQtrzE9rdzmEFOkxBYxy7x7eUONoib
 XSS+L37JDmELS7w6vgXKlpH4v3M+E4RdL3G/5SUjRHMHo8T0Q/+gEvYSW16fY4c5ev0ufYiw
 o8Sbq/tYQMISAnwSN94KQtzAJzFp23RmiDCvREebEES1isTvg8uZIWwpiZvvPrNPYFSaheSz
 WUi+mYXkm1kIexcwsqxiFE8tLc5NTy02zkst1ytOzC0uzUvXS87P3cQITDqn/x3/uoNx35+k
 Q4wCHIxKPLwvHrDHCLEmlhVX5h5ilOBgVhLhXWEDFOJNSaysSi3Kjy8qzUktPsQozcGiJM5b
 zfAgWkggPbEkNTs1tSC1CCbLxMEp1cB4pW+aw5+1ry/9emTpfeaVrmgqz6bwrwJFlZLdoW0N
 OSs4D3N1PFnZkZ2fuE2xhsd2n1y0G0/bhynxVnEZq6adZDD6Om3JBImu+2bHBdJnzOte9ev0
 3jWVb+5krt3Z1FSn1n9tdSaXxHqFyjPzTbVnCssyNZp2isVU8AkyNOwsNCk89X3hy4VKLMUZ
 iYZazEXFiQCTGTqONgMAAA==
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrBIsWRmVeSWpSXmKPExsVy+t/xe7oh39hjDL4tZbd492k7k8WV9p/s
 FufWLGWxONa5h8Xi/69XrBZbG/4zWWy+OInJgd3j14KlrB6L97xk8ni/7yqbR9+WVYwBLFF6
 NkX5pSWpChn5xSW2StGGFkZ6hpYWekYmlnqGxuaxVkamSvp2NimpOZllqUX6dgl6GUtf+BRc
 lqy48XYdawPjRZEuRk4OCQETiaaGBSxdjFwcQgJLGSXeXzjADJGQkvjx6wIrhC0s8edaFxtE
 0XtGiV9Pz7CAJIQFfCV6u5eAdYsINDNKXDs2ixEkISRgL3H7XANYN5uAjsSp1UeA4hwczAKa
 Eve3qYKEeQXsJH69vcEEYrMIqEgs+j0NbKaoQITE2ZfrGCFqBCVOznwCFucUcJA4v78LLM4s
 oC7xZ94lZghbXKLpy0pWCFteYvvbOcwTGIVmIWmfhaRlFpKWWUhaFjCyrGIUSS0tzk3PLTbU
 K07MLS7NS9dLzs/dxAiMtW3Hfm7ewXhpY/AhRgEORiUe3hcP2GOEWBPLiitzDzFKcDArifCu
 sAEK8aYkVlalFuXHF5XmpBYfYjQFem4is5Rocj4wDeSVxBuaGppbWBqaG5sbm1koifOeN6iM
 EhJITyxJzU5NLUgtgulj4uCUamCMjkmf2F2cvHviltprN/syX2f2bGbuNt70+6zFzr/PZGfk
 8vsbLnpu8IPB4b5tm9onJga26Yt0W18yLDj0NWlH7QLR2zoyxb4Fv7XaJAR/xNpeP/PUY/fK
 w0/+PJ28tl+uiPe8qrLEySaNhZY2gQybpXXfHSh/xb/ikeLnDK0IlXW6D34fNM9XYinOSDTU
 Yi4qTgQADBjXLcsCAAA=
X-CMS-MailID: 20181205160124eucas1p1470e3dc9afe8e59ceab54a58140cf400
X-Msg-Generator: CA
Content-Type: text/plain; charset="utf-8"
X-RootMTR: 20181205160124eucas1p1470e3dc9afe8e59ceab54a58140cf400
X-EPHeader: CA
CMS-TYPE: 201P
X-CMS-RootMailID: 20181205160124eucas1p1470e3dc9afe8e59ceab54a58140cf400
References: <20181128094700.14598-1-maxime.coquelin@redhat.com>
 <CGME20181205160124eucas1p1470e3dc9afe8e59ceab54a58140cf400@eucas1p1.samsung.com>
Subject: Re: [dpdk-dev] vhost: batch used descriptors chains write-back with
 packed ring
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 05 Dec 2018 16:01:26 -0000

On 28.11.2018 12:47, Maxime Coquelin wrote:
> Instead of writing back descriptors chains in order, let's
> write the first chain flags last in order to improve batching.

I'm not sure if this fully compliant with virtio spec.
It says that 'each side (driver and device) are only required to poll
(or test) a single location in memory', but it does not forbid to
test other descriptors. So, if the driver will try to check not only
'the next device descriptor after the one they processed previously,
in circular order' but a few descriptors ahead, it could read an
inconsistent memory because there are no more write barriers between
updates for flags and id/len for them.

What do you think ?

> 
> With Kernel's pktgen benchmark, ~3% performance gain is measured.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Tested-by: Jens Freimann <jfreimann@redhat.com>
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>
> ---
>  lib/librte_vhost/virtio_net.c | 37 ++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 5e1a1a727..f54642c2d 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -135,19 +135,10 @@ flush_shadow_used_ring_packed(struct virtio_net *dev,
>  			struct vhost_virtqueue *vq)
>  {
>  	int i;
> -	uint16_t used_idx = vq->last_used_idx;
> +	uint16_t head_flags, head_idx = vq->last_used_idx;
>  
> -	/* Split loop in two to save memory barriers */
> -	for (i = 0; i < vq->shadow_used_idx; i++) {
> -		vq->desc_packed[used_idx].id = vq->shadow_used_packed[i].id;
> -		vq->desc_packed[used_idx].len = vq->shadow_used_packed[i].len;
> -
> -		used_idx += vq->shadow_used_packed[i].count;
> -		if (used_idx >= vq->size)
> -			used_idx -= vq->size;
> -	}
> -
> -	rte_smp_wmb();
> +	if (unlikely(vq->shadow_used_idx == 0))
> +		return;
>  
>  	for (i = 0; i < vq->shadow_used_idx; i++) {
>  		uint16_t flags;
> @@ -165,12 +156,22 @@ flush_shadow_used_ring_packed(struct virtio_net *dev,
>  			flags &= ~VRING_DESC_F_AVAIL;
>  		}
>  
> -		vq->desc_packed[vq->last_used_idx].flags = flags;
> +		vq->desc_packed[vq->last_used_idx].id =
> +			vq->shadow_used_packed[i].id;
> +		vq->desc_packed[vq->last_used_idx].len =
> +			vq->shadow_used_packed[i].len;
> +
> +		if (i > 0) {
> +			vq->desc_packed[vq->last_used_idx].flags = flags;
>  
> -		vhost_log_cache_used_vring(dev, vq,
> +			vhost_log_cache_used_vring(dev, vq,
>  					vq->last_used_idx *
>  					sizeof(struct vring_packed_desc),
>  					sizeof(struct vring_packed_desc));
> +		} else {
> +			head_idx = vq->last_used_idx;
> +			head_flags = flags;
> +		}
>  
>  		vq->last_used_idx += vq->shadow_used_packed[i].count;
>  		if (vq->last_used_idx >= vq->size) {
> @@ -180,7 +181,15 @@ flush_shadow_used_ring_packed(struct virtio_net *dev,
>  	}
>  
>  	rte_smp_wmb();
> +
> +	vq->desc_packed[head_idx].flags = head_flags;
>  	vq->shadow_used_idx = 0;
> +
> +	vhost_log_cache_used_vring(dev, vq,
> +				head_idx *
> +				sizeof(struct vring_packed_desc),
> +				sizeof(struct vring_packed_desc));
> +
>  	vhost_log_cache_sync(dev, vq);
>  }
>  
>