* 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 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 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] 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
* [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] 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
* 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
* 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: 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 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: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] 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: 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 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: 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
* [PATCH v3] mm: page_reporting: allow driver to set batch capacity
From: Michael S. Tsirkin @ 2026-07-05 6:25 UTC (permalink / raw)
To: linux-kernel
Cc: David Hildenbrand (Arm), Gregory Price, Zi Yan, Pankaj Gupta,
Jason Wang, Xuan Zhuo, Eugenio Pérez, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Alexander Duyck, virtualization, linux-mm
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
^ permalink raw reply related
* [PATCH v2] virtio_balloon: disable indirect descriptors
From: Michael S. Tsirkin @ 2026-07-05 6:24 UTC (permalink / raw)
To: linux-kernel
Cc: David Hildenbrand (Arm), Jason Wang, Xuan Zhuo,
Eugenio Pérez, Andrew Morton, Alexander Duyck,
virtualization
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?
Changes in v2:
- Qualified OOM scenario as "very theoretically" (David Hildenbrand)
drivers/virtio/virtio_balloon.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 581ac799d974..69160da9cd5d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -7,6 +7,7 @@
*/
#include <linux/virtio.h>
+#include <uapi/linux/virtio_ring.h>
#include <linux/virtio_balloon.h>
#include <linux/swap.h>
#include <linux/workqueue.h>
@@ -1165,6 +1166,11 @@ static int virtballoon_validate(struct virtio_device *vdev)
else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON))
__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
+ /*
+ * Disable indirect descriptors to avoid memory allocation in
+ * virtqueue_add during page reporting.
+ */
+ __virtio_clear_bit(vdev, VIRTIO_RING_F_INDIRECT_DESC);
__virtio_clear_bit(vdev, VIRTIO_F_ACCESS_PLATFORM);
return 0;
}
--
MST
^ permalink raw reply related
* [PATCH] vdpa: octeon_ep: add missing MODULE_DEVICE_TABLE()
From: Pengpeng Hou @ 2026-07-05 0:25 UTC (permalink / raw)
To: schalla, vattunuru, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez
Cc: Pengpeng Hou, virtualization, linux-kernel
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.
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 related
* Re: [PATCH] vdpa: alibaba: add missing MODULE_DEVICE_TABLE()
From: Michael S. Tsirkin @ 2026-07-04 15:35 UTC (permalink / raw)
To: Pengpeng Hou
Cc: Jason Wang, Xuan Zhuo, Eugenio Pérez, virtualization,
linux-kernel
In-Reply-To: <20260704152732.55338-1-pengpeng@iscas.ac.cn>
On Sat, Jul 04, 2026 at 11:27:32PM +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/alibaba/eni_vdpa.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
> index e476504db0c8..fd6fdba46094 100644
> --- a/drivers/vdpa/alibaba/eni_vdpa.c
> +++ b/drivers/vdpa/alibaba/eni_vdpa.c
> @@ -545,6 +545,7 @@ static struct pci_device_id eni_pci_ids[] = {
> VIRTIO_ID_NET) },
> { 0 },
> };
> +MODULE_DEVICE_TABLE(pci, eni_pci_ids);
>
> static struct pci_driver eni_vdpa_driver = {
> .name = "alibaba-eni-vdpa",
> --
> 2.53.0
^ permalink raw reply
* [PATCH] vdpa: alibaba: add missing MODULE_DEVICE_TABLE()
From: Pengpeng Hou @ 2026-07-04 15:27 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: Pengpeng Hou, virtualization, linux-kernel
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.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/vdpa/alibaba/eni_vdpa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/alibaba/eni_vdpa.c b/drivers/vdpa/alibaba/eni_vdpa.c
index e476504db0c8..fd6fdba46094 100644
--- a/drivers/vdpa/alibaba/eni_vdpa.c
+++ b/drivers/vdpa/alibaba/eni_vdpa.c
@@ -545,6 +545,7 @@ static struct pci_device_id eni_pci_ids[] = {
VIRTIO_ID_NET) },
{ 0 },
};
+MODULE_DEVICE_TABLE(pci, eni_pci_ids);
static struct pci_driver eni_vdpa_driver = {
.name = "alibaba-eni-vdpa",
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 01/13] mm: introduce vma_flags_can_grow() and vma_can_grow()
From: Zi Yan @ 2026-07-03 15:19 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Thomas Bogendoerfer, Madhavan Srinivasan, Michael Ellerman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Lucas Stach, Inki Dae, Seung-Woo Kim,
Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Rob Clark,
Dmitry Baryshkov, Lyude Paul, Danilo Krummrich, Tomi Valkeinen,
Sandy Huang, Heiko Stübner, Andy Yan, Thierry Reding,
Mikko Perttunen, Jonathan Hunter, Gerd Hoffmann, Dmitry Osipenko,
Zack Rusin, Matthew Brost, Thomas Hellstrom,
Oleksandr Andrushchenko, Helge Deller, Benjamin LaHaise,
Alexander Viro, Christian Brauner, Muchun Song, Oscar Salvador,
David Hildenbrand, Baolin Wang, Liam R . Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Hugh Dickins,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jann Horn, Pedro Falcato, Kees Cook, Jaroslav Kysela,
Takashi Iwai, linux-mips, linux-kernel, linuxppc-dev, dri-devel,
etnaviv, linux-arm-kernel, linux-samsung-soc, intel-gfx,
linux-arm-msm, freedreno, nouveau, linux-rockchip, linux-tegra,
virtualization, intel-xe, xen-devel, linux-fbdev, linux-aio,
linux-fsdevel, linux-mm, linux-sound
In-Reply-To: <f2e8c32515d328db62279cc8bab8398ea278d74f.1782760670.git.ljs@kernel.org>
On Mon Jun 29, 2026 at 3:25 PM EDT, Lorenzo Stoakes wrote:
> These test whether the VMA has stack sematics, i.e. is able to grow upwards
> or downwards depending on the architecture.
>
> In order to account for arches which do not support upward-growing stacks,
> introduce VMA_GROWSUP whose definition depends on the architecture
> supporting it, and use vma_flags_test_single_mask() in vma_flags_can_grow()
> to account for this.
>
> Update the VMA userland tests to reflect the changes
>
> No functional change intended.
>
> Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> ---
> include/linux/mm.h | 21 ++++++++++++++++++---
> tools/testing/vma/include/dup.h | 4 ++++
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
Reviewed-by: Zi Yan <ziy@nvidia.com>
--
Best Regards,
Yan, Zi
^ permalink raw reply
* Re: [PATCH v3] drm/vblank: Don't arm vblank timer with invalid frame duration
From: Thomas Zimmermann @ 2026-07-03 7:26 UTC (permalink / raw)
To: Roman Ilin, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: Louis Chauvet, Javier Martinez Canillas, Dmitry Osipenko,
dri-devel, virtualization, linux-kernel, Ville Syrjälä,
Thorsten Leemhuis, Peter Arnesen
In-Reply-To: <E697A2AE-5413-48AE-BFD6-3383819B388B@romanilin.is>
Hi
Am 02.07.26 um 20:23 schrieb Roman Ilin:
> Apologies, I accidentally fired off send-email before saving my final
> changelog notes. Please ignore the changelog and notes in the original
> v3 email.
>
> The actual changes in v3 are:
>
> - Changed the WARN_ON_ONCE to drm_dbg_kms (to avoid user-triggerable
> panics).
We want to know when this happens. And user space will only be able to
trigger this once, so there's no risk of spamming the kernel log.
So this is not really a problem. drm_WARN_ON_ONCE was ok for that. You
can also use a regular DRM print macro. But instead of drm_dbg_kms()
should use drm_err_once(). But please also output linedur_ns and
framedur_ns in the error. We want to know which of them is incorrect.
You can also add more information to the error message. See [1] for the
mode-formating macros.
[1]
https://elixir.bootlin.com/linux/v7.1.2/source/include/drm/drm_modes.h#L422
> - Updated drm_calc_timestamping_constants with the goto error fallback
> to clear the stale state.
>
> Also, an automated review bot pointed out an AB-BA deadlock in
> drm_crtc_vblank_start_timer(). But I am leaving this out of the patch to
> keep the fixes orthogonal.
>
> Sorry for the noise.
>
>> On Jul 2, 2026, at 21:10, Roman Ilin <me@romanilin.is> wrote:
>>
>> When a CRTC's display mode carries a too small pixel clock,
>> drm_calc_timestamping_constants() computes a frame duration that
>> exceeds INT_MAX. drm_vblank_crtc.framedur_ns becomes negative.
>> drm_crtc_vblank_start_timer() then arms the vblank hrtimer with this
>> interval, after which vblank events are no longer delivered. Pending
>> page flips never complete and the display appears frozen.
>>
>> This could be triggered on virtio-gpu guests that have dynamic resolution
>> enabled: when the SPICE agent or the X server resizes the output, it
>> submits a mode whose pixel clock is off by a factor of 1000, e.g.:
>>
>> clock = 406 kHz, htotal = 3152, vtotal = 2148
>>
>> framedur_ns = 3152 * 2148 * 1000000 / 406 = 16675852216 ns (~16.7 s)
>>
>> 16675852216 does not fit into an int and wraps to roughly -504000000.
>> ns_to_ktime() then yields a negative interval and the timer stops working.
>>
>> Found by bisection, which pointed at commit a036f5fceedb ("drm/virtgpu:
>> Use vblank timer"). That commit merely made virtio-gpu use the vblank
>> timer and thereby exposed the pre-existing problem in the timer setup
>> added by commit 74afeb812850 ("drm/vblank: Add vblank timer").
>>
>> To fix this, modify drm_calc_timestamping_constants() to use u64 for
>> calculations, check for INT_MAX overflows, and return an error code.
>> drm_crtc_vblank_start_timer() will then propagate the error, enabling
>> the driver to fall back to immediate vblank events. Valid modes are
>> unaffected, and the timer self-heals on the next mode with a sane clock.
>>
>> Fixes: 74afeb812850 ("drm/vblank: Add vblank timer")
>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Roman Ilin <me@romanilin.is>
>> ---
>> Changes in v3:
>>
>> - Changed the WARN_ON_ONCE to drm_err_once
>>
>> Notes:
>>
>>
>>
>> drivers/gpu/drm/drm_vblank.c | 71 +++++++++++++++++++++++-------------
>> include/drm/drm_vblank.h | 4 +-
>> 2 files changed, 48 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index f90fb2d13..629b9fcc7 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -631,42 +631,51 @@ EXPORT_SYMBOL(drm_crtc_vblank_waitqueue);
>> * drm_crtc_vblank_helper_get_vblank_timestamp(). They are derived from
>> * CRTC's true scanout timing, so they take things like panel scaling or
>> * other adjustments into account.
>> + *
>> + * Returns:
>> + * 0 on success, or a negative errno code otherwise.
>> */
>> -void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>> - const struct drm_display_mode *mode)
>> +int drm_calc_timestamping_constants(struct drm_crtc *crtc,
>> + const struct drm_display_mode *mode)
>> {
>> struct drm_device *dev = crtc->dev;
>> unsigned int pipe = drm_crtc_index(crtc);
>> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> - int linedur_ns = 0, framedur_ns = 0;
>> + u64 linedur_ns, framedur_ns;
>> int dotclock = mode->crtc_clock;
>> + unsigned int frame_size;
>>
>> if (!drm_dev_has_vblank(dev))
>> - return;
>> + return 0;
>>
>> if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>> - return;
>> + return -EINVAL;
>>
>> - /* Valid dotclock? */
>> - if (dotclock > 0) {
>> - int frame_size = mode->crtc_htotal * mode->crtc_vtotal;
>> + if (dotclock <= 0) {
>> + drm_err(dev, "crtc %u: Can't calculate constants, dotclock = %d!\n",
Please turn this into drm_err_once() because this call can actually be
triggered repeatedly from userspace.
Best regards
Thomas
>> + crtc->base.id, dotclock);
>> + goto error;
>> + }
>>
>> - /*
>> - * Convert scanline length in pixels and video
>> - * dot clock to line duration and frame duration
>> - * in nanoseconds:
>> - */
>> - linedur_ns = div_u64((u64) mode->crtc_htotal * 1000000, dotclock);
>> - framedur_ns = div_u64((u64) frame_size * 1000000, dotclock);
>> + frame_size = (unsigned int)mode->crtc_htotal * (unsigned int)mode->crtc_vtotal;
>>
>> - /*
>> - * Fields of interlaced scanout modes are only half a frame duration.
>> - */
>> - if (mode->flags & DRM_MODE_FLAG_INTERLACE)
>> - framedur_ns /= 2;
>> - } else {
>> - drm_err(dev, "crtc %u: Can't calculate constants, dotclock = 0!\n",
>> - crtc->base.id);
>> + /*
>> + * Convert scanline length in pixels and video dot clock to line duration
>> + * and frame duration in nanoseconds.
>> + */
>> + linedur_ns = div_u64((u64)mode->crtc_htotal * 1000000, dotclock);
>> + framedur_ns = div_u64((u64)frame_size * 1000000, dotclock);
>> +
>> + /*
>> + * Fields of interlaced scanout modes are only half a frame duration.
>> + */
>> + if (mode->flags & DRM_MODE_FLAG_INTERLACE)
>> + framedur_ns /= 2;
>> +
>> + if (linedur_ns > INT_MAX || framedur_ns > INT_MAX) {
>> + drm_dbg_kms(dev, "crtc %u: Can't calculate constants, mode clock too small!\n",
>> + crtc->base.id);
>> + goto error;
>> }
>>
>> vblank->linedur_ns = linedur_ns;
>> @@ -678,7 +687,16 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>> crtc->base.id, mode->crtc_htotal,
>> mode->crtc_vtotal, mode->crtc_vdisplay);
>> drm_dbg_core(dev, "crtc %u: clock %d kHz framedur %d linedur %d\n",
>> - crtc->base.id, dotclock, framedur_ns, linedur_ns);
>> + crtc->base.id, dotclock,
>> + vblank->framedur_ns, vblank->linedur_ns);
>> +
>> + return 0;
>> +
>> +error:
>> + vblank->linedur_ns = 0;
>> + vblank->framedur_ns = 0;
>> + drm_mode_copy(&vblank->hwmode, mode);
>> + return -EINVAL;
>> }
>> EXPORT_SYMBOL(drm_calc_timestamping_constants);
>>
>> @@ -2221,6 +2239,7 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> struct drm_vblank_crtc_timer *vtimer = &vblank->vblank_timer;
>> unsigned long flags;
>> + int ret;
>>
>> if (!vtimer->crtc) {
>> /*
>> @@ -2239,7 +2258,9 @@ int drm_crtc_vblank_start_timer(struct drm_crtc *crtc)
>> hrtimer_try_to_cancel(&vtimer->timer);
>> }
>>
>> - drm_calc_timestamping_constants(crtc, &crtc->mode);
>> + ret = drm_calc_timestamping_constants(crtc, &crtc->mode);
>> + if (ret)
>> + return ret;
>>
>> spin_lock_irqsave(&vtimer->interval_lock, flags);
>> vtimer->interval = ns_to_ktime(vblank->framedur_ns);
>> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
>> index 2fcef9c0f..d99772dfa 100644
>> --- a/include/drm/drm_vblank.h
>> +++ b/include/drm/drm_vblank.h
>> @@ -311,8 +311,8 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc);
>> u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
>> void drm_crtc_vblank_restore(struct drm_crtc *crtc);
>>
>> -void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>> - const struct drm_display_mode *mode);
>> +int drm_calc_timestamping_constants(struct drm_crtc *crtc,
>> + const struct drm_display_mode *mode);
>> wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc);
>> void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc,
>> u32 max_vblank_count);
>> --
>> 2.54.0
>>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
From: Ryosuke Yasuoka @ 2026-07-03 5:58 UTC (permalink / raw)
To: Dmitry Osipenko, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas
Cc: dri-devel, virtualization, linux-kernel
In-Reply-To: <1b56e514-17b5-4da1-9ebd-4534e3204ea4@collabora.com>
On 02/07/2026 14:26, Dmitry Osipenko wrote:
> On 7/1/26 12:23, Ryosuke Yasuoka wrote:
>>
>>
>> On 30/06/2026 16:46, Dmitry Osipenko wrote:
>>> Hi,
>>>
>>> On 6/30/26 12:16, Ryosuke Yasuoka wrote:
>>>> A probe-time deadlock can occur between the dequeue worker and
>>>> drm_client_register(). During probe, drm_client_register() holds
>>>> clientlist_mutex and calls the fbdev hotplug callback, which triggers an
>>>> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
>>>> waiting for virtqueue space. The dequeue worker that would free that
>>>> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
>>>> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
>>>> to acquire the same clientlist_mutex. Since wake_up() is only called
>>>> after the resp_cb loop, the probe thread is never woken and both threads
>>>> deadlock.
>>>>
>>>> Fix this by deferring the hotplug notification from
>>>> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The
>>>> display data (outputs[i].info) is still updated synchronously in the
>>>> callback, and the deferred work only triggers a re-probe notification to
>>>> DRM clients.
>>>>
>>>> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
>>>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>>>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>>>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>>>> ---
>>>> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++
>>>> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++
>>>> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++--
>>>> 3 files changed, 16 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
>>>> index 7449907754a4..27ffa4697ae9 100644
>>>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
>>>> @@ -264,6 +264,8 @@ struct virtio_gpu_device {
>>>>
>>>> struct work_struct config_changed_work;
>>>>
>>>> + struct work_struct hotplug_work;
>>>> +
>>>> struct work_struct obj_free_work;
>>>> spinlock_t obj_free_lock;
>>>> struct list_head obj_free_list;
>>>> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
>>>> uint32_t x, uint32_t y,
>>>> struct virtio_gpu_object_array *objs,
>>>> struct virtio_gpu_fence *fence);
>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work);
>>>> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev,
>>>> uint32_t resource_id,
>>>> uint32_t x, uint32_t y,
>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
>>>> index cfde9f573df6..cfb532ba43a4 100644
>>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
>>>> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
>>>> INIT_WORK(&vgdev->config_changed_work,
>>>> virtio_gpu_config_changed_work_func);
>>>>
>>>> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func);
>>>> +
>>>> INIT_WORK(&vgdev->obj_free_work,
>>>> virtio_gpu_array_put_free_work);
>>>> INIT_LIST_HEAD(&vgdev->obj_free_list);
>>>> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
>>>> flush_work(&vgdev->obj_free_work);
>>>> flush_work(&vgdev->ctrlq.dequeue_work);
>>>> flush_work(&vgdev->cursorq.dequeue_work);
>>>> + flush_work(&vgdev->hotplug_work);
>>>> flush_work(&vgdev->config_changed_work);
>>>> virtio_reset_device(vgdev->vdev);
>>>> vgdev->vdev->config->del_vqs(vgdev->vdev);
>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
>>>> index 67865810a2e7..084d98f5dc7b 100644
>>>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
>>>> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev,
>>>> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence);
>>>> }
>>>>
>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work)
>>>> +{
>>>> + struct virtio_gpu_device *vgdev =
>>>> + container_of(work, struct virtio_gpu_device, hotplug_work);
>>>> +
>>>> + if (!drm_helper_hpd_irq_event(vgdev->ddev))
>>>> + drm_kms_helper_hotplug_event(vgdev->ddev);
>>>> +}
>>>> +
>>>> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>>>> struct virtio_gpu_vbuffer *vbuf)
>>>> {
>>>> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>>>> spin_unlock(&vgdev->display_info_lock);
>>>> wake_up(&vgdev->resp_wq);
>>>>
>>>> - if (!drm_helper_hpd_irq_event(vgdev->ddev))
>>>> - drm_kms_helper_hotplug_event(vgdev->ddev);
>>>> + schedule_work(&vgdev->hotplug_work);
>>>> }
>>>>
>>>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
>>>
>>
>> Hi,
>> Thank you for your review.
>>
>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout.
>>
>> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event()
>> into virtio_gpu_init() after wait_event_timeout() would not prevent the
>> issue.
>>
>> Looking at the syzbot call traces[1][2], the deadlock occurs during
>> drm_client_setup(), which runs after virtio_gpu_init() has already
>> returned. The display_info_cb that triggers the deadlock is called from
>> the dequeue worker while drm_client_register() holds clientlist_mutex.
>>
>> Thread A:
>> virtio_gpu_probe()
>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s
>
> You mean that the timeout happens and it's again syzkaller report for a
> broken host. A day ago I applied [1] that should fix this "bogus"
> syzkaller report.
>
> [1] https://patchwork.freedesktop.org/patch/735301/
Thank you for [1] and the clarificaiton. I agree that [1] addresses the
broken host scenario where virtio_gpu_init() times out and probe
continues with a pending display_info response. I think there is another
scenario causing the same deadlock reported by syzbot and my patch
addresses both cases.
The deadlock can also occur with the following scenario:
after virtio_gpu_init() completes successfully (wait_event_timeout
returns), if a config_changed event arrives before drm_client_setup()
completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its
display_info_cb fires in the dequeue worker and blocks on
clientlist_mutex held by drm_client_register(). This is the same
deadlock path reported by syzbot.
Since [1] only prevents the timeout case, I believe the schedule_work()
approach is still needed to prevent display_info_cb from blocking on
clientlist_mutex in the dequeue worker context, regardless of the
trigger.
Best regards,
Ryosuke
^ permalink raw reply
* Re: [PATCH v2 0/4] virtio_balloon: quiesce balloon work on device shutdown
From: Michael S. Tsirkin @ 2026-07-02 22:33 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Denis V. Lunev, Denis V. Lunev, virtualization, linux-kernel
In-Reply-To: <00ebb86d-f7b8-4ddc-a17c-c4700cab73a9@kernel.org>
On Thu, Jul 02, 2026 at 09:30:26PM +0200, David Hildenbrand (Arm) wrote:
> On 7/2/26 19:50, Denis V. Lunev wrote:
> > On 6/24/26 16:08, Denis V. Lunev wrote:
> >> This email originated from an IP that might not be authorized by the domain it was sent from.
> >> Do not click links or open attachments unless it is an email you expected to receive.
> >> Since commit 8bd2fa086a04 ("virtio: break and reset virtio devices on
> >> device_shutdown()") the virtio bus breaks and resets every virtio device
> >> during device_shutdown(), i.e. on reboot and kexec. virtio_balloon has no
> >> .shutdown of its own, so that generic path runs while the balloon's
> >> asynchronous work is still armed: the free page reporting worker, the
> >> inflate/deflate and stats workers, the OOM notifier and the free page
> >> shrinker.
> >>
> >> Once the device has been broken, virtqueue_add_inbuf() in
> >> virtballoon_free_page_report() returns -EIO and trips its WARN_ON_ONCE().
> >> On a kernel booted with panic_on_warn that turns an ordinary reboot into a
> >> fatal panic in the middle of device_shutdown(), so the machine never
> >> reaches the new kernel. The inflate/deflate and OOM paths do not warn but
> >> are no better off: they call wait_event(vb->acked, ...) and would block
> >> forever on a queue that can no longer complete.
> >>
> >> This was hit in the field as an intermittent failure of a virtualization
> >> cluster upgrade: guest storage nodes were rebooted via kexec into the new
> >> kernel, and the ones whose free page reporting happened to run during
> >> device_shutdown() panicked (the guests run with panic_on_warn) and never
> >> came back, stalling the rolling upgrade. The crash dump showed the WARN at
> >> virtio_balloon.c:216 in a page_reporting kworker, with all the balloon
> >> virtqueues already broken.
> >>
> >> Validated by churning balloon inflate/deflate from the host while
> >> kexec-rebooting the guest in a loop under panic_on_warn: the unpatched
> >> kernel reproduces the WARN within a couple of cycles, while the patched
> >> kernel survives many consecutive kexec cycles cleanly (12/12 in the final
> >> run, 0 WARNs). checkpatch is clean across the series.
> >>
> >> Changes in v2:
> >> - Add a virtio_device_shutdown() core helper and call it from the balloon
> >> .shutdown handler instead of open-coding break + synchronize_cbs + reset
> >> (David Hildenbrand).
> >> - New patch: make tell_host() warn and bail instead of hanging if a buffer
> >> add ever fails (David Hildenbrand); kept as a separate patch
> >> (Michael S. Tsirkin).
> >>
> >> v1: https://lore.kernel.org/all/20260622133715.3707707-1-den@openvz.org
> >>
> >> Denis V. Lunev (4):
> >> virtio: add virtio_device_shutdown() helper
> >> virtio_balloon: factor out virtballoon_quiesce()
> >> virtio_balloon: quiesce balloon work before device shutdown
> >> virtio_balloon: warn on failed buffer add in tell_host()
> >>
> >> drivers/virtio/virtio.c | 41 ++++++++++++++++++++++-----------
> >> drivers/virtio/virtio_balloon.c | 40 ++++++++++++++++++++++++--------
> >> include/linux/virtio.h | 1 +
> >> 3 files changed, 59 insertions(+), 23 deletions(-)
> >>
> > Hi, David!
>
> Hi! :)
>
> >
> > Is this good to go in? I have not seen the confirmation that the
> > series is taken into your tree.
>
> I don't have a tree (yet), and once I have one it will likely be more mm focused :)
>
> @MST, I think this is good to go!
>
> --
> Cheers,
>
> David
Indeed, it's just a bugfix and I'm trying to get features into qemu
now before their freeze. I'll work on linux end of next week.
^ permalink raw reply
* Re: [PATCH net 1/2] vsock/virtio: collapse receive queue under memory pressure
From: Bobby Eshleman @ 2026-07-02 20:09 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Jason Wang, Jakub Kicinski, Paolo Abeni,
Michael S. Tsirkin, kvm, virtualization, Xuan Zhuo, Eric Dumazet,
Simon Horman, linux-kernel, Stefan Hajnoczi, David S. Miller,
Eugenio Pérez, stable, Brien Oberstein
In-Reply-To: <akYl38_9Y4ydXuqE@sgarzare-redhat>
On Thu, Jul 02, 2026 at 10:56:04AM +0200, Stefano Garzarella wrote:
> On Wed, Jul 01, 2026 at 09:34:35AM -0700, Bobby Eshleman wrote:
> > On Fri, Jun 26, 2026 at 03:48:22PM +0200, Stefano Garzarella wrote:
>
> [...]
>
> > > +out:
> > > + if (new_skb)
> > > + __skb_queue_tail(&new_queue, new_skb);
> > > +
> > > + skb_queue_splice(&new_queue, &vvs->rx_queue);
> >
> > I think the new skbs will also need skb_set_owner_sk_safe(skb, sk)
> > when adding to rx_queue?
>
> IIRC we added it in the rx path, mainily for loopback to pass the ownership
> from the tx socket to the rx socket, but here we are already in the rx path,
> so the skb will never leave this socket.
>
Ah that's right, I stand corrected. There is no sender to leak in this
case.
> Maybe it's necessary for the eBPF path?
Looking through sockmap, I don't think it depends on skb->sk being
non-null either (it reassigns owner to the redirect socket anyway using
skb_set_owner_r()).
Sorry for the false alarm. LGTM.
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
^ 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