Re: [dpdk-dev] [PATCH] vhost: avoid iotlb mempool allocation while IOMMU disabled
From: Xia, Chenbo <hidden>
Date: 2021-02-01 07:41:40
Hi Junjie,
quoted hunk ↗ jump to hunk
-----Original Message----- From: Wan Junjie <redacted> Sent: Saturday, January 30, 2021 12:39 AM To: maxime.coquelin@redhat.com; Xia, Chenbo <redacted> Cc: dev@dpdk.org; Wan Junjie <redacted>; Peng He [off-list ref]; Zhihong Wang [off-list ref] 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 <redacted> Signed-off-by: Wan Junjie <redacted> Reviewed-by: Zhihong Wang <redacted> --- 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.
quoted hunk ↗ jump to hunk
rte_free(vq); }@@ -556,7 +557,8 @@ init_vring_queue(struct virtio_net *dev, uint32_tvring_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
quoted hunk ↗ jump to hunk
/* 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