Re: [PATCH v2 2/2] vdpa: commit all host notifier MRs in a single MR transaction

[Philippe Mathieu-Daudé]

On 6/12/22 09:18, Longpeng(Mike) via 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[1] (64vq per device).
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg921541.html
>
> Signed-off-by: Longpeng <longpeng2@huawei.com>
> ---
>   hw/virtio/vhost-vdpa.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)


> @@ -562,16 +571,25 @@ static void vhost_vdpa_host_notifiers_init(struct 
> vhost_dev *dev)
>           return;
>       }
>   
> +    /*
> +     * 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 + dev->nvqs; i++) {
>           if (vhost_vdpa_host_notifier_init(dev, i)) {
>               goto err;

Could we simplify by replacing this goto statement with:

               vhost_vdpa_host_notifiers_uninit(dev, i - dev->vq_index);
               break;

?

>           }
>       }
>   
> +    memory_region_transaction_commit();
>       return;
>   
>   err:
>       vhost_vdpa_host_notifiers_uninit(dev, i - dev->vq_index);
> +    memory_region_transaction_commit();
>       return;
>   }
>