Skip to content

Commit 60bd04f

Browse files
committed
virtio_balloon: fix shrinker scan number of pages
virtio_balloon_shrinker_scan should return number of system pages freed, but because it's calling functions that deal with balloon pages, it gets confused and sometimes returns the number of balloon pages. It does not matter practically as the exact number isn't used, but it seems better to be consistent in case someone starts using this API. Further, if we ever tried to iteratively leak pages as virtio_balloon_shrinker_scan tries to do, we'd run into issues - this is because freed_pages was accumulating total freed pages, but was also subtracted on each iteration from pages_to_free, which can result in either leaking less memory than we were supposed to free, or more if pages_to_free underruns. On a system with 4K pages we are lucky that we are never asked to leak more than 128 pages while we can leak up to 256 at a time, but it looks like a real issue for systems with page size != 4K. Fixes: 7199462 ("virtio_balloon: replace oom notifier with shrinker") Reported-by: Khazhismel Kumykov <[email protected]> Reviewed-by: Wei Wang <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]>
1 parent d791cfc commit 60bd04f

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

drivers/virtio/virtio_balloon.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,13 @@ static unsigned long shrink_free_pages(struct virtio_balloon *vb,
772772
return blocks_freed << VIRTIO_BALLOON_FREE_PAGE_ORDER;
773773
}
774774

775+
static unsigned long leak_balloon_pages(struct virtio_balloon *vb,
776+
unsigned long pages_to_free)
777+
{
778+
return leak_balloon(vb, pages_to_free * VIRTIO_BALLOON_PAGES_PER_PAGE) /
779+
VIRTIO_BALLOON_PAGES_PER_PAGE;
780+
}
781+
775782
static unsigned long shrink_balloon_pages(struct virtio_balloon *vb,
776783
unsigned long pages_to_free)
777784
{
@@ -782,11 +789,10 @@ static unsigned long shrink_balloon_pages(struct virtio_balloon *vb,
782789
* VIRTIO_BALLOON_ARRAY_PFNS_MAX balloon pages, so we call it
783790
* multiple times to deflate pages till reaching pages_to_free.
784791
*/
785-
while (vb->num_pages && pages_to_free) {
786-
pages_freed += leak_balloon(vb, pages_to_free) /
787-
VIRTIO_BALLOON_PAGES_PER_PAGE;
788-
pages_to_free -= pages_freed;
789-
}
792+
while (vb->num_pages && pages_freed < pages_to_free)
793+
pages_freed += leak_balloon_pages(vb,
794+
pages_to_free - pages_freed);
795+
790796
update_balloon_size(vb);
791797

792798
return pages_freed;
@@ -799,7 +805,7 @@ static unsigned long virtio_balloon_shrinker_scan(struct shrinker *shrinker,
799805
struct virtio_balloon *vb = container_of(shrinker,
800806
struct virtio_balloon, shrinker);
801807

802-
pages_to_free = sc->nr_to_scan * VIRTIO_BALLOON_PAGES_PER_PAGE;
808+
pages_to_free = sc->nr_to_scan;
803809

804810
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
805811
pages_freed = shrink_free_pages(vb, pages_to_free);

0 commit comments

Comments
 (0)