On Mon, Feb 14, 2022 at 3:33 PM Laurent Vivier <lviv...@redhat.com> wrote:
>
> On 14/02/2022 04:20, Jason Wang wrote:
> > On Sat, Feb 12, 2022 at 12:13 AM Laurent Vivier <lviv...@redhat.com> wrote:
> >>
> >> vhost_vdpa_host_notifiers_init() initializes queue notifiers
> >> for queues "dev->vq_index" to queue "dev->vq_index + dev->nvqs",
> >> whereas vhost_vdpa_host_notifiers_uninit() uninitializes the
> >> same notifiers for queue "0" to queue "dev->nvqs".
> >>
> >> This asymmetry seems buggy, fix that by using dev->vq_index
> >> as the base for both.
> >>
> >> Fixes: d0416d487bd5 ("vhost-vdpa: map virtqueue notification area if
> >> possible")
> >> Cc: jasow...@redhat.com
> >> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> >> ---
> >> hw/virtio/vhost-vdpa.c | 20 ++++++++++----------
> >> 1 file changed, 10 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> >> index 04ea43704f5d..9be3dc66580c 100644
> >> --- a/hw/virtio/vhost-vdpa.c
> >> +++ b/hw/virtio/vhost-vdpa.c
> >> @@ -395,15 +395,6 @@ static void vhost_vdpa_host_notifier_uninit(struct
> >> vhost_dev *dev,
> >> }
> >> }
> >>
> >> -static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)
> >> -{
> >> - int i;
> >> -
> >> - for (i = 0; i < n; i++) {
> >> - vhost_vdpa_host_notifier_uninit(dev, i);
> >> - }
> >> -}
> >> -
> >> static int vhost_vdpa_host_notifier_init(struct vhost_dev *dev, int
> >> queue_index)
> >> {
> >> size_t page_size = qemu_real_host_page_size;
> >> @@ -442,6 +433,15 @@ err:
> >> return -1;
> >> }
> >>
> >> +static void vhost_vdpa_host_notifiers_uninit(struct vhost_dev *dev, int n)
> >> +{
> >> + int i;
> >> +
> >> + for (i = dev->vq_index; i < dev->vq_index + n; i++) {
> >> + vhost_vdpa_host_notifier_uninit(dev, i);
> >> + }
> >> +}
> >
> > Patch looks good but I wonder why we need to move this function?
>
> I moved the _uninit function close to the _init one to be able to compare
> them easier.
> I think it will help reviewers in the future if code is side-by-side.
Fine.
So
Acked-by: Jason Wang <jasow...@redhat.com>
Thanks
>
> But we can let it at its original place.
>
> Thanks,
> Laurent
>
