[PATCH v2 0/4] virtio_balloon: quiesce balloon work on device shutdown

[PATCH v2 0/4] virtio_balloon: quiesce balloon work on device shutdown

[Denis V. Lunev]

Since commit 8bd2fa086a04 ("virtio: break and reset virtio devices on
device_shutdown()") the virtio bus breaks and resets every virtio device
during device_shutdown(), i.e. on reboot and kexec. virtio_balloon has no
.shutdown of its own, so that generic path runs while the balloon's
asynchronous work is still armed: the free page reporting worker, the
inflate/deflate and stats workers, the OOM notifier and the free page
shrinker.

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 into a
fatal panic in the middle of device_shutdown(), so the machine never
reaches the new kernel. The inflate/deflate and OOM paths do not warn but
are no better off: they call wait_event(vb->acked, ...) and would block
forever on a queue that can no longer complete.

This was hit in the field as an intermittent failure of a virtualization
cluster upgrade: guest storage nodes were rebooted via kexec into the new
kernel, and the ones whose free page reporting happened to run during
device_shutdown() panicked (the guests run with panic_on_warn) and never
came back, stalling the rolling upgrade. The crash dump showed the WARN at
virtio_balloon.c:216 in a page_reporting kworker, with all the balloon
virtqueues already broken.

Validated by churning balloon inflate/deflate from the host while
kexec-rebooting the guest in a loop under panic_on_warn: the unpatched
kernel reproduces the WARN within a couple of cycles, while the patched
kernel survives many consecutive kexec cycles cleanly (12/12 in the final
run, 0 WARNs). checkpatch is clean across the series.

Changes in v2:
- Add a virtio_device_shutdown() core helper and call it from the balloon
  .shutdown handler instead of open-coding break + synchronize_cbs + reset
  (David Hildenbrand).
- New patch: make tell_host() warn and bail instead of hanging if a buffer
  add ever fails (David Hildenbrand); kept as a separate patch
  (Michael S. Tsirkin).

v1: https://lore.kernel.org/all/20260622133715.3707707-1-den@openvz.org

Denis V. Lunev (4):
  virtio: add virtio_device_shutdown() helper
  virtio_balloon: factor out virtballoon_quiesce()
  virtio_balloon: quiesce balloon work before device shutdown
  virtio_balloon: warn on failed buffer add in tell_host()

 drivers/virtio/virtio.c         | 41 ++++++++++++++++++++++-----------
 drivers/virtio/virtio_balloon.c | 40 ++++++++++++++++++++++++--------
 include/linux/virtio.h          |  1 +
 3 files changed, 59 insertions(+), 23 deletions(-)

-- 
2.53.0

[PATCH v2 1/4] virtio: add virtio_device_shutdown() helper

[Denis V. Lunev]

The generic virtio bus .shutdown handler, virtio_dev_shutdown(), breaks
and resets a device once it has established that the driver has no
.shutdown of its own. A driver that does implement .shutdown, to quiesce
its own activity first, still needs the same break and reset afterwards
and would otherwise have to open code it.

Factor the break + synchronize_cbs + reset sequence out of
virtio_dev_shutdown() into an exported virtio_device_shutdown() helper so
such drivers can reuse it instead of duplicating the core logic.

No functional change.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/virtio/virtio.c | 41 +++++++++++++++++++++++++++--------------
 include/linux/virtio.h  |  1 +
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 299fa83be1d5..75bb4ffe3b87 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -401,6 +401,32 @@ static const struct cpumask *virtio_irq_get_affinity(struct device *_d,
 	return dev->config->get_vq_affinity(dev, irq_vec);
 }
 
+/**
+ * virtio_device_shutdown - break and reset a device on shutdown
+ * @dev: the device
+ *
+ * Drivers with their own .shutdown method should quiesce their activity and
+ * then call this to stop the device the way the generic shutdown path does.
+ */
+void virtio_device_shutdown(struct virtio_device *dev)
+{
+	/*
+	 * Some devices get wedged if you kick them after they are
+	 * reset. Mark all vqs as broken to make sure we don't.
+	 */
+	virtio_break_device(dev);
+	/*
+	 * Guarantee that any callback will see vq->broken as true.
+	 */
+	virtio_synchronize_cbs(dev);
+	/*
+	 * As IOMMUs are reset on shutdown, this will block device access to memory.
+	 * Some devices get wedged if this happens, so reset to make sure it does not.
+	 */
+	dev->config->reset(dev);
+}
+EXPORT_SYMBOL_GPL(virtio_device_shutdown);
+
 static void virtio_dev_shutdown(struct device *_d)
 {
 	struct virtio_device *dev = dev_to_virtio(_d);
@@ -419,20 +445,7 @@ static void virtio_dev_shutdown(struct device *_d)
 		return;
 	}
 
-	/*
-	 * Some devices get wedged if you kick them after they are
-	 * reset. Mark all vqs as broken to make sure we don't.
-	 */
-	virtio_break_device(dev);
-	/*
-	 * Guarantee that any callback will see vq->broken as true.
-	 */
-	virtio_synchronize_cbs(dev);
-	/*
-	 * As IOMMUs are reset on shutdown, this will block device access to memory.
-	 * Some devices get wedged if this happens, so reset to make sure it does not.
-	 */
-	dev->config->reset(dev);
+	virtio_device_shutdown(dev);
 }
 
 static int virtio_dev_num_vf(struct device *dev)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index bf089e51970e..66184828fdd0 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -213,6 +213,7 @@ int virtio_device_freeze(struct virtio_device *dev);
 int virtio_device_restore(struct virtio_device *dev);
 #endif
 void virtio_reset_device(struct virtio_device *dev);
+void virtio_device_shutdown(struct virtio_device *dev);
 int virtio_device_reset_prepare(struct virtio_device *dev);
 int virtio_device_reset_done(struct virtio_device *dev);
 
-- 
2.53.0

Re: [PATCH v2 1/4] virtio: add virtio_device_shutdown() helper

[David Hildenbrand (Arm)]

On 6/24/26 16:08, Denis V. Lunev wrote:
> The generic virtio bus .shutdown handler, virtio_dev_shutdown(), breaks
> and resets a device once it has established that the driver has no
> .shutdown of its own. A driver that does implement .shutdown, to quiesce
> its own activity first, still needs the same break and reset afterwards
> and would otherwise have to open code it.
> 
> Factor the break + synchronize_cbs + reset sequence out of
> virtio_dev_shutdown() into an exported virtio_device_shutdown() helper so
> such drivers can reuse it instead of duplicating the core logic.
> 
> No functional change.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---

Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

[PATCH v2 2/4] virtio_balloon: factor out virtballoon_quiesce()

[Denis V. Lunev]

virtballoon_remove() stops all of the balloon's asynchronous work (the
free page reporting worker, the inflate/deflate and stats workers, the
OOM notifier and the free page shrinker) before tearing the device
down. A following change needs the same teardown from a .shutdown
handler, so move it into a virtballoon_quiesce() helper.

No functional change.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/virtio/virtio_balloon.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 088b3a0e6ce6..5b02d9191ac6 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1098,26 +1098,39 @@ static void remove_common(struct virtio_balloon *vb)
 	vb->vdev->config->del_vqs(vb->vdev);
 }
 
-static void virtballoon_remove(struct virtio_device *vdev)
+/*
+ * Stop all asynchronous balloon work. The device must still be alive so that
+ * in-flight requests can drain via the host before it is reset or freed.
+ */
+static void virtballoon_quiesce(struct virtio_balloon *vb)
 {
-	struct virtio_balloon *vb = vdev->priv;
+	struct virtio_device *vdev = vb->vdev;
 
-	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_REPORTING))
 		page_reporting_unregister(&vb->pr_dev_info);
-	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
 		unregister_oom_notifier(&vb->oom_nb);
-	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
 		virtio_balloon_unregister_shrinker(vb);
+
 	spin_lock_irq(&vb->stop_update_lock);
 	vb->stop_update = true;
 	spin_unlock_irq(&vb->stop_update_lock);
 	cancel_work_sync(&vb->update_balloon_size_work);
 	cancel_work_sync(&vb->update_balloon_stats_work);
 
-	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
 		cancel_work_sync(&vb->report_free_page_work);
+}
+
+static void virtballoon_remove(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+
+	virtballoon_quiesce(vb);
+
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
 		destroy_workqueue(vb->balloon_wq);
-	}
 
 	remove_common(vb);
 	mutex_destroy(&vb->balloon_lock);
-- 
2.53.0

Re: [PATCH v2 2/4] virtio_balloon: factor out virtballoon_quiesce()

[David Hildenbrand (Arm)]

On 6/24/26 16:08, Denis V. Lunev wrote:
> virtballoon_remove() stops all of the balloon's asynchronous work (the
> free page reporting worker, the inflate/deflate and stats workers, the
> OOM notifier and the free page shrinker) before tearing the device
> down. A following change needs the same teardown from a .shutdown
> handler, so move it into a virtballoon_quiesce() helper.
> 
> No functional change.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

[PATCH v2 3/4] virtio_balloon: quiesce balloon work before device shutdown

[Denis V. Lunev]

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.

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 via virtio_device_shutdown(). 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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 5b02d9191ac6..26fc3c40d5b2 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1137,6 +1137,12 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	kfree(vb);
 }
 
+static void virtballoon_shutdown(struct virtio_device *vdev)
+{
+	virtballoon_quiesce(vdev->priv);
+	virtio_device_shutdown(vdev);
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int virtballoon_freeze(struct virtio_device *vdev)
 {
@@ -1202,6 +1208,7 @@ static struct virtio_driver virtio_balloon_driver = {
 	.validate =	virtballoon_validate,
 	.probe =	virtballoon_probe,
 	.remove =	virtballoon_remove,
+	.shutdown =	virtballoon_shutdown,
 	.config_changed = virtballoon_changed,
 #ifdef CONFIG_PM_SLEEP
 	.freeze	=	virtballoon_freeze,
-- 
2.53.0

Re: [PATCH v2 3/4] virtio_balloon: quiesce balloon work before device shutdown

[David Hildenbrand (Arm)]

On 6/24/26 16:08, 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.
> 
> 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 via virtio_device_shutdown(). 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 | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 5b02d9191ac6..26fc3c40d5b2 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -1137,6 +1137,12 @@ static void virtballoon_remove(struct virtio_device *vdev)
>  	kfree(vb);
>  }
>  
> +static void virtballoon_shutdown(struct virtio_device *vdev)
> +{
> +	virtballoon_quiesce(vdev->priv);
> +	virtio_device_shutdown(vdev);
> +}

I'm curious why virtio_gpu_shutdown() doesn't need that (did not look into the
details).

Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 3/4] virtio_balloon: quiesce balloon work before device shutdown

[Denis V. Lunev]

On 6/24/26 16:55, David Hildenbrand (Arm) wrote:
> On 6/24/26 16:08, 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.
>>
>> 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 via virtio_device_shutdown(). 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 | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>> index 5b02d9191ac6..26fc3c40d5b2 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -1137,6 +1137,12 @@ static void virtballoon_remove(struct virtio_device *vdev)
>>  	kfree(vb);
>>  }
>>  
>> +static void virtballoon_shutdown(struct virtio_device *vdev)
>> +{
>> +	virtballoon_quiesce(vdev->priv);
>> +	virtio_device_shutdown(vdev);
>> +}
> I'm curious why virtio_gpu_shutdown() doesn't need that (did not look into the
> details).
>
> Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
>
I would spend more time with other drivers once we will
done with this. I have strong candidate - virtio-mem.

Den

Re: [PATCH v2 3/4] virtio_balloon: quiesce balloon work before device shutdown

[David Hildenbrand (Arm)]

On 6/24/26 17:00, Denis V. Lunev wrote:
> On 6/24/26 16:55, David Hildenbrand (Arm) wrote:
>> On 6/24/26 16:08, 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.
>>>
>>> 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 via virtio_device_shutdown(). 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 | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>>> index 5b02d9191ac6..26fc3c40d5b2 100644
>>> --- a/drivers/virtio/virtio_balloon.c
>>> +++ b/drivers/virtio/virtio_balloon.c
>>> @@ -1137,6 +1137,12 @@ static void virtballoon_remove(struct virtio_device *vdev)
>>>  	kfree(vb);
>>>  }
>>>  
>>> +static void virtballoon_shutdown(struct virtio_device *vdev)
>>> +{
>>> +	virtballoon_quiesce(vdev->priv);
>>> +	virtio_device_shutdown(vdev);
>>> +}
>> I'm curious why virtio_gpu_shutdown() doesn't need that (did not look into the
>> details).
>>
>> Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
>>
> I would spend more time with other drivers once we will
> done with this. I have strong candidate - virtio-mem.

Heh, I briefly checked and it should handle it better I think.

If virtqueue_add_sgs() fails, it propagates the error (-EIO?) back to the main
loop where we end up in

switch (rc) {
	...
	default:
	/* Unknown error, mark as broken */
	dev_err(&vm->vdev->dev, ...
	vm->broken = true;
}

And just stop.

But I didn't actually look into the details.

-- 
Cheers,

David

[PATCH v2 4/4] virtio_balloon: warn on failed buffer add in tell_host()

[Denis V. Lunev]

tell_host() ignores the return value of virtqueue_add_outbuf() and goes
on to kick the queue and wait_event() for the host's ack. The comment
claims "We should always be able to add one buffer to an empty queue",
but that does not hold once the virtqueue has been broken (e.g. on
device shutdown): the add then fails with -EIO and the following
wait_event() would block forever on a buffer the host can never return.

Warn and bail out on failure, mirroring virtballoon_free_page_report().

Suggested-by: David Hildenbrand <david@kernel.org>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/virtio/virtio_balloon.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 26fc3c40d5b2..0866a8781f0b 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -184,16 +184,18 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 {
 	struct scatterlist sg;
 	unsigned int len;
+	int err;
 
 	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
 
 	/* We should always be able to add one buffer to an empty queue. */
-	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	if (WARN_ON_ONCE(err))
+		return;
 	virtqueue_kick(vq);
 
 	/* When host has read buffer, this completes via balloon_ack */
 	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
-
 }
 
 static int virtballoon_free_page_report(struct page_reporting_dev_info *pr_dev_info,
-- 
2.53.0

Re: [PATCH v2 4/4] virtio_balloon: warn on failed buffer add in tell_host()

[David Hildenbrand (Arm)]

On 6/24/26 16:08, Denis V. Lunev wrote:
> tell_host() ignores the return value of virtqueue_add_outbuf() and goes
> on to kick the queue and wait_event() for the host's ack. The comment
> claims "We should always be able to add one buffer to an empty queue",
> but that does not hold once the virtqueue has been broken (e.g. on
> device shutdown): the add then fails with -EIO and the following
> wait_event() would block forever on a buffer the host can never return.
> 
> Warn and bail out on failure, mirroring virtballoon_free_page_report().
> 
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  drivers/virtio/virtio_balloon.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 26fc3c40d5b2..0866a8781f0b 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -184,16 +184,18 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
>  {
>  	struct scatterlist sg;
>  	unsigned int len;
> +	int err;
>  
>  	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
>  
>  	/* We should always be able to add one buffer to an empty queue. */
> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	if (WARN_ON_ONCE(err))
> +		return;
>  	virtqueue_kick(vq);
>  
>  	/* When host has read buffer, this completes via balloon_ack */
>  	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> -
>  }

We have another uncheck instance in stats_handle_request(), what about that one?

-- 
Cheers,

David

[PATCH v2 5/4] virtio_balloon: warn on failed buffer add in stats_handle_request()

[Denis V. Lunev]

Like tell_host(), stats_handle_request() ignores the return value of
virtqueue_add_outbuf() and kicks the queue regardless. The same "we
should always be able to add one buffer to an empty queue" assumption
does not hold once the virtqueue has been broken (e.g. on device
shutdown), where the add fails with -EIO. Unlike tell_host() it does
not wait_event() afterwards so it cannot hang, but it still kicks a
queue with nothing queued.

Warn and bail out on failure, mirroring tell_host() and
virtballoon_free_page_report().

Suggested-by: David Hildenbrand <david@kernel.org>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/virtio/virtio_balloon.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0866a8781f0b..454bbb77331d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -445,6 +445,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
 	struct virtqueue *vq;
 	struct scatterlist sg;
 	unsigned int len, num_stats;
+	int err;
 
 	num_stats = update_balloon_stats(vb);
 
@@ -452,7 +453,9 @@ static void stats_handle_request(struct virtio_balloon *vb)
 	if (!virtqueue_get_buf(vq, &len))
 		return;
 	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
-	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+	if (WARN_ON_ONCE(err))
+		return;
 	virtqueue_kick(vq);
 }
 
-- 
2.53.0

Re: [PATCH v2 5/4] virtio_balloon: warn on failed buffer add in stats_handle_request()

[David Hildenbrand (Arm)]

On 6/24/26 17:40, Denis V. Lunev wrote:
> Like tell_host(), stats_handle_request() ignores the return value of
> virtqueue_add_outbuf() and kicks the queue regardless. The same "we
> should always be able to add one buffer to an empty queue" assumption
> does not hold once the virtqueue has been broken (e.g. on device
> shutdown), where the add fails with -EIO. Unlike tell_host() it does
> not wait_event() afterwards so it cannot hang, but it still kicks a
> queue with nothing queued.
> 
> Warn and bail out on failure, mirroring tell_host() and
> virtballoon_free_page_report().
> 
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  drivers/virtio/virtio_balloon.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 0866a8781f0b..454bbb77331d 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -445,6 +445,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
>  	struct virtqueue *vq;
>  	struct scatterlist sg;
>  	unsigned int len, num_stats;
> +	int err;
>  
>  	num_stats = update_balloon_stats(vb);
>  
> @@ -452,7 +453,9 @@ static void stats_handle_request(struct virtio_balloon *vb)
>  	if (!virtqueue_get_buf(vq, &len))
>  		return;
>  	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
> +	if (WARN_ON_ONCE(err))
> +		return;
>  	virtqueue_kick(vq);
>  }
>  

Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>

Although I would just squash #4 and #5.

-- 
Cheers,

David

Re: [PATCH v2 5/4] virtio_balloon: warn on failed buffer add in stats_handle_request()

[Denis V. Lunev]

On 6/24/26 18:56, David Hildenbrand (Arm) wrote:
> On 6/24/26 17:40, Denis V. Lunev wrote:
>> Like tell_host(), stats_handle_request() ignores the return value of
>> virtqueue_add_outbuf() and kicks the queue regardless. The same "we
>> should always be able to add one buffer to an empty queue" assumption
>> does not hold once the virtqueue has been broken (e.g. on device
>> shutdown), where the add fails with -EIO. Unlike tell_host() it does
>> not wait_event() afterwards so it cannot hang, but it still kicks a
>> queue with nothing queued.
>>
>> Warn and bail out on failure, mirroring tell_host() and
>> virtballoon_free_page_report().
>>
>> Suggested-by: David Hildenbrand <david@kernel.org>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> ---
>>  drivers/virtio/virtio_balloon.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
>> index 0866a8781f0b..454bbb77331d 100644
>> --- a/drivers/virtio/virtio_balloon.c
>> +++ b/drivers/virtio/virtio_balloon.c
>> @@ -445,6 +445,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
>>  	struct virtqueue *vq;
>>  	struct scatterlist sg;
>>  	unsigned int len, num_stats;
>> +	int err;
>>  
>>  	num_stats = update_balloon_stats(vb);
>>  
>> @@ -452,7 +453,9 @@ static void stats_handle_request(struct virtio_balloon *vb)
>>  	if (!virtqueue_get_buf(vq, &len))
>>  		return;
>>  	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
>> -	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
>> +	err = virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
>> +	if (WARN_ON_ONCE(err))
>> +		return;
>>  	virtqueue_kick(vq);
>>  }
>>  
> Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
>
> Although I would just squash #4 and #5.
>
Sure thing. Does this way to avoid re-submit if that is possible.

Den