On 18.05.20 17:35, Alexander Duyck wrote:
> On Mon, May 18, 2020 at 1:37 AM David Hildenbrand <da...@redhat.com> wrote:
>>
>> We took a reference when realizing, so let's drop that reference when
>> unrealizing.
>>
>> Fixes: c13c4153f76d ("virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT")
>> Cc: Wei Wang <wei.w.w...@intel.com>
>> Cc: Alexander Duyck <alexander.du...@gmail.com>
>> Cc: Michael S. Tsirkin <m...@redhat.com>
>> Cc: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>> ---
>> hw/virtio/virtio-balloon.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index a4fcf2d777..3f8fc50be0 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -820,6 +820,7 @@ static void virtio_balloon_device_unrealize(DeviceState
>> *dev)
>>
>> if (s->free_page_bh) {
>> qemu_bh_delete(s->free_page_bh);
>> + object_unref(OBJECT(s->iothread));
>> virtio_balloon_free_page_stop(s);
>> precopy_remove_notifier(&s->free_page_report_notify);
>> }
>
> I'm not entirely sure about this order of operations. It seems like it
> would make more sense to remove the notifier, stop the hinting, delete
> the bh, and then release the IO thread.
This is the reverse order of the steps in
virtio_balloon_device_realize(). And I guess it should be fine. The
notifier cannot really be active/trigger while we are removing devices
(cannot happen with concurrent migration). After qemu_bh_delete(), the
iothread is effectively unused.
I am unsure about many things regarding free page hinting (e.g., if the
virtio_balloon_free_page_stop() is of any use while we are ripping out
the device and it will be gone in a second).
--
Thanks,
David / dhildenb
