[dpdk-dev] [PATCH] vhost: avoid iotlb mempool allocation while IOMMU disabled

Xia, Chenbo chenbo.xia at intel.com
Mon Feb 1 08:41:30 CET 2021


Hi Junjie,

> -----Original Message-----
> From: Wan Junjie <wanjunjie at bytedance.com>
> Sent: Saturday, January 30, 2021 12:39 AM
> To: maxime.coquelin at redhat.com; Xia, Chenbo <chenbo.xia at intel.com>
> Cc: dev at dpdk.org; Wan Junjie <wanjunjie at bytedance.com>; Peng He
> <hepeng.0320 at bytedance.com>; Zhihong Wang <wangzhihong.wzh at bytedance.com>
> Subject: [PATCH] vhost: avoid iotlb mempool allocation while IOMMU disabled
> 
> If vhost device's IOMMU feature is disabled, iotlb mempool allocation
> is unnecessary.
> 
> Reported-by: Peng He <hepeng.0320 at bytedance.com>
> Signed-off-by: Wan Junjie <wanjunjie at bytedance.com>
> Reviewed-by: Zhihong Wang <wangzhihong.wzh at bytedance.com>
> ---
>  lib/librte_vhost/vhost.c      | 6 ++++--
>  lib/librte_vhost/vhost_user.c | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index efb136edd..00c5040e2 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -352,7 +352,8 @@ free_vq(struct virtio_net *dev, struct vhost_virtqueue *vq)
>  		vhost_free_async_mem(vq);
>  	}
>  	rte_free(vq->batch_copy_elems);
> -	rte_mempool_free(vq->iotlb_pool);
> +	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
> +		rte_mempool_free(vq->iotlb_pool);

We could make it simpler, check vq->iotlb_pool is not NULL and then free the
mempool, which ignores the problem that features may not be set yet (See below
comment). Although we could also keep the original because rte_mempool_free
will do the check, let's do it outside to avoid useless func call.

>  	rte_free(vq);
>  }
> 
> @@ -556,7 +557,8 @@ init_vring_queue(struct virtio_net *dev, uint32_t
> vring_idx)
>  	vq->callfd = VIRTIO_UNINITIALIZED_EVENTFD;
>  	vq->notif_enable = VIRTIO_UNINITIALIZED_NOTIF;
> 
> -	vhost_user_iotlb_init(dev, vring_idx);
> +	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
> +		vhost_user_iotlb_init(dev, vring_idx);

Note that when this func is called, dev->features are not set. So it's meaningless
to do this. So, we should do it after the features are set.

Thanks,
Chenbo

>  	/* Backends are set to -1 indicating an inactive device. */
>  	vq->backend = -1;
>  }
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index a60bb945a..d415607d7 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -575,7 +575,7 @@ numa_realloc(struct virtio_net *dev, int index)
>  	dev->virtqueue[index] = vq;
>  	vhost_devices[dev->vid] = dev;
> 
> -	if (old_vq != vq)
> +	if (old_vq != vq && (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)))
>  		vhost_user_iotlb_init(dev, index);
> 
>  	return dev;
> --
> 2.29.2



More information about the dev mailing list