Re: [PATCH v3 2/3] vhost: configure all host notifiers in a single MR transaction

[Philippe Mathieu-Daudé]

On 27/12/22 08:20, Longpeng(Mike) wrote:
> From: Longpeng <longpeng2@huawei.com>
>
> This allows the vhost device to batch the setup of all its host notifiers.
> This significantly reduces the device starting time, e.g. the time spend
> on enabling notifiers reduce from 376ms to 9.1ms 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.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 5994559da8..064d4abe5c 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1562,16 +1562,25 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>           return r;
>       }
>   
> +    /*
> +     * Batch all the host notifiers in a single transaction to avoid
> +     * quadratic time complexity in address_space_update_ioeventfds().
> +     */
> +    memory_region_transaction_begin();
> +
>       for (i = 0; i < hdev->nvqs; ++i) {
>           r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
>                                            true);
>           if (r < 0) {
>               error_report("vhost VQ %d notifier binding failed: %d", i, -r);
> +            memory_region_transaction_commit();
>               vhost_dev_disable_notifiers(hdev, vdev);

Could we 'break' here, ...

>               return r;
>           }
>       }
>   
> +    memory_region_transaction_commit();
> +
>       return 0;

... and return 'r' here?

>   }

Otherwise LGTM.