diff mbox series

vhost: avoid iotlb mempool allocation while IOMMU disabled

Message ID 20210129163909.35970-1-wanjunjie@bytedance.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series vhost: avoid iotlb mempool allocation while IOMMU disabled |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-testing warning Testing issues

Commit Message

Wan Junjie Jan. 29, 2021, 4:39 p.m. UTC 
  If vhost device's IOMMU feature is disabled, iotlb mempool allocation
is unnecessary.

Reported-by: Peng He <hepeng.0320@bytedance.com>
Signed-off-by: Wan Junjie <wanjunjie@bytedance.com>
Reviewed-by: Zhihong Wang <wangzhihong.wzh@bytedance.com>
---
 lib/librte_vhost/vhost.c      | 6 ++++--
 lib/librte_vhost/vhost_user.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

Comments

Chenbo Xia Feb. 1, 2021, 7:41 a.m. UTC | #1 
Hi Junjie,

> -----Original Message-----
> From: Wan Junjie <wanjunjie@bytedance.com>
> Sent: Saturday, January 30, 2021 12:39 AM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Wan Junjie <wanjunjie@bytedance.com>; Peng He
> <hepeng.0320@bytedance.com>; Zhihong Wang <wangzhihong.wzh@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@bytedance.com>
> Signed-off-by: Wan Junjie <wanjunjie@bytedance.com>
> Reviewed-by: Zhihong Wang <wangzhihong.wzh@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
diff mbox series

Patch

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);
 	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);
 	/* 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;