From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id EB4FD4CA5; Mon, 22 Oct 2018 10:31:51 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 25CB8CD4D2; Mon, 22 Oct 2018 08:31:51 +0000 (UTC) Received: from [10.36.112.57] (ovpn-112-57.ams2.redhat.com [10.36.112.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B3B0A6D76E; Mon, 22 Oct 2018 08:31:46 +0000 (UTC) To: Tiwei Bie Cc: dev@dpdk.org, zhihong.wang@intel.com, jfreimann@redhat.com, stable@dpdk.org References: <20181019140058.4981-1-maxime.coquelin@redhat.com> <20181022071510.GA30145@debian> From: Maxime Coquelin Message-ID: <2132245c-4a16-b858-ab7f-fd2e7f94134b@redhat.com> Date: Mon, 22 Oct 2018 10:31:44 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181022071510.GA30145@debian> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 22 Oct 2018 08:31:51 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] vhost: avoid memory barriers when no descriptors dequeued X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Oct 2018 08:31:52 -0000 On 10/22/2018 09:15 AM, Tiwei Bie wrote: > On Fri, Oct 19, 2018 at 04:00:58PM +0200, Maxime Coquelin wrote: >> In both split and packed dequeue paths, flush_shadow_used_ring >> and vhost_ring_call variants gets called even if not packets >> have been dequeued, and so no descriptors updates happened. >> >> It has an impact on CPU pipeline, as memory barriers are used >> in these functions. >> >> This patch don't call these functions if no descriptors have >> been dequeued. The performance gain with split ring when >> dequeue zero-copy is disabled should be null, but should be >> noticeable with packed ring or dequeue zero-copy enabled. >> >> Fixes: ae999ce49dcb ("vhost: add Tx support for packed ring") >> Fixes: 915cf9404225 ("vhost: use shadow used ring in dequeue path") >> Cc: stable@dpdk.org >> >> Signed-off-by: Maxime Coquelin >> --- >> lib/librte_vhost/virtio_net.c | 16 ++++++++++------ >> 1 file changed, 10 insertions(+), 6 deletions(-) > [...] >> >> - if (likely(dev->dequeue_zero_copy == 0)) { >> + if (likely(dev->dequeue_zero_copy == 0 && i != 0)) { >> do_data_copy_dequeue(vq); >> if (unlikely(i < count)) >> vq->shadow_used_idx = i; > > When i is 0, we may need to update vq->shadow_used_idx to 0, > e.g. when error happens after update_shadow_used_ring_split() > in the first iteration of the loop. I totally agree, it is broken when error happens. I will fix that in next revision. Thanks, Maxime > >> @@ -1475,8 +1477,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, >> } >> } >> >> - flush_shadow_used_ring_packed(dev, vq); >> - vhost_vring_call_packed(dev, vq); >> + if (likely(vq->shadow_used_idx)) { >> + flush_shadow_used_ring_packed(dev, vq); >> + vhost_vring_call_packed(dev, vq); >> + } >> } >> >> VHOST_LOG_DEBUG(VHOST_DATA, "(%d) %s\n", dev->vid, __func__); >> @@ -1550,7 +1554,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, >> } >> } >> >> - if (likely(dev->dequeue_zero_copy == 0)) { >> + if (likely(dev->dequeue_zero_copy == 0 && i != 0)) { > > Ditto > >> do_data_copy_dequeue(vq); >> if (unlikely(i < count)) >> vq->shadow_used_idx = i; >> -- >> 2.17.1 >>