[dpdk-dev] [PATCH] vhost: avoid memory barriers when no descriptors dequeued
Maxime Coquelin
maxime.coquelin at redhat.com
Mon Oct 22 10:31:44 CEST 2018
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 at dpdk.org
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
>> ---
>> 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
>>
More information about the dev
mailing list