Re: [PATCH v1 3/3] virtio-balloon: unref the iothread when unrealizing

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

+Stefan/Paolo in case they have a better clue.

On 5/18/20 5:41 PM, David Hildenbrand wrote:
> On 18.05.20 17:35, Alexander Duyck wrote:
>> On Mon, May 18, 2020 at 1:37 AM David Hildenbrand <david@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.wang@intel.com>
>>> Cc: Alexander Duyck <alexander.duyck@gmail.com>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Signed-off-by: David Hildenbrand <david@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).
>