[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] virtio: Add corresponding memory_listener_unregister to unre
From: |
Eugenio Perez Martin |
Subject: |
Re: [PATCH] virtio: Add corresponding memory_listener_unregister to unrealize |
Date: |
Mon, 25 Jan 2021 20:22:47 +0100 |
On Mon, Jan 25, 2021 at 6:18 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Mon, Jan 25, 2021 at 05:55:35PM +0100, Eugenio Perez Martin wrote:
> > On Mon, Jan 25, 2021 at 4:15 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > >
> > > On 2021/1/23 上午4:08, Eugenio Pérez wrote:
> > > > Cannot destroy address spaces of IOMMU-aware virtio devices without it,
> > > > since they can contain memory listeners.
> > >
> > >
> > > It's better to explain why the one in finalize doesn't work here.
> > >
> >
> > Hi Jason! Good point. The other call is at virtio_device_instance_finalize.
> >
> > Function virtio_device_instance_finalize is called after
> > address_space_destroy if we follow steps of buglink.
> >
> > Address_space_destroy is called by
> > pci_qdev_unrealize/do_pci_unregister_device, after call to
> > virtio_device_unrealize. After that call,
> > virtio_device_instance_finalize is called through object_deinit,
> > freeing the bus.
> >
> > Also, memory_listener_unregister can be called again because it checks
> > for listener->address_space != NULL at start, and sets it to NULL at
> > end.
> >
> > In regular shutdown, nothing of this is called, so maybe we could
> > safely delete the call to memory_listener_unregister at
> > virtio_device_instance_finalize?
>
> I didn't notice this; if so we'd better remove that call if it's destined to
> be
> a noop after all.
>
> >
> > If not, should I send again the patch with a new commit message?
>
> Maybe attach the full backtrace too along with above? The assertion itself
> could be a very good explanation of what's happened.
>
I agree, sending v2.
Thanks!
> Thanks,
>
> --
> Peter Xu
>