Re: [PATCH v3 2/3] vhost: Remove vhost_backend_can_merge() callback

2023-05-23 Thread Peter Xu
On Wed, May 03, 2023 at 07:21:20PM +0200, David Hildenbrand wrote:
> Checking whether the memory regions are equal is sufficient: if they are
> equal, then most certainly the contained fd is equal.

Looks reasonable to me.

I double checked the src of the change and there's no bug report attached
either.  Maybe just a double safety belt, but definitely Michael will know
the best.

https://lore.kernel.org/qemu-devel/1456067090-18187-1-git-send-email-...@redhat.com/

> 
> The whole vhost-user memslot handling is suboptimal and overly
> complicated. We shouldn't have to lookup a RAM memory regions we got
> notified about in vhost_user_get_mr_data() using a host pointer. But that
> requires a bigger rework -- especially an alternative vhost_set_mem_table()
> backend call that simply consumes MemoryRegionSections.
> 
> For now, let's just drop vhost_backend_can_merge().
> 
> Acked-by: Stefan Hajnoczi 
> Reviewed-by: Igor Mammedov 
> Acked-by: Igor Mammedov 
> Signed-off-by: David Hildenbrand 

Reviewed-by: Peter Xu 

-- 
Peter Xu




[PATCH v3 2/3] vhost: Remove vhost_backend_can_merge() callback

2023-05-03 Thread David Hildenbrand
Checking whether the memory regions are equal is sufficient: if they are
equal, then most certainly the contained fd is equal.

The whole vhost-user memslot handling is suboptimal and overly
complicated. We shouldn't have to lookup a RAM memory regions we got
notified about in vhost_user_get_mr_data() using a host pointer. But that
requires a bigger rework -- especially an alternative vhost_set_mem_table()
backend call that simply consumes MemoryRegionSections.

For now, let's just drop vhost_backend_can_merge().

Acked-by: Stefan Hajnoczi 
Reviewed-by: Igor Mammedov 
Acked-by: Igor Mammedov 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/vhost-user.c| 14 --
 hw/virtio/vhost-vdpa.c|  1 -
 hw/virtio/vhost.c |  6 +-
 include/hw/virtio/vhost-backend.h |  4 
 4 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 0c3e2702b1..831375a967 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2195,19 +2195,6 @@ static int vhost_user_migration_done(struct vhost_dev 
*dev, char* mac_addr)
 return -ENOTSUP;
 }
 
-static bool vhost_user_can_merge(struct vhost_dev *dev,
- uint64_t start1, uint64_t size1,
- uint64_t start2, uint64_t size2)
-{
-ram_addr_t offset;
-int mfd, rfd;
-
-(void)vhost_user_get_mr_data(start1, , );
-(void)vhost_user_get_mr_data(start2, , );
-
-return mfd == rfd;
-}
-
 static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
 {
 VhostUserMsg msg;
@@ -2704,7 +2691,6 @@ const VhostOps user_ops = {
 .vhost_set_vring_enable = vhost_user_set_vring_enable,
 .vhost_requires_shm_log = vhost_user_requires_shm_log,
 .vhost_migration_done = vhost_user_migration_done,
-.vhost_backend_can_merge = vhost_user_can_merge,
 .vhost_net_set_mtu = vhost_user_net_set_mtu,
 .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
 .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index bc6bad23d5..38d98528e7 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1355,7 +1355,6 @@ const VhostOps vdpa_ops = {
 .vhost_set_config = vhost_vdpa_set_config,
 .vhost_requires_shm_log = NULL,
 .vhost_migration_done = NULL,
-.vhost_backend_can_merge = NULL,
 .vhost_net_set_mtu = NULL,
 .vhost_set_iotlb_callback = NULL,
 .vhost_send_device_iotlb_msg = NULL,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 4fe08c809f..6148892798 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -729,11 +729,7 @@ static void vhost_region_add_section(struct vhost_dev *dev,
 size_t offset = mrs_gpa - prev_gpa_start;
 
 if (prev_host_start + offset == mrs_host &&
-section->mr == prev_sec->mr &&
-(!dev->vhost_ops->vhost_backend_can_merge ||
- dev->vhost_ops->vhost_backend_can_merge(dev,
-mrs_host, mrs_size,
-prev_host_start, prev_size))) {
+section->mr == prev_sec->mr) {
 uint64_t max_end = MAX(prev_host_end, mrs_host + mrs_size);
 need_add = false;
 prev_sec->offset_within_address_space =
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index 2349a4a7d2..f3ba7b676b 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -86,9 +86,6 @@ typedef int (*vhost_set_vring_enable_op)(struct vhost_dev 
*dev,
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
 typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
char *mac_addr);
-typedef bool (*vhost_backend_can_merge_op)(struct vhost_dev *dev,
-   uint64_t start1, uint64_t size1,
-   uint64_t start2, uint64_t size2);
 typedef int (*vhost_vsock_set_guest_cid_op)(struct vhost_dev *dev,
 uint64_t guest_cid);
 typedef int (*vhost_vsock_set_running_op)(struct vhost_dev *dev, int start);
@@ -163,7 +160,6 @@ typedef struct VhostOps {
 vhost_set_vring_enable_op vhost_set_vring_enable;
 vhost_requires_shm_log_op vhost_requires_shm_log;
 vhost_migration_done_op vhost_migration_done;
-vhost_backend_can_merge_op vhost_backend_can_merge;
 vhost_vsock_set_guest_cid_op vhost_vsock_set_guest_cid;
 vhost_vsock_set_running_op vhost_vsock_set_running;
 vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
-- 
2.40.0