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

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

在 2022/12/6 16:30, 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;
>
> ?
Good suggestion! I'll do in v3, thanks.

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