qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 3/3] vdpa: commit all host notifier MRs in a single MR tra


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v3 3/3] vdpa: commit all host notifier MRs in a single MR transaction
Date: Wed, 28 Dec 2022 14:14:16 +0100
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.6.1

On 27/12/22 18:56, Michael S. Tsirkin wrote:
On Tue, Dec 27, 2022 at 05:51:47PM +0100, Philippe Mathieu-Daudé wrote:
On 27/12/22 08:20, Longpeng(Mike) wrote:
From: Longpeng <longpeng2@huawei.com>

This allows the vhost-vdpa device to batch the setup of all its MRs of
host notifiers.

This significantly reduces the device starting time, e.g. the time spend
on setup the host notifier MRs reduce from 423ms to 32ms for a VM with
64 vCPUs and 3 vhost-vDPA generic devices (vdpa_sim_blk, 64vq per device).

Signed-off-by: Longpeng <longpeng2@huawei.com>
---
   hw/virtio/vhost-vdpa.c | 25 +++++++++++++++++++------
   1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index fd0c33b0e1..870265188a 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -512,9 +512,18 @@ static void vhost_vdpa_host_notifiers_uninit(struct 
vhost_dev *dev, int n)
   {
       int i;
+    /*
+     * Pack all the changes to the memory regions in a single
+     * transaction to avoid a few updating of the address space
+     * topology.
+     */
+    memory_region_transaction_begin();
+
       for (i = dev->vq_index; i < dev->vq_index + n; i++) {
           vhost_vdpa_host_notifier_uninit(dev, i);
       }
+
+    memory_region_transaction_commit();
   }

Instead of optimizing one frontend, I wonder if we shouldn't expose
a 'bool memory_region_transaction_in_progress()' helper in memory.c,
and in virtio_queue_set_host_notifier_mr() backend, assert we are
within a transaction. That way we'd optimize all virtio frontends.


If we are doing something like this, I'd rather pass around
a "transaction" structure so this can be checked statically.

Ah, clever.

Looks like something that can be done on top though.

Sure.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>





reply via email to

[Prev in Thread] Current Thread [Next in Thread]