On Fri, Jul 17, 2026 at 12:22:21AM +0000, Link Lin wrote:
> During PM freeze (e.g. S4 hibernation), virtballoon_freeze() calls
> remove_common() which resets the virtio device and deletes all virtqueues.
> However, the balloon shrinker remains registered with core MM.
>
> If memory pressure occurs during S4 hibernation image creation/saving, MM
> invokes virtio_balloon_shrinker_scan(), which attempts to reclaim free
> pages. Although return_free_pages_to_mm() only frees pages back to MM,
> reclaiming free pages under memory pressure can trigger page reporting
> which might access the deleted reporting virtqueue if it is not yet
> frozen, or interact with other parts of the driver in a teardown state.
i feel this is the thing to fix then? not the freeing itself.
>
> Avoid this by adding a `suspended` flag to `struct virtio_balloon`. Set
> this flag to true in virtballoon_freeze() and false in
> virtballoon_restore(). Check this flag in both shrinker callbacks (scan
> and count) and return 0 if the device is suspended, preventing any
> shrinker execution while virtqueues are deleted. Wrap the lockless reads
> in READ_ONCE() and writes in WRITE_ONCE() to prevent compiler
> optimization issues and KCSAN data race warnings.
so you prevented the warnings but did you fix the races? CB
>
> Fixes: 71019de8219b ("virtio_balloon: Add free page hinting support")
> Cc: [email protected]
> Acked-by: David Rientjes <[email protected]>
> Signed-off-by: Link Lin <[email protected]>
> ---
> drivers/virtio/virtio_balloon.c | 49 +++++++++++++++++++++++++++------
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 088b3a0e6c..38e1166a64 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -68,6 +68,8 @@ struct virtio_balloon {
> /* Prevent updating balloon when it is being canceled. */
> spinlock_t stop_update_lock;
> bool stop_update;
> + /* Prevent shrinker from running while device is suspended. */
> + bool suspended;
> /* Bitmap to indicate if reading the related config fields are needed */
> unsigned long config_read_bitmap;
>
> @@ -471,9 +473,9 @@ static inline s64 towards_target(struct virtio_balloon
> *vb)
> return target - vb->num_pages;
> }
>
> -/* Gives back @num_to_return blocks of free pages to mm. */
> -static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb,
> - unsigned long num_to_return)
> +/* Helper: must be called with free_page_list_lock held */
> +static unsigned long __return_free_pages_to_mm(struct virtio_balloon *vb,
> + unsigned long num_to_return)
> {
> unsigned long num_returned = 0;
> struct page *page, *next;
> @@ -481,8 +483,6 @@ static unsigned long return_free_pages_to_mm(struct
> virtio_balloon *vb,
> if (unlikely(!num_to_return))
> return 0;
>
> - spin_lock_irq(&vb->free_page_list_lock);
> -
> list_for_each_entry_safe(page, next, &vb->free_page_list, lru) {
> list_del(&page->lru);
> __free_pages(page, VIRTIO_BALLOON_HINT_BLOCK_ORDER);
> @@ -490,11 +490,27 @@ static unsigned long return_free_pages_to_mm(struct
> virtio_balloon *vb,
> break;
> }
> vb->num_free_page_blocks -= num_returned;
> - spin_unlock_irq(&vb->free_page_list_lock);
>
> return num_returned;
> }
>
> +/* Gives back @num_to_return blocks of free pages to mm. */
> +static unsigned long return_free_pages_to_mm(struct virtio_balloon *vb,
> + unsigned long num_to_return)
> +{
> + unsigned long ret;
> +
> + spin_lock_irq(&vb->free_page_list_lock);
> + if (vb->suspended) {
> + spin_unlock_irq(&vb->free_page_list_lock);
> + return 0;
> + }
> + ret = __return_free_pages_to_mm(vb, num_to_return);
> + spin_unlock_irq(&vb->free_page_list_lock);
> +
> + return ret;
> +}
> +
> static void virtio_balloon_queue_free_page_work(struct virtio_balloon *vb)
> {
> if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> @@ -871,6 +887,9 @@ static unsigned long virtio_balloon_shrinker_scan(struct
> shrinker *shrinker,
> {
> struct virtio_balloon *vb = shrinker->private_data;
>
> + if (READ_ONCE(vb->suspended))
> + return 0;
> +
what if vb->suspended is set right here?
> return shrink_free_pages(vb, sc->nr_to_scan);
> }
>
> @@ -879,6 +898,9 @@ static unsigned long virtio_balloon_shrinker_count(struct
> shrinker *shrinker,
> {
> struct virtio_balloon *vb = shrinker->private_data;
>
> + if (READ_ONCE(vb->suspended))
> + return 0;
> +
or here?
> return vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
> }
>
> @@ -1089,8 +1111,11 @@ static void remove_common(struct virtio_balloon *vb)
> update_balloon_size(vb);
>
> /* There might be free pages that are being reported: release them. */
> - if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> - return_free_pages_to_mm(vb, ULONG_MAX);
> + if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> + spin_lock_irq(&vb->free_page_list_lock);
> + __return_free_pages_to_mm(vb, ULONG_MAX);
> + spin_unlock_irq(&vb->free_page_list_lock);
this is remove, taking locks seems futile - if something will acquire
the lock here, we will be in trouble.
> + }
>
> /* Now we reset the device so we can clean up the queues. */
> virtio_reset_device(vb->vdev);
> @@ -1133,6 +1158,10 @@ static int virtballoon_freeze(struct virtio_device
> *vdev)
> * The workqueue is already frozen by the PM core before this
> * function is called.
> */
> + spin_lock_irq(&vb->free_page_list_lock);
> + WRITE_ONCE(vb->suspended, true);
> + spin_unlock_irq(&vb->free_page_list_lock);
> +
> remove_common(vb);
> return 0;
> }
> @@ -1148,6 +1177,10 @@ static int virtballoon_restore(struct virtio_device
> *vdev)
>
> virtio_device_ready(vdev);
>
> + spin_lock_irq(&vb->free_page_list_lock);
> + WRITE_ONCE(vb->suspended, false);
> + spin_unlock_irq(&vb->free_page_list_lock);
> +
> if (towards_target(vb))
> virtballoon_changed(vdev);
> update_balloon_size(vb);
> --
> 2.55.0.229.g6434b31f56-goog
