From: "Michael S. Tsirkin" <mst@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: mm-commits@vger.kernel.org, xuanzhuo@linux.alibaba.com,
vbabka@kernel.org, stable@vger.kernel.org, rientjes@google.com,
jthoughton@google.com, jiaqiyan@google.com, jasowang@redhat.com,
gthelen@google.com, david@redhat.com, david@kernel.org,
alexander.duyck@gmail.com, linkl@google.com
Subject: Re: + virtio_balloon-avoid-shrinker-execution-during-pm-suspend.patch added to mm-new branch
Date: Sat, 18 Jul 2026 10:16:54 -0400 [thread overview]
Message-ID: <20260718101506-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20260718003902.B6DD51F000E9@smtp.kernel.org>
On Fri, Jul 17, 2026 at 05:39:02PM -0700, Andrew Morton wrote:
>
> The patch titled
> Subject: virtio_balloon: avoid shrinker execution during PM suspend
> has been added to the -mm mm-new branch. Its filename is
> virtio_balloon-avoid-shrinker-execution-during-pm-suspend.patch
>
> This patch will shortly appear at
> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/virtio_balloon-avoid-shrinker-execution-during-pm-suspend.patch
>
> This patch will later appear in the mm-new branch at
> git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>
> Note, mm-new is a provisional staging ground for work-in-progress
> patches, and acceptance into mm-new is a notification for others take
> notice and to finish up reviews. Please do not hesitate to respond to
> review feedback and post updated versions to replace or incrementally
> fixup patches in mm-new.
>
> The mm-new branch of mm.git is not included in linux-next
>
> If a few days of testing in mm-new is successful, the patch will me moved
> into mm.git's mm-unstable branch, which is included in linux-next
>
> Before you just go and hit "reply", please:
> a) Consider who else should be cc'ed
> b) Prefer to cc a suitable mailing list as well
> c) Ideally: find the original patch on the mailing list and do a
> reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
>
> The -mm tree is included into linux-next via various
> branches at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> and is updated there most days
>
> ------------------------------------------------------
> From: Link Lin <linkl@google.com>
> Subject: virtio_balloon: avoid shrinker execution during PM suspend
> Date: Fri, 17 Jul 2026 00:22:21 +0000
>
> 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.
>
> 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.
>
> Link: https://lore.kernel.org/20260717002311.681748-3-linkl@google.com
> Fixes: 71019de8219b ("virtio_balloon: Add free page hinting support")
> Signed-off-by: Link Lin <linkl@google.com>
> Acked-by: David Rientjes <rientjes@google.com>
> Suggested-by: James Houghton <jthoughton@google.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Alexander Duyck <alexander.duyck@gmail.com>
> Cc: David Hildenbrand <david@kernel.org>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Jiaqi Yan <jiaqiyan@google.com>
> Cc: Vlastimil Babka <vbabka@kernel.org>
> Cc: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This patch generated quite a bit of discussion. Are you
sure it makes sense to keep it in mm as is?
In particular:
> ---
>
> drivers/virtio/virtio_balloon.c | 49 +++++++++++++++++++++++++-----
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
> --- a/drivers/virtio/virtio_balloon.c~virtio_balloon-avoid-shrinker-execution-during-pm-suspend
> +++ a/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
> 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_t
> 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_t
> 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_shri
> {
> struct virtio_balloon *vb = shrinker->private_data;
>
> + if (READ_ONCE(vb->suspended))
> + return 0;
> +
what if suspended bit is set right afterwards?
Seems racy.
> return shrink_free_pages(vb, sc->nr_to_scan);
> }
>
> @@ -879,6 +898,9 @@ static unsigned long virtio_balloon_shri
> {
> struct virtio_balloon *vb = shrinker->private_data;
>
> + if (READ_ONCE(vb->suspended))
> + return 0;
> +
> return vb->num_free_page_blocks * VIRTIO_BALLOON_HINT_BLOCK_PAGES;
> }
>
> @@ -1086,8 +1108,11 @@ static void remove_common(struct virtio_
> 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);
> + }
>
> /* Now we reset the device so we can clean up the queues. */
> virtio_reset_device(vb->vdev);
> @@ -1130,6 +1155,10 @@ static int virtballoon_freeze(struct vir
> * 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;
> }
> @@ -1145,6 +1174,10 @@ static int virtballoon_restore(struct vi
>
> 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);
> _
>
> Patches currently in -mm which might be from linkl@google.com are
>
> mm-page_reporting-use-system_freezable_wq-to-fix-uaf-during-suspend.patch
> virtio_balloon-avoid-shrinker-execution-during-pm-suspend.patch
next prev parent reply other threads:[~2026-07-18 14:17 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-18 0:39 + virtio_balloon-avoid-shrinker-execution-during-pm-suspend.patch added to mm-new branch Andrew Morton
2026-07-18 14:16 ` Michael S. Tsirkin [this message]
2026-07-18 20:05 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260718101506-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.duyck@gmail.com \
--cc=david@kernel.org \
--cc=david@redhat.com \
--cc=gthelen@google.com \
--cc=jasowang@redhat.com \
--cc=jiaqiyan@google.com \
--cc=jthoughton@google.com \
--cc=linkl@google.com \
--cc=mm-commits@vger.kernel.org \
--cc=rientjes@google.com \
--cc=stable@vger.kernel.org \
--cc=vbabka@kernel.org \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox