* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Andrew Morton @ 2026-07-05 8:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <444c24cf39f3f3620fc90ef4695bd6b0979f4c4b.1783232420.git.mst@redhat.com>
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
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-07-05 8:25 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705011635.9974cac7f0defec9961e85c4@linux-foundation.org>
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!
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-07-05 8:34 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705011635.9974cac7f0defec9961e85c4@linux-foundation.org>
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
^ permalink raw reply
* Re: [PATCH] vdpa: octeon_ep: add missing MODULE_DEVICE_TABLE()
From: Michael S. Tsirkin @ 2026-07-05 8:35 UTC (permalink / raw)
To: Pengpeng Hou
Cc: schalla, vattunuru, Jason Wang, Xuan Zhuo, Eugenio Pérez,
virtualization, linux-kernel
In-Reply-To: <20260705002546.85004-1-pengpeng@iscas.ac.cn>
On Sun, Jul 05, 2026 at 08:25:46AM +0800, Pengpeng Hou wrote:
> The driver has a match table for the pci bus wired into its driver
> structure, but the table is not exported with MODULE_DEVICE_TABLE().
>
> Add the missing MODULE_DEVICE_TABLE() entry so module alias information
> is generated for automatic module loading.
>
> This is a source-level fix. It does not claim dynamic hardware
> reproduction; the evidence is the driver-owned match table, its use by
> the driver registration structure, and the missing module alias
> publication.
what does this paragraph mean?
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> drivers/vdpa/octeon_ep/octep_vdpa_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> index 31a02e7fd7f2..9bedcd81a174 100644
> --- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> +++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> @@ -884,6 +884,7 @@ static struct pci_device_id octep_pci_vdpa_map[] = {
> { PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_VDPA_DEVID_CN103K_VF) },
> { 0 },
> };
> +MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map);
>
> static struct pci_driver octep_pci_vdpa = {
> .name = OCTEP_VDPA_DRIVER_NAME,
> --
> 2.53.0
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Andrew Morton @ 2026-07-05 8:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705042313-mutt-send-email-mst@kernel.org>
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.
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-07-05 8:42 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705013956.36f649b5cb13409d9088838a@linux-foundation.org>
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
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Andrew Morton @ 2026-07-05 8:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705044139-mutt-send-email-mst@kernel.org>
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?
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-07-05 9:03 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705015116.bdf3335e9a955f057a6d9c5d@linux-foundation.org>
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
^ permalink raw reply
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-07-05 9:06 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
In-Reply-To: <20260705015116.bdf3335e9a955f057a6d9c5d@linux-foundation.org>
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
^ permalink raw reply
* [PATCH] virtio_balloon: prime stats vq after virtio_device_ready()
From: Michael S. Tsirkin @ 2026-07-05 12:29 UTC (permalink / raw)
To: linux-kernel
Cc: Adam Litke, David Hildenbrand, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Rusty Russell, virtualization, Andrew Morton,
linux-mm
The virtio spec requires the driver not to kick the device before
DRIVER_OK is set. init_vqs() primes the stats virtqueue with a buffer
and kicks the device before virtio_device_ready() is called in
virtballoon_probe(), violating this requirement.
Further, if the device responds to the early kick by processing the
buffer before DRIVER_OK, stats_request() fires and queues
update_balloon_stats_work. Should probe then fail and free vb, the work
runs against freed memory.
To fix, move buffer setup to after DRIVER_OK. Be careful to
disable update_balloon_stats_work while this is going on,
to make sure it does not race with the setup.
Testing: tested that stats still work after the change.
Fixes: 9564e138b1f6 ("virtio: Add memory statistics reporting to the balloon driver (V4)")
Reported-by: Sashiko:gemini-3.1-pro-preview
Cc: Adam Litke <agl@us.ibm.com>
Cc: David Hildenbrand <david@kernel.org>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Fixes a bug reported by Sashiko and pointed out to me by Andrew.
I guess I'll queue this myself.
drivers/virtio/virtio_balloon.c | 39 ++++++++++++++++++---------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 088b3a0e6ce6..d4cd2dd388e9 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -611,25 +611,8 @@ static int init_vqs(struct virtio_balloon *vb)
vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
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);
+ disable_work(&vb->update_balloon_stats_work);
}
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
@@ -916,6 +899,22 @@ static int virtio_balloon_register_shrinker(struct virtio_balloon *vb)
return 0;
}
+static void setup_vqs(struct virtio_balloon *vb)
+{
+ struct scatterlist sg;
+ unsigned int num_stats;
+
+ if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
+ return;
+
+ num_stats = update_balloon_stats(vb);
+ sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
+ if (!virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL))
+ virtqueue_kick(vb->stats_vq);
+ enable_and_queue_work(system_freezable_wq,
+ &vb->update_balloon_stats_work);
+}
+
static int virtballoon_probe(struct virtio_device *vdev)
{
struct virtio_balloon *vb;
@@ -1059,6 +1058,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
virtio_device_ready(vdev);
+ setup_vqs(vb);
+
if (towards_target(vb))
virtballoon_changed(vdev);
return 0;
@@ -1148,6 +1149,8 @@ static int virtballoon_restore(struct virtio_device *vdev)
virtio_device_ready(vdev);
+ setup_vqs(vb);
+
if (towards_target(vb))
virtballoon_changed(vdev);
update_balloon_size(vb);
--
MST
^ permalink raw reply related
* [PATCH] timekeeping: Document monotonic raw timestamps in snapshots correctly
From: Thomas Gleixner @ 2026-07-05 12:38 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: LKML, David Woodhouse, Miroslav Lichvar, John Stultz,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
Arthur Kiyanovski, Rodolfo Giometti, Vincent Donnefort,
Marc Zyngier, Oliver Upton, kvmarm, Oliver Upton, Richard Cochran,
netdev, Takashi Iwai, Miri Korenblit, Johannes Berg, Jacob Keller,
Tony Nguyen, Saeed Mahameed, Peter Hilber, Michael S. Tsirkin,
virtualization, linux-wireless, linux-sound
In-Reply-To: <87bjctsptt.ffs@fw13>
The comments related to raw monotonic timestamps for the various
snapshot mechanisms in code and struct documentation are ambiguous. They
reference them as CLOCK_MONOTONIC_RAW timestamps, but with the arrival
of AUX clocks that's not longer correct.
The raw monotonic timestamps only represent CLOCK_MONOTONIC_RAW for the
system time clock IDs, i.e. REALTIME, MONOTONIC, BOOTTIME, TAI.
For AUX clocks they refer to the monotonic raw clock which is related to
the individual AUX clocks. These monotonic raw timestamps have the same
conversion factor as CLOCK_MONOTONIC_RAW, but differ from that by an
offset:
MONORAW(AUX$N) = MONORAW(SYSTEM) + OFFSET(AUX$N)
The offset is established when a AUX clock is enabled and stays constant
for the lifetime of the AUX clock.
Update the comments so they reflect reality.
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reported-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
include/linux/timekeeping.h | 10 +++++++++-
kernel/time/timekeeping.c | 16 +++++++++++++++-
2 files changed, 24 insertions(+), 2 deletions(-)
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -276,7 +276,7 @@ static inline bool ktime_get_aux_ts64(cl
#endif
/**
- * struct system_time_snapshot - Simultaneous time capture of CLOCK_MONOTONIC_RAW,
+ * struct system_time_snapshot - Simultaneous time capture of monotonic raw time,
* a selected CLOCK_* and the clocksource counter value
* @cycles: Clocksource counter value to produce the system times
* @hw_cycles: For derived clocksources, the hardware counter value from
@@ -289,6 +289,10 @@ static inline bool ktime_get_aux_ts64(cl
* @clock_was_set_seq: The sequence number of clock-was-set events
* @cs_was_changed_seq: The sequence number of clocksource change events
* @valid: True if the snapshot is valid
+ *
+ * @monoraw is CLOCK_MONOTONIC_RAW for system time CLOCK ids. For CLOCK_AUX$N
+ * clock ids it's the monotonic raw time related to the AUX clock, which is
+ * CLOCK_MONOTONIC_RAW plus a AUX clock specific offset.
*/
struct system_time_snapshot {
u64 cycles;
@@ -326,6 +330,10 @@ struct system_counterval_t {
* @sys_counter: Clocksource counter value simultaneous with device time
* @sys_systime: System time for @clock_id
* @sys_monoraw: Monotonic raw simultaneous with device time
+ *
+ * @sys_monoraw is CLOCK_MONOTONIC_RAW for system time CLOCK ids. For
+ * CLOCK_AUX$N clock ids it's the monotonic raw time related to the AUX clock,
+ * which is CLOCK_MONOTONIC_RAW plus a AUX clock specific offset.
*/
struct system_device_crosststamp {
clockid_t clock_id;
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -1202,10 +1202,21 @@ static inline u64 tk_clock_read_snapshot
/**
* ktime_get_snapshot_id - Simultaneously snapshot a given clock ID with
- * CLOCK_MONOTONIC_RAW and the underlying
+ * the corresponding monotonic raw and the underlying
* clocksource counter value.
* @clock_id: The clock ID to snapshot
* @systime_snapshot: Pointer to struct receiving the system time snapshot
+ *
+ * For the system time keeping clocks (REALTIME, MONOTONIC and BOOTTIME) the
+ * monotonic raw clock is CLOCK_MONOTONIC_RAW. For AUX clocks this is the
+ * monotonic raw clock related to the AUX clock. These AUX clock related
+ * monotonic raw clocks have a strict linear offset to the system time
+ * CLOCK_MONOTONIC_RAW:
+ *
+ * MONOTONIC_RAW(AUX$N) = CLOCK_MONOTONIC_RAW(system) + offset(AUX$N)
+ *
+ * The offset is established when a AUX clock is initialized, but it is
+ * currently not accessible.
*/
void ktime_get_snapshot_id(clockid_t clock_id, struct system_time_snapshot *systime_snapshot)
{
@@ -1512,6 +1523,9 @@ EXPORT_SYMBOL_GPL(ktime_real_to_base_clo
* @xtstamp: Receives simultaneously captured system and device time
*
* Reads a timestamp from a device and correlates it to system time
+ *
+ * See documentation for ktime_get_snapshot_id() for information about the raw
+ * monotonic time stamp which is used here.
*/
int get_device_system_crosststamp(int (*get_time_fn)
(ktime_t *device_time,
^ permalink raw reply
* [PATCH] i2c: virtio: Reset virtqueue before freeing interrupted requests
From: Guangshuo Li @ 2026-07-05 15:59 UTC (permalink / raw)
To: Viresh Kumar, Chen, Jian Jun, Andi Shyti, linux-i2c,
virtualization, linux-kernel
Cc: Guangshuo Li
virtio_i2c_complete_reqs() waits for each queued request to complete
before releasing the per-message DMA-safe buffer. The request array is
then freed by virtio_i2c_xfer().
After wait_for_completion_interruptible() was introduced, the wait can
return before the current request has completed. In that case, the
request and the remaining requests may still be owned by the virtqueue
and may still be completed by the device later.
The error path nevertheless continues to release each message buffer and
virtio_i2c_xfer() frees the request array. A later device completion can
then write to freed request memory or call complete() on a freed
completion object.
Reset the virtqueue when the wait is interrupted so that outstanding
buffers are recovered before the request array and message buffers are
freed.
Fixes: a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible completion wait")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/i2c/busses/i2c-virtio.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
index 5da6fef92bec..6ef09eda3ed0 100644
--- a/drivers/i2c/busses/i2c-virtio.c
+++ b/drivers/i2c/busses/i2c-virtio.c
@@ -117,12 +117,19 @@ static int virtio_i2c_complete_reqs(struct virtqueue *vq,
struct virtio_i2c_req *req = &reqs[i];
if (!failed) {
- if (wait_for_completion_interruptible(&req->completion))
+ if (wait_for_completion_interruptible(&req->completion)) {
+ /*
+ * The remaining buffers may still be owned by
+ * the device. Reset the virtqueue before freeing
+ * the request array and message buffers.
+ */
+ virtqueue_reset(vq, NULL, NULL);
failed = true;
- else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
+ } else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
failed = true;
- else
+ } else {
j++;
+ }
}
i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Andrew Morton @ 2026-07-05 18:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Hildenbrand (Arm), Gregory Price, Zi Yan,
Pankaj Gupta, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm,
Sasha Levin
In-Reply-To: <20260705050445-mutt-send-email-mst@kernel.org>
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.
^ permalink raw reply
* [PATCH v2] virtio_balloon: prime stats vq after virtio_device_ready()
From: Michael S. Tsirkin @ 2026-07-05 19:12 UTC (permalink / raw)
To: linux-kernel
Cc: David Hildenbrand, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Adam Litke, Rusty Russell, virtualization, Andrew Morton,
linux-mm
The virtio spec requires the driver not to kick the device before
DRIVER_OK is set. init_vqs() primes the stats virtqueue with a buffer
and kicks the device before virtio_device_ready() is called in
virtballoon_probe(), violating this requirement.
Further, if the device responds to the early kick by processing the
buffer before DRIVER_OK, stats_request() fires and queues
update_balloon_stats_work. Should probe then fail and free vb, the work
runs against freed memory.
To fix, move buffer setup to after DRIVER_OK. Be careful to
disable update_balloon_stats_work while this is going on,
to make sure it does not race with the setup.
setup_vqs() warns but does not fail probe or restore if
virtqueue_add_outbuf() fails; the call never actually fails in these
contexts since the queue is freshly initialized and empty.
Testing: tested that stats still work after the change.
Fixes: 9564e138b1f6 ("virtio: Add memory statistics reporting to the balloon driver (V4)")
Reported-by: Sashiko:gemini-3.1-pro-preview
Cc: David Hildenbrand <david@kernel.org>
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
changes from v1:
check that work enable/disable is balanced
explain how add buf never fails in probe/restore
drivers/virtio/virtio_balloon.c | 50 +++++++++++++++++++++------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 088b3a0e6ce6..bc0a2b19ca7d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -611,25 +611,8 @@ static int init_vqs(struct virtio_balloon *vb)
vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
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);
+ disable_work(&vb->update_balloon_stats_work);
}
if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
@@ -916,6 +899,33 @@ static int virtio_balloon_register_shrinker(struct virtio_balloon *vb)
return 0;
}
+static void setup_vqs(struct virtio_balloon *vb)
+{
+ struct scatterlist sg;
+ unsigned int num_stats;
+ bool ret;
+
+ if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
+ return;
+
+ /*
+ * 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);
+ if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)) {
+ dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n", __func__);
+ return;
+ }
+ virtqueue_kick(vb->stats_vq);
+
+ ret = enable_and_queue_work(system_freezable_wq,
+ &vb->update_balloon_stats_work);
+ /* Make sure we balanced enable/disable, or we won't report stats. */
+ BUG_ON(!ret);
+}
+
static int virtballoon_probe(struct virtio_device *vdev)
{
struct virtio_balloon *vb;
@@ -1059,6 +1069,8 @@ static int virtballoon_probe(struct virtio_device *vdev)
virtio_device_ready(vdev);
+ setup_vqs(vb);
+
if (towards_target(vb))
virtballoon_changed(vdev);
return 0;
@@ -1148,6 +1160,8 @@ static int virtballoon_restore(struct virtio_device *vdev)
virtio_device_ready(vdev);
+ setup_vqs(vb);
+
if (towards_target(vb))
virtballoon_changed(vdev);
update_balloon_size(vb);
--
MST
^ permalink raw reply related
* Re: [PATCH] i2c: virtio: Reset virtqueue before freeing interrupted requests
From: Viresh Kumar @ 2026-07-06 4:08 UTC (permalink / raw)
To: Guangshuo Li, Gavin Li
Cc: Chen, Jian Jun, Andi Shyti, linux-i2c, virtualization,
linux-kernel
In-Reply-To: <20260705155925.292765-1-lgs201920130244@gmail.com>
On 05-07-26, 23:59, Guangshuo Li wrote:
> virtio_i2c_complete_reqs() waits for each queued request to complete
> before releasing the per-message DMA-safe buffer. The request array is
> then freed by virtio_i2c_xfer().
>
> After wait_for_completion_interruptible() was introduced, the wait can
> return before the current request has completed. In that case, the
> request and the remaining requests may still be owned by the virtqueue
> and may still be completed by the device later.
>
> The error path nevertheless continues to release each message buffer and
> virtio_i2c_xfer() frees the request array. A later device completion can
> then write to freed request memory or call complete() on a freed
> completion object.
>
> Reset the virtqueue when the wait is interrupted so that outstanding
> buffers are recovered before the request array and message buffers are
> freed.
>
> Fixes: a663b3c47ab1 ("i2c: virtio: Avoid hang by using interruptible completion wait")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/i2c/busses/i2c-virtio.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-virtio.c b/drivers/i2c/busses/i2c-virtio.c
> index 5da6fef92bec..6ef09eda3ed0 100644
> --- a/drivers/i2c/busses/i2c-virtio.c
> +++ b/drivers/i2c/busses/i2c-virtio.c
> @@ -117,12 +117,19 @@ static int virtio_i2c_complete_reqs(struct virtqueue *vq,
> struct virtio_i2c_req *req = &reqs[i];
>
> if (!failed) {
> - if (wait_for_completion_interruptible(&req->completion))
> + if (wait_for_completion_interruptible(&req->completion)) {
> + /*
> + * The remaining buffers may still be owned by
> + * the device. Reset the virtqueue before freeing
> + * the request array and message buffers.
> + */
> + virtqueue_reset(vq, NULL, NULL);
> failed = true;
> - else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK)
> + } else if (req->in_hdr.status != VIRTIO_I2C_MSG_OK) {
> failed = true;
> - else
> + } else {
> j++;
> + }
> }
>
> i2c_put_dma_safe_msg_buf(reqs[i].buf, &msgs[i], !failed);
+ Gavin, who was also trying to solve this path recently.
https://lore.kernel.org/all/20260610155834.79207-1-gavin.li@samsara.com/
--
viresh
^ permalink raw reply
* [PATCH] vdpa/mlx5: fix wrong MLX5_ADDR_OF struct type in alloc_inout()
From: lirongqing @ 2026-07-06 6:09 UTC (permalink / raw)
To: Dragos Tatulea, Michael S . Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Cindy Lu, Kees Cook, virtualization,
linux-kernel
Cc: Li RongQing
From: Li RongQing <lirongqing@baidu.com>
In alloc_inout(), the qpc field offset was computed using
MLX5_ADDR_OF(rst2init_qp_in, ...) in both the INIT2RTR_QP and
RTR2RTS_QP cases. This is a copy-paste error: each case should use
its own input structure type to get the correct qpc offset.
Fix the INIT2RTR_QP case to use MLX5_ADDR_OF(init2rtr_qp_in, ...)
and the RTR2RTS_QP case to use MLX5_ADDR_OF(rtr2rts_qp_in, ...).
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index ad0d5fbb..eb431a1 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1080,7 +1080,7 @@ static void alloc_inout(struct mlx5_vdpa_net *ndev, int cmd, void **in, int *inl
MLX5_SET(init2rtr_qp_in, *in, opcode, cmd);
MLX5_SET(init2rtr_qp_in, *in, uid, ndev->mvdev.res.uid);
MLX5_SET(init2rtr_qp_in, *in, qpn, qpn);
- qpc = MLX5_ADDR_OF(rst2init_qp_in, *in, qpc);
+ qpc = MLX5_ADDR_OF(init2rtr_qp_in, *in, qpc);
MLX5_SET(qpc, qpc, mtu, MLX5_QPC_MTU_256_BYTES);
MLX5_SET(qpc, qpc, log_msg_max, 30);
MLX5_SET(qpc, qpc, remote_qpn, rqpn);
@@ -1098,7 +1098,7 @@ static void alloc_inout(struct mlx5_vdpa_net *ndev, int cmd, void **in, int *inl
MLX5_SET(rtr2rts_qp_in, *in, opcode, cmd);
MLX5_SET(rtr2rts_qp_in, *in, uid, ndev->mvdev.res.uid);
MLX5_SET(rtr2rts_qp_in, *in, qpn, qpn);
- qpc = MLX5_ADDR_OF(rst2init_qp_in, *in, qpc);
+ qpc = MLX5_ADDR_OF(rtr2rts_qp_in, *in, qpc);
pp = MLX5_ADDR_OF(qpc, qpc, primary_address_path);
MLX5_SET(ads, pp, ack_timeout, 14);
MLX5_SET(qpc, qpc, retry_count, 7);
--
2.9.4
^ permalink raw reply related
* Re: [PATCH] vdpa/mlx5: fix wrong MLX5_ADDR_OF struct type in alloc_inout()
From: Dragos Tatulea @ 2026-07-06 7:09 UTC (permalink / raw)
To: lirongqing, Michael S . Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Cindy Lu, Kees Cook, virtualization,
linux-kernel
In-Reply-To: <20260706060902.2341-1-lirongqing@baidu.com>
On 06.07.26 08:09, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
>
> In alloc_inout(), the qpc field offset was computed using
> MLX5_ADDR_OF(rst2init_qp_in, ...) in both the INIT2RTR_QP and
> RTR2RTS_QP cases. This is a copy-paste error: each case should use
> its own input structure type to get the correct qpc offset.
>
> Fix the INIT2RTR_QP case to use MLX5_ADDR_OF(init2rtr_qp_in, ...)
> and the RTR2RTS_QP case to use MLX5_ADDR_OF(rtr2rts_qp_in, ...).
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index ad0d5fbb..eb431a1 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -1080,7 +1080,7 @@ static void alloc_inout(struct mlx5_vdpa_net *ndev, int cmd, void **in, int *inl
> MLX5_SET(init2rtr_qp_in, *in, opcode, cmd);
> MLX5_SET(init2rtr_qp_in, *in, uid, ndev->mvdev.res.uid);
> MLX5_SET(init2rtr_qp_in, *in, qpn, qpn);
> - qpc = MLX5_ADDR_OF(rst2init_qp_in, *in, qpc);
> + qpc = MLX5_ADDR_OF(init2rtr_qp_in, *in, qpc);
> MLX5_SET(qpc, qpc, mtu, MLX5_QPC_MTU_256_BYTES);
> MLX5_SET(qpc, qpc, log_msg_max, 30);
> MLX5_SET(qpc, qpc, remote_qpn, rqpn);
> @@ -1098,7 +1098,7 @@ static void alloc_inout(struct mlx5_vdpa_net *ndev, int cmd, void **in, int *inl
> MLX5_SET(rtr2rts_qp_in, *in, opcode, cmd);
> MLX5_SET(rtr2rts_qp_in, *in, uid, ndev->mvdev.res.uid);
> MLX5_SET(rtr2rts_qp_in, *in, qpn, qpn);
> - qpc = MLX5_ADDR_OF(rst2init_qp_in, *in, qpc);
> + qpc = MLX5_ADDR_OF(rtr2rts_qp_in, *in, qpc);
> pp = MLX5_ADDR_OF(qpc, qpc, primary_address_path);
> MLX5_SET(ads, pp, ack_timeout, 14);
> MLX5_SET(qpc, qpc, retry_count, 7);
Thanks for the catch. Luckily those structs have the same layout.
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Thanks,
Dragos
^ permalink raw reply
* Re: [PATCH v2] virtio_balloon: prime stats vq after virtio_device_ready()
From: David Hildenbrand (Arm) @ 2026-07-06 7:28 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Adam Litke,
Rusty Russell, virtualization, Andrew Morton, linux-mm
In-Reply-To: <2f0d0de033f3005d3b985883be3e5cf37b3f6c42.1783278596.git.mst@redhat.com>
On 7/5/26 21:12, Michael S. Tsirkin wrote:
> The virtio spec requires the driver not to kick the device before
> DRIVER_OK is set. init_vqs() primes the stats virtqueue with a buffer
> and kicks the device before virtio_device_ready() is called in
> virtballoon_probe(), violating this requirement.
>
> Further, if the device responds to the early kick by processing the
> buffer before DRIVER_OK, stats_request() fires and queues
> update_balloon_stats_work. Should probe then fail and free vb, the work
> runs against freed memory.
>
> To fix, move buffer setup to after DRIVER_OK. Be careful to
> disable update_balloon_stats_work while this is going on,
> to make sure it does not race with the setup.
>
> setup_vqs() warns but does not fail probe or restore if
> virtqueue_add_outbuf() fails; the call never actually fails in these
> contexts since the queue is freshly initialized and empty.
>
> Testing: tested that stats still work after the change.
>
> Fixes: 9564e138b1f6 ("virtio: Add memory statistics reporting to the balloon driver (V4)")
> Reported-by: Sashiko:gemini-3.1-pro-preview
> Cc: David Hildenbrand <david@kernel.org>
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> changes from v1:
> check that work enable/disable is balanced
> explain how add buf never fails in probe/restore
>
> drivers/virtio/virtio_balloon.c | 50 +++++++++++++++++++++------------
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 088b3a0e6ce6..bc0a2b19ca7d 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -611,25 +611,8 @@ static int init_vqs(struct virtio_balloon *vb)
> vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
> vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> 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);
> + disable_work(&vb->update_balloon_stats_work);
That's to stop the stats queue triggering stats_request() I assume? Is that
valid before we actually added+kicked ourselves?
Also, can't we simply handle that in stats_request(), just ignoring it there?
Then we wouldn't have to go through the siable + enable.
> }
>
> if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> @@ -916,6 +899,33 @@ static int virtio_balloon_register_shrinker(struct virtio_balloon *vb)
> return 0;
> }
>
> +static void setup_vqs(struct virtio_balloon *vb)
> +{
> + struct scatterlist sg;
> + unsigned int num_stats;
> + bool ret;
> +
> + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
> + return;
> +
> + /*
> + * 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);
> + if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)) {
> + dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n", __func__);
> + return;
> + }
> + virtqueue_kick(vb->stats_vq);
> +
> + ret = enable_and_queue_work(system_freezable_wq,
> + &vb->update_balloon_stats_work);
> + /* Make sure we balanced enable/disable, or we won't report stats. */
> + BUG_ON(!ret);
A WARN_ON_ONCE() should be good enough here, no need to crash the kernel (no new
BUG_ON's).
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2] virtio_balloon: prime stats vq after virtio_device_ready()
From: Michael S. Tsirkin @ 2026-07-06 8:23 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: linux-kernel, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Adam Litke, Rusty Russell, virtualization, Andrew Morton,
linux-mm
In-Reply-To: <13794c5e-375a-4fc5-84b4-a637d077f8a0@kernel.org>
On Mon, Jul 06, 2026 at 09:28:50AM +0200, David Hildenbrand (Arm) wrote:
> On 7/5/26 21:12, Michael S. Tsirkin wrote:
> > The virtio spec requires the driver not to kick the device before
> > DRIVER_OK is set. init_vqs() primes the stats virtqueue with a buffer
> > and kicks the device before virtio_device_ready() is called in
> > virtballoon_probe(), violating this requirement.
> >
> > Further, if the device responds to the early kick by processing the
> > buffer before DRIVER_OK, stats_request() fires and queues
> > update_balloon_stats_work. Should probe then fail and free vb, the work
> > runs against freed memory.
> >
> > To fix, move buffer setup to after DRIVER_OK. Be careful to
> > disable update_balloon_stats_work while this is going on,
> > to make sure it does not race with the setup.
> >
> > setup_vqs() warns but does not fail probe or restore if
> > virtqueue_add_outbuf() fails; the call never actually fails in these
> > contexts since the queue is freshly initialized and empty.
> >
> > Testing: tested that stats still work after the change.
> >
> > Fixes: 9564e138b1f6 ("virtio: Add memory statistics reporting to the balloon driver (V4)")
> > Reported-by: Sashiko:gemini-3.1-pro-preview
> > Cc: David Hildenbrand <david@kernel.org>
> > Assisted-by: Claude:claude-sonnet-4-6
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > changes from v1:
> > check that work enable/disable is balanced
> > explain how add buf never fails in probe/restore
> >
> > drivers/virtio/virtio_balloon.c | 50 +++++++++++++++++++++------------
> > 1 file changed, 32 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 088b3a0e6ce6..bc0a2b19ca7d 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -611,25 +611,8 @@ static int init_vqs(struct virtio_balloon *vb)
> > vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
> > vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> > 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);
> > + disable_work(&vb->update_balloon_stats_work);
>
> That's to stop the stats queue triggering stats_request() I assume?
Yes.
> Is that
> valid before we actually added+kicked ourselves?
Why not?
> Also, can't we simply handle that in stats_request(), just ignoring it there?
Then we'd need to maintain a special flag and I dislike that.
> Then we wouldn't have to go through the siable + enable.
We'd still need to flip the flag on/off.
> > }
> >
> > if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
> > @@ -916,6 +899,33 @@ static int virtio_balloon_register_shrinker(struct virtio_balloon *vb)
> > return 0;
> > }
> >
> > +static void setup_vqs(struct virtio_balloon *vb)
> > +{
> > + struct scatterlist sg;
> > + unsigned int num_stats;
> > + bool ret;
> > +
> > + if (!virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
> > + return;
> > +
> > + /*
> > + * 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);
> > + if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)) {
> > + dev_warn(&vb->vdev->dev, "%s: add stat_vq failed\n", __func__);
> > + return;
> > + }
> > + virtqueue_kick(vb->stats_vq);
> > +
> > + ret = enable_and_queue_work(system_freezable_wq,
> > + &vb->update_balloon_stats_work);
> > + /* Make sure we balanced enable/disable, or we won't report stats. */
> > + BUG_ON(!ret);
>
> A WARN_ON_ONCE() should be good enough here, no need to crash the kernel (no new
> BUG_ON's).
Will do, thanks!
> --
> Cheers,
>
> David
^ permalink raw reply
* [PATCH] iommu/virtio: reject short event buffers
From: raoxu @ 2026-07-06 8:31 UTC (permalink / raw)
To: jpb; +Cc: joro, will, robin.murphy, virtualization, iommu, linux-kernel,
raoxu
From: Xu Rao <raoxu@uniontech.com>
The event queue uses fixed-size buffers for struct viommu_event. The
device-reported used length is currently only checked for oversized
buffers, so a short used buffer can still be passed to the fault
handler.
In that case the handler parses fields that were not written by the
device for this event, potentially using stale data from a previous
event and reporting a bogus fault.
Reject any event buffer whose used length is not exactly the expected
event size. Still recycle the buffer back to the event queue so a bad
event does not permanently shrink the queue.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
drivers/iommu/virtio-iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 587fc13197f1..3ace4a6dd02a 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -635,7 +635,7 @@ static void viommu_event_handler(struct virtqueue *vq)
struct viommu_dev *viommu = vq->vdev->priv;
while ((evt = virtqueue_get_buf(vq, &len)) != NULL) {
- if (len > sizeof(*evt)) {
+ if (len != sizeof(*evt)) {
dev_err(viommu->dev,
"invalid event buffer (len %u != %zu)\n",
len, sizeof(*evt));
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2] virtio_balloon: prime stats vq after virtio_device_ready()
From: David Hildenbrand (Arm) @ 2026-07-06 8:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Adam Litke, Rusty Russell, virtualization, Andrew Morton,
linux-mm
In-Reply-To: <20260706042207-mutt-send-email-mst@kernel.org>
>>> - virtqueue_kick(vb->stats_vq);
>>> + disable_work(&vb->update_balloon_stats_work);
>>
>> That's to stop the stats queue triggering stats_request() I assume?
>
> Yes.
>
>> Is that
>> valid before we actually added+kicked ourselves?
>
> Why not?
I was wondering whether the spec would state something about that.
>
>> Also, can't we simply handle that in stats_request(), just ignoring it there?
>
> Then we'd need to maintain a special flag and I dislike that.
I was hoping that we could use the device-ready indication, but there would be a
small race window indeed.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] timekeeping: Document monotonic raw timestamps in snapshots correctly
From: Thomas Weißschuh @ 2026-07-06 8:42 UTC (permalink / raw)
To: Thomas Gleixner
Cc: LKML, David Woodhouse, Miroslav Lichvar, John Stultz,
Stephen Boyd, Anna-Maria Behnsen, Frederic Weisbecker,
Arthur Kiyanovski, Rodolfo Giometti, Vincent Donnefort,
Marc Zyngier, Oliver Upton, kvmarm, Oliver Upton, Richard Cochran,
netdev, Takashi Iwai, Miri Korenblit, Johannes Berg, Jacob Keller,
Tony Nguyen, Saeed Mahameed, Peter Hilber, Michael S. Tsirkin,
virtualization, linux-wireless, linux-sound
In-Reply-To: <87wlv9k3wz.ffs@fw13>
On Sun, Jul 05, 2026 at 02:38:04PM +0200, Thomas Gleixner wrote:
> The comments related to raw monotonic timestamps for the various
> snapshot mechanisms in code and struct documentation are ambiguous. They
> reference them as CLOCK_MONOTONIC_RAW timestamps, but with the arrival
> of AUX clocks that's not longer correct.
>
> The raw monotonic timestamps only represent CLOCK_MONOTONIC_RAW for the
> system time clock IDs, i.e. REALTIME, MONOTONIC, BOOTTIME, TAI.
>
> For AUX clocks they refer to the monotonic raw clock which is related to
> the individual AUX clocks. These monotonic raw timestamps have the same
> conversion factor as CLOCK_MONOTONIC_RAW, but differ from that by an
> offset:
>
> MONORAW(AUX$N) = MONORAW(SYSTEM) + OFFSET(AUX$N)
>
> The offset is established when a AUX clock is enabled and stays constant
> for the lifetime of the AUX clock.
>
> Update the comments so they reflect reality.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Reported-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Thanks!
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
(...)
^ permalink raw reply
* Re: [PATCH v2] virtio_balloon: disable indirect descriptors
From: David Hildenbrand (Arm) @ 2026-07-06 8:43 UTC (permalink / raw)
To: Michael S. Tsirkin, linux-kernel
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Morton,
Alexander Duyck, virtualization
In-Reply-To: <73fac8a629fd9aca7bb3265ac243a769c28af25d.1783232420.git.mst@redhat.com>
On 7/5/26 08:24, Michael S. Tsirkin wrote:
> The page reporting callback submits an sg list to the reporting
> virtqueue. With VIRTIO_RING_F_INDIRECT_DESC negotiated and
> total_sg > 1 (which it typically is), virtqueue_add reports it to the
> host by allocating an indirect descriptor via kmalloc(GFP_KERNEL).
>
> This is not pretty: the reporting worker isolates potentially hundreds
> of MB of free pages from the buddy allocator (reported pages are at
> least pageblock_order, and the sg can contain up to
> PAGE_REPORTING_CAPACITY entries of varying orders). As the result,
> very theoretically, the kmalloc might trigger OOM when we have in fact a
> ton of free memory.
>
> Clear VIRTIO_RING_F_INDIRECT_DESC, to avoid using indirect descriptors.
>
> 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>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
>
> I assume I'm the one merging this?
>
I would think so :)
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2] virtio_balloon: prime stats vq after virtio_device_ready()
From: Michael S. Tsirkin @ 2026-07-06 8:43 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: linux-kernel, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Adam Litke, Rusty Russell, virtualization, Andrew Morton,
linux-mm
In-Reply-To: <6b8e4ad3-6909-4d0a-89a4-104a5ad9fe79@kernel.org>
On Mon, Jul 06, 2026 at 10:38:18AM +0200, David Hildenbrand (Arm) wrote:
> >>> - virtqueue_kick(vb->stats_vq);
> >>> + disable_work(&vb->update_balloon_stats_work);
> >>
> >> That's to stop the stats queue triggering stats_request() I assume?
> >
> > Yes.
> >
> >> Is that
> >> valid before we actually added+kicked ourselves?
> >
> > Why not?
>
> I was wondering whether the spec would state something about that.
Sorry I do not get the question. Virtio API expects
callers to serialize calls for each VQ. So this just prevents the work
from running and accessing the VQ while probe calls add_buf/kick.
I'll add a comment explaining that.
But it has nothing to do with the spec.
> >
> >> Also, can't we simply handle that in stats_request(), just ignoring it there?
> >
> > Then we'd need to maintain a special flag and I dislike that.
>
> I was hoping that we could use the device-ready indication, but there would be a
> small race window indeed.
>
> --
> Cheers,
>
> David
^ permalink raw reply
* Re: [PATCH v2] virtio_balloon: prime stats vq after virtio_device_ready()
From: David Hildenbrand (Arm) @ 2026-07-06 8:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Adam Litke, Rusty Russell, virtualization, Andrew Morton,
linux-mm
In-Reply-To: <20260706044122-mutt-send-email-mst@kernel.org>
On 7/6/26 10:43, Michael S. Tsirkin wrote:
> On Mon, Jul 06, 2026 at 10:38:18AM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> Yes.
>>>
>>>
>>> Why not?
>>
>> I was wondering whether the spec would state something about that.
>
> Sorry I do not get the question. Virtio API expects
> callers to serialize calls for each VQ. So this just prevents the work
> from running and accessing the VQ while probe calls add_buf/kick.
> I'll add a comment explaining that.
> But it has nothing to do with the spec.
The device is clearly not properly initialized yet.
Yet, we expect that we get a stats_request() callback that would try to
queue_work(). And IIUC, the stats_request() will be directly issued by the device.
In QEMU, that would mean that balloon_stats_poll_cb() runs, which would do a
if (s->stats_vq_elem == NULL) {
...
return
}
virtqueue_push(s->svq, s->stats_vq_elem, 0);
virtio_notify(vdev, s->svq);
That's why I'm confused :)
--
Cheers,
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox