[PATCH v3] mm: page_reporting: allow driver to set batch capacity

[PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Michael S. Tsirkin]

At the moment, if a virtio balloon device has a page reporting vq but
its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
probe.

But, there's no way for host to know this value, so it can easily
create a smaller vq and suddenly adding the reporting capability
to the device makes all of the driver fail. Not pretty.

Add a capacity field to page_reporting_dev_info so drivers can
control the maximum number of pages per report batch.

In virtio-balloon, set the capacity to the reporting virtqueue size,
letting page_reporting adapt to whatever the device provides.

Capacity need not be a power of two.  Code previously called out
division by PAGE_REPORTING_CAPACITY as cheap since it was a power
of 2, but no performance difference was observed with non-power-of-2
values.

If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
page_reporting_register(), before the reporting work is scheduled,
so we never get division by 0.

Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
Assisted-by: Claude:claude-opus-4-6
Acked-by: David Hildenbrand (Arm) <david@kernel.org>
Reviewed-by: Gregory Price <gourry@gourry.net>
Acked-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Who's merging this? Me?

Changes in v3:
 - Dropped stale "This value should always be a power of 2" comment
   above PAGE_REPORTING_CAPACITY (David Hildenbrand, others)

Changes in v2:
 - Added paragraph explaining capacity need not be a power of two
 - Added paragraph documenting capacity=0 default and no div-by-zero
   (Gregory Price)
 - Improved struct field comment: "0 (default) means PAGE_REPORTING_CAPACITY"

 drivers/virtio/virtio_balloon.c |  5 +----
 include/linux/page_reporting.h  |  4 +++-
 mm/page_reporting.c             | 24 ++++++++++++------------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 088b3a0e6ce6..581ac799d974 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -1017,10 +1017,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		unsigned int capacity;
 
 		capacity = virtqueue_get_vring_size(vb->reporting_vq);
-		if (capacity < PAGE_REPORTING_CAPACITY) {
-			err = -ENOSPC;
-			goto out_unregister_oom;
-		}
 
 		vb->pr_dev_info.order = PAGE_REPORTING_ORDER_UNSPECIFIED;
 
@@ -1041,6 +1037,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		vb->pr_dev_info.order = 5;
 #endif
 
+		vb->pr_dev_info.capacity = capacity;
 		err = page_reporting_register(&vb->pr_dev_info);
 		if (err)
 			goto out_unregister_oom;
diff --git a/include/linux/page_reporting.h b/include/linux/page_reporting.h
index 9d4ca5c218a0..272b1274efdc 100644
--- a/include/linux/page_reporting.h
+++ b/include/linux/page_reporting.h
@@ -5,7 +5,6 @@
 #include <linux/mmzone.h>
 #include <linux/scatterlist.h>
 
-/* This value should always be a power of 2, see page_reporting_cycle() */
 #define PAGE_REPORTING_CAPACITY		32
 #define PAGE_REPORTING_ORDER_UNSPECIFIED	-1
 
@@ -22,6 +21,9 @@ struct page_reporting_dev_info {
 
 	/* Minimal order of page reporting */
 	unsigned int order;
+
+	/* Max pages per report batch; 0 (default) means PAGE_REPORTING_CAPACITY */
+	unsigned int capacity;
 };
 
 /* Tear-down and bring-up for page reporting devices */
diff --git a/mm/page_reporting.c b/mm/page_reporting.c
index 7418f2e500bb..942e84b6908a 100644
--- a/mm/page_reporting.c
+++ b/mm/page_reporting.c
@@ -173,11 +173,8 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
 	 * any pages that may have already been present from the previous
 	 * list processed. This should result in us reporting all pages on
 	 * an idle system in about 30 seconds.
-	 *
-	 * The division here should be cheap since PAGE_REPORTING_CAPACITY
-	 * should always be a power of 2.
 	 */
-	budget = DIV_ROUND_UP(area->nr_free, PAGE_REPORTING_CAPACITY * 16);
+	budget = DIV_ROUND_UP(area->nr_free, prdev->capacity * 16);
 
 	/* loop through free list adding unreported pages to sg list */
 	list_for_each_entry_safe(page, next, list, lru) {
@@ -222,10 +219,10 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
 		spin_unlock_irq(&zone->lock);
 
 		/* begin processing pages in local list */
-		err = prdev->report(prdev, sgl, PAGE_REPORTING_CAPACITY);
+		err = prdev->report(prdev, sgl, prdev->capacity);
 
 		/* reset offset since the full list was reported */
-		*offset = PAGE_REPORTING_CAPACITY;
+		*offset = prdev->capacity;
 
 		/* update budget to reflect call to report function */
 		budget--;
@@ -234,7 +231,7 @@ page_reporting_cycle(struct page_reporting_dev_info *prdev, struct zone *zone,
 		spin_lock_irq(&zone->lock);
 
 		/* flush reported pages from the sg list */
-		page_reporting_drain(prdev, sgl, PAGE_REPORTING_CAPACITY, !err);
+		page_reporting_drain(prdev, sgl, prdev->capacity, !err);
 
 		/*
 		 * Reset next to first entry, the old next isn't valid
@@ -260,13 +257,13 @@ static int
 page_reporting_process_zone(struct page_reporting_dev_info *prdev,
 			    struct scatterlist *sgl, struct zone *zone)
 {
-	unsigned int order, mt, leftover, offset = PAGE_REPORTING_CAPACITY;
+	unsigned int order, mt, leftover, offset = prdev->capacity;
 	unsigned long watermark;
 	int err = 0;
 
 	/* Generate minimum watermark to be able to guarantee progress */
 	watermark = low_wmark_pages(zone) +
-		    (PAGE_REPORTING_CAPACITY << page_reporting_order);
+		    (prdev->capacity << page_reporting_order);
 
 	/*
 	 * Cancel request if insufficient free memory or if we failed
@@ -290,7 +287,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev,
 	}
 
 	/* report the leftover pages before going idle */
-	leftover = PAGE_REPORTING_CAPACITY - offset;
+	leftover = prdev->capacity - offset;
 	if (leftover) {
 		sgl = &sgl[offset];
 		err = prdev->report(prdev, sgl, leftover);
@@ -322,11 +319,11 @@ static void page_reporting_process(struct work_struct *work)
 	atomic_set(&prdev->state, state);
 
 	/* allocate scatterlist to store pages being reported on */
-	sgl = kmalloc_objs(*sgl, PAGE_REPORTING_CAPACITY);
+	sgl = kmalloc_objs(*sgl, prdev->capacity);
 	if (!sgl)
 		goto err_out;
 
-	sg_init_table(sgl, PAGE_REPORTING_CAPACITY);
+	sg_init_table(sgl, prdev->capacity);
 
 	for_each_zone(zone) {
 		err = page_reporting_process_zone(prdev, sgl, zone);
@@ -377,6 +374,9 @@ int page_reporting_register(struct page_reporting_dev_info *prdev)
 			page_reporting_order = pageblock_order;
 	}
 
+	if (!prdev->capacity || prdev->capacity > PAGE_REPORTING_CAPACITY)
+		prdev->capacity = PAGE_REPORTING_CAPACITY;
+
 	/* initialize state and work structures */
 	atomic_set(&prdev->state, PAGE_REPORTING_IDLE);
 	INIT_DELAYED_WORK(&prdev->work, &page_reporting_process);
-- 
MST

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Andrew Morton]

On Sun, 5 Jul 2026 02:25:13 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:

> At the moment, if a virtio balloon device has a page reporting vq but
> its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> probe.
> 
> But, there's no way for host to know this value, so it can easily
> create a smaller vq and suddenly adding the reporting capability
> to the device makes all of the driver fail. Not pretty.
> 
> Add a capacity field to page_reporting_dev_info so drivers can
> control the maximum number of pages per report batch.
> 
> In virtio-balloon, set the capacity to the reporting virtqueue size,
> letting page_reporting adapt to whatever the device provides.
> 
> Capacity need not be a power of two.  Code previously called out
> division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> of 2, but no performance difference was observed with non-power-of-2
> values.
> 
> If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> page_reporting_register(), before the reporting work is scheduled,
> so we never get division by 0.

Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?

It seems that Sashiko has found a pre-existing issue, a hard-to-hit
error path thing:

	https://sashiko.dev/#/patchset/444c24cf39f3f3620fc90ef4695bd6b0979f4c4b.1783232420.git.mst@redhat.com

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Michael S. Tsirkin]

On Sun, Jul 05, 2026 at 01:16:35AM -0700, Andrew Morton wrote:
> On Sun, 5 Jul 2026 02:25:13 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > At the moment, if a virtio balloon device has a page reporting vq but
> > its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> > probe.
> > 
> > But, there's no way for host to know this value, so it can easily
> > create a smaller vq and suddenly adding the reporting capability
> > to the device makes all of the driver fail. Not pretty.
> > 
> > Add a capacity field to page_reporting_dev_info so drivers can
> > control the maximum number of pages per report batch.
> > 
> > In virtio-balloon, set the capacity to the reporting virtqueue size,
> > letting page_reporting adapt to whatever the device provides.
> > 
> > Capacity need not be a power of two.  Code previously called out
> > division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> > of 2, but no performance difference was observed with non-power-of-2
> > values.
> > 
> > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > page_reporting_register(), before the reporting work is scheduled,
> > so we never get division by 0.
> 
> Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?

If possible, I'd like it in 7.2, have a setup like this (small
queue is useful for perf testing).
It's very early in the cycle and the code seems straight forward...
I judge the chances of breaking anything is tiny because
it just failed probe previously.

> It seems that Sashiko has found a pre-existing issue, a hard-to-hit
> error path thing:
> 
> 	https://sashiko.dev/#/patchset/444c24cf39f3f3620fc90ef4695bd6b0979f4c4b.1783232420.git.mst@redhat.com
>

will look into this, thanks!

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Andrew Morton]

On Sun, 5 Jul 2026 04:25:53 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:

> > > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > > page_reporting_register(), before the reporting work is scheduled,
> > > so we never get division by 0.
> > 
> > Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?
> 
> If possible, I'd like it in 7.2, have a setup like this (small
> queue is useful for perf testing).
> It's very early in the cycle and the code seems straight forward...
> I judge the chances of breaking anything is tiny because
> it just failed probe previously.

No probs, I moved this to the the mm-hotfixes-unstable queue.  No cc:stable.

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Michael S. Tsirkin]

On Sun, Jul 05, 2026 at 01:39:56AM -0700, Andrew Morton wrote:
> On Sun, 5 Jul 2026 04:25:53 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > > > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > > > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > > > page_reporting_register(), before the reporting work is scheduled,
> > > > so we never get division by 0.
> > > 
> > > Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?
> > 
> > If possible, I'd like it in 7.2, have a setup like this (small
> > queue is useful for perf testing).
> > It's very early in the cycle and the code seems straight forward...
> > I judge the chances of breaking anything is tiny because
> > it just failed probe previously.
> 
> No probs, I moved this to the the mm-hotfixes-unstable queue.  No cc:stable.

I suspect the Fixes tag will make their AI pick it anyway. But hey.

-- 
MST

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Andrew Morton]

On Sun, 5 Jul 2026 04:42:38 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Sun, Jul 05, 2026 at 01:39:56AM -0700, Andrew Morton wrote:
> > On Sun, 5 Jul 2026 04:25:53 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > > > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > > > > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > > > > page_reporting_register(), before the reporting work is scheduled,
> > > > > so we never get division by 0.
> > > > 
> > > > Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?
> > > 
> > > If possible, I'd like it in 7.2, have a setup like this (small
> > > queue is useful for perf testing).
> > > It's very early in the cycle and the code seems straight forward...
> > > I judge the chances of breaking anything is tiny because
> > > it just failed probe previously.
> > 
> > No probs, I moved this to the the mm-hotfixes-unstable queue.  No cc:stable.
> 
> I suspect the Fixes tag will make their AI pick it anyway. But hey.

-stable maintainers have been asked not to do that - only backport
patches which have an explicit cc:stable.

So should I add one?

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Michael S. Tsirkin]

On Sun, Jul 05, 2026 at 01:51:16AM -0700, Andrew Morton wrote:
> On Sun, 5 Jul 2026 04:42:38 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Sun, Jul 05, 2026 at 01:39:56AM -0700, Andrew Morton wrote:
> > > On Sun, 5 Jul 2026 04:25:53 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > > > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > > > > > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > > > > > page_reporting_register(), before the reporting work is scheduled,
> > > > > > so we never get division by 0.
> > > > > 
> > > > > Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?
> > > > 
> > > > If possible, I'd like it in 7.2, have a setup like this (small
> > > > queue is useful for perf testing).
> > > > It's very early in the cycle and the code seems straight forward...
> > > > I judge the chances of breaking anything is tiny because
> > > > it just failed probe previously.
> > > 
> > > No probs, I moved this to the the mm-hotfixes-unstable queue.  No cc:stable.
> > 
> > I suspect the Fixes tag will make their AI pick it anyway. But hey.
> 
> -stable maintainers have been asked not to do that - only backport
> patches which have an explicit cc:stable.
> 
> So should I add one?

The "scratch my own itch, I want my hardware to work ASAP" part of me wants
to say yes. But the "decades of experience taught me I'm fallible" part
says let's have this out in the field for a while first. I had time
for coffee, and I'm dr Jekyll now. So no, thanks!

-- 
MST

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Michael S. Tsirkin]

On Sun, Jul 05, 2026 at 01:51:16AM -0700, Andrew Morton wrote:
> On Sun, 5 Jul 2026 04:42:38 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Sun, Jul 05, 2026 at 01:39:56AM -0700, Andrew Morton wrote:
> > > On Sun, 5 Jul 2026 04:25:53 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > > > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > > > > > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > > > > > page_reporting_register(), before the reporting work is scheduled,
> > > > > > so we never get division by 0.
> > > > > 
> > > > > Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?
> > > > 
> > > > If possible, I'd like it in 7.2, have a setup like this (small
> > > > queue is useful for perf testing).
> > > > It's very early in the cycle and the code seems straight forward...
> > > > I judge the chances of breaking anything is tiny because
> > > > it just failed probe previously.
> > > 
> > > No probs, I moved this to the the mm-hotfixes-unstable queue.  No cc:stable.
> > 
> > I suspect the Fixes tag will make their AI pick it anyway. But hey.
> 
> -stable maintainers have been asked not to do that - only backport
> patches which have an explicit cc:stable.

Out of curiousity, is this specific to -mm?

-- 
MST

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Andrew Morton]

On Sun, 5 Jul 2026 05:06:27 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:

> > -stable maintainers have been asked not to do that - only backport
> > patches which have an explicit cc:stable.
> 
> Out of curiousity, is this specific to -mm?

Not sure.  I have vague memories of being told "we added MM to the
list", but perhaps I made that up.

Sasha, it MM the only subsystem which requires explicit cc:stable
for patches to be backported?

Thanks.

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Sasha Levin]

On Sun, Jul 05, 2026 at 11:06:49AM -0700, Andrew Morton wrote:
>On Sun, 5 Jul 2026 05:06:27 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> > -stable maintainers have been asked not to do that - only backport
>> > patches which have an explicit cc:stable.
>>
>> Out of curiousity, is this specific to -mm?
>
>Not sure.  I have vague memories of being told "we added MM to the
>list", but perhaps I made that up.
>
>Sasha, it MM the only subsystem which requires explicit cc:stable
>for patches to be backported?

There are a few more. We maintain the list here:

	https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/ignore_list

-- 
Thanks,
Sasha

Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity

[Michael S. Tsirkin]

On Sun, Jul 05, 2026 at 01:16:35AM -0700, Andrew Morton wrote:
> On Sun, 5 Jul 2026 02:25:13 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > At the moment, if a virtio balloon device has a page reporting vq but
> > its size is < PAGE_REPORTING_CAPACITY (32), the balloon driver fails
> > probe.
> > 
> > But, there's no way for host to know this value, so it can easily
> > create a smaller vq and suddenly adding the reporting capability
> > to the device makes all of the driver fail. Not pretty.
> > 
> > Add a capacity field to page_reporting_dev_info so drivers can
> > control the maximum number of pages per report batch.
> > 
> > In virtio-balloon, set the capacity to the reporting virtqueue size,
> > letting page_reporting adapt to whatever the device provides.
> > 
> > Capacity need not be a power of two.  Code previously called out
> > division by PAGE_REPORTING_CAPACITY as cheap since it was a power
> > of 2, but no performance difference was observed with non-power-of-2
> > values.
> > 
> > If capacity is 0 or exceeds PAGE_REPORTING_CAPACITY, it defaults
> > to PAGE_REPORTING_CAPACITY.  The 0 check and the clamping is done in
> > page_reporting_register(), before the reporting work is scheduled,
> > so we never get division by 0.
> 
> Thanks.  What's the priority here?  Should we fix 7.2?  Earlier?
> 
> It seems that Sashiko has found a pre-existing issue, a hard-to-hit
> error path thing:
> 
> 	https://sashiko.dev/#/patchset/444c24cf39f3f3620fc90ef4695bd6b0979f4c4b.1783232420.git.mst@redhat.com
> 

Ugh. Yes but it is a minor symptom actually(

This is the root cause:

        virtio_device_ready(vdev);

        if (towards_target(vb))
                virtballoon_changed(vdev);
        return 0;

DRIVER_OK set almost the last thing. But, e.g.:



        if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
                struct scatterlist sg;
                unsigned int num_stats;
                vb->stats_vq = vqs[VIRTIO_BALLOON_VQ_STATS];

                /*
                 * Prime this virtqueue with one buffer so the hypervisor can
                 * use it to signal us later (it can't be broken yet!).
                 */
                num_stats = update_balloon_stats(vb);

                sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
                err = virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb,
                                           GFP_KERNEL);
                if (err) {
                        dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n",
                                 __func__);
                        return err;
                }
                virtqueue_kick(vb->stats_vq);
        }

this happens before DRIVER_OK and it's quite out of spec.

All of balloon initialization needs to be rethought and fixed.
Maybe.
After coffee.



-- 
MST