Re: [PATCH 2/2] virtio_balloon: quiesce balloon work before device shutdown

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jun 23, 2026 at 09:25:18PM +0200, Denis V. Lunev wrote:
> On 6/22/26 16:38, David Hildenbrand (Arm) wrote:
> > This email originated from an IP that might not be authorized by the domain it was sent from.
> > Do not click links or open attachments unless it is an email you expected to receive.
> > On 6/22/26 15:37, Denis V. Lunev wrote:
> >> Commit 8bd2fa086a04 ("virtio: break and reset virtio devices on
> >> device_shutdown()") added a generic virtio bus .shutdown handler that
> >> breaks and resets every virtio device during device_shutdown(), i.e. on
> >> reboot and kexec.
> >>
> >> virtio_balloon provides no .shutdown of its own, so that generic path
> >> runs while the balloon's asynchronous work is still armed. Once the
> >> device has been broken, virtqueue_add_inbuf() in
> >> virtballoon_free_page_report() returns -EIO and trips its
> >> WARN_ON_ONCE(). On a kernel booted with panic_on_warn that turns an
> >> ordinary reboot, for example a kexec based upgrade, into a fatal panic
> >> in the middle of device_shutdown(), so the machine never reaches the
> >> new kernel.
> >>
> >> Relaxing that single WARN_ON_ONCE() would only hide the symptom: the
> >> inflate/deflate and OOM paths do not warn, they call
> >> wait_event(vb->acked, ...) and would instead block forever on a broken
> >> queue that can no longer complete. The device has to be quiesced, not
> >> just kept quiet.
> > Ah, so
> >
> > 	/* We should always be able to add one buffer to an empty queue. */
> > 	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> >
> > is not actually correct.
> >
> > Yeah, quiescing sounds cleaner, although I am thinking whether we should also
> > warn if virtqueue_add_outbuf() fails, similar to what we do in
> > virtballoon_free_page_report().
> Good catch, will do.,

separate patch pls.

> >> Add a .shutdown handler that quiesces the balloon via the shared
> >> virtballoon_quiesce() helper while the device is still alive, and only
> >> then breaks and resets it. The break and reset are repeated here rather
> >> than reused from virtio_dev_shutdown(): drv->shutdown replaces the
> >> generic handler rather than augmenting it, so that drivers such as
> >> virtio-gpu can opt out of the reset. Unlike virtballoon_remove() the
> >> balloon workqueue is not destroyed, as shutdown does not free the
> >> device and cancel_work_sync() together with stop_update already prevent
> >> any further work from being queued.
> >>
> >> Fixes: 8bd2fa086a04 ("virtio: break and reset virtio devices on device_shutdown()")
> >> Signed-off-by: Denis V. Lunev <den@openvz.org>
> >> ---
> >>  drivers/virtio/virtio_balloon.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> >> index 5b02d9191ac6..e35ada767b4b 100644
> >> --- a/drivers/virtio/virtio_balloon.c
> >> +++ b/drivers/virtio/virtio_balloon.c
> >> @@ -1137,6 +1137,15 @@ static void virtballoon_remove(struct virtio_device *vdev)
> >>  	kfree(vb);
> >>  }
> >>  
> >> +static void virtballoon_shutdown(struct virtio_device *vdev)
> >> +{
> >> +	virtballoon_quiesce(vdev->priv);
> >> +
> >> +	virtio_break_device(vdev);
> >> +	virtio_synchronize_cbs(vdev);
> >> +	vdev->config->reset(vdev);
> > I guess it would be good if we wouldn't have to copy what the default handler
> > does, but could instead just have it in a reusable core function?
> Ok. Sounds great. Will do.
> 
> Thanks for review,
>     Den