* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: Dmitry Osipenko @ 2026-05-13 21:54 UTC (permalink / raw)
To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter
Cc: dri-devel, virtualization, linux-kernel
In-Reply-To: <82aeba42-7872-4e92-ade8-2b7b6a8a5e43@collabora.com>
On 5/14/26 00:40, Dmitry Osipenko wrote:
> Hi,
>
> On 5/12/26 11:59, Ryosuke Yasuoka wrote:
>> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
>> wait_event() without timeout when waiting for virtqueue space. If the
>> host device stops processing commands, these waits block indefinitely.
>> Since callers may hold DRM locks, this can make the entire system
>> unresponsive.
>>
>> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
>> consistent with the existing timeout pattern in the driver. On timeout,
>> clean up and return -ENODEV, following the same error path as
>> drm_dev_enter() failure.
>>
>> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>> ---
>> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> If host stops processing commands, this is a problem on host side. Isn't it?
It may be acceptable to have wait_event_timeout() in a loop, printing
warnings about unresponsive host.
Don't think we can assume that 5 seconds is enough to say that host is
busted, unless spec says so.
There could be a driver module parameter, specifying the timeoout. This
will be acceptable as user takes responsibility for the special timeout
behaviour.
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: syzbot @ 2026-05-13 21:41 UTC (permalink / raw)
To: dmitry.osipenko
Cc: airlied, dmitry.osipenko, dri-devel, gurchetansingh, kraxel,
linux-kernel, maarten.lankhorst, mripard, olvaffe, ryasuoka,
simona, tzimmermann, virtualization
In-Reply-To: <82aeba42-7872-4e92-ade8-2b7b6a8a5e43@collabora.com>
> Hi,
>
> On 5/12/26 11:59, Ryosuke Yasuoka wrote:
>> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
>> wait_event() without timeout when waiting for virtqueue space. If the
>> host device stops processing commands, these waits block indefinitely.
>> Since callers may hold DRM locks, this can make the entire system
>> unresponsive.
>>
>> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
>> consistent with the existing timeout pattern in the driver. On timeout,
>> clean up and return -ENODEV, following the same error path as
>> drm_dev_enter() failure.
>>
>> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>> ---
>> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> If host stops processing commands, this is a problem on host side. Isn't it?
>
> --
> Best regards,
> Dmitry
I see the command but can't find the corresponding bug.
The email is sent to syzbot+HASH@syzkaller.appspotmail.com address
but the HASH does not correspond to any known bug.
Please double check the address.
^ permalink raw reply
* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: syzbot @ 2026-05-13 21:41 UTC (permalink / raw)
To: dmitry.osipenko
Cc: airlied, dmitry.osipenko, dri-devel, gurchetansingh, kraxel,
linux-kernel, maarten.lankhorst, mripard, olvaffe, ryasuoka,
simona, tzimmermann, virtualization
In-Reply-To: <82aeba42-7872-4e92-ade8-2b7b6a8a5e43@collabora.com>
> Hi,
>
> On 5/12/26 11:59, Ryosuke Yasuoka wrote:
>> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
>> wait_event() without timeout when waiting for virtqueue space. If the
>> host device stops processing commands, these waits block indefinitely.
>> Since callers may hold DRM locks, this can make the entire system
>> unresponsive.
>>
>> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
>> consistent with the existing timeout pattern in the driver. On timeout,
>> clean up and return -ENODEV, following the same error path as
>> drm_dev_enter() failure.
>>
>> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
>> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
>> ---
>> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> If host stops processing commands, this is a problem on host side. Isn't it?
>
> --
> Best regards,
> Dmitry
I see the command but can't find the corresponding bug.
The email is sent to syzbot+HASH@syzkaller.appspotmail.com address
but the HASH does not correspond to any known bug.
Please double check the address.
^ permalink raw reply
* Re: [PATCH] drm/virtio: add timeout to virtqueue wait to avoid hung task
From: Dmitry Osipenko @ 2026-05-13 21:40 UTC (permalink / raw)
To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter
Cc: dri-devel, virtualization, linux-kernel,
syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224,
syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e
In-Reply-To: <20260512-virtio-gpu_wait_event-v1-1-207eb4c1a69a@redhat.com>
Hi,
On 5/12/26 11:59, Ryosuke Yasuoka wrote:
> virtio_gpu_queue_ctrl_sgs() and virtio_gpu_queue_cursor() use
> wait_event() without timeout when waiting for virtqueue space. If the
> host device stops processing commands, these waits block indefinitely.
> Since callers may hold DRM locks, this can make the entire system
> unresponsive.
>
> Replace wait_event() with wait_event_timeout() using a 5-second timeout,
> consistent with the existing timeout pattern in the driver. On timeout,
> clean up and return -ENODEV, following the same error path as
> drm_dev_enter() failure.
>
> Reported-by: syzbot+d6dd6f86d3aaf7eebe7406e45c1c6e549453f224@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> Reported-by: syzbot+908bd910da5dd79b88de4cf7baf376cc873a922e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> drivers/gpu/drm/virtio/virtgpu_vq.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
If host stops processing commands, this is a problem on host side. Isn't it?
--
Best regards,
Dmitry
^ permalink raw reply
* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Peter Zijlstra @ 2026-05-13 19:33 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-kernel, linux-mips,
virtualization, linux-arch, linux-mm, linux-trace-kernel,
kernel-team, Paul E. McKenney
In-Reply-To: <5d7ea75ffe74a785e6b234ada9f23c6373d4b4c1.1777999826.git.d@ilvokhin.com>
On Tue, May 05, 2026 at 05:09:34PM +0000, Dmitry Ilvokhin wrote:
> Use the arch-overridable queued_spin_release(), introduced in the
> previous commit, to ensure the tracepoint works correctly across all
> architectures, including those with custom unlock implementations (e.g.
> x86 paravirt).
>
> When the tracepoint is disabled, the only addition to the hot path is a
> single NOP instruction (the static branch). When enabled, the contention
> check, trace call, and unlock are combined in an out-of-line function to
> minimize hot path impact, avoiding the compiler needing to preserve the
> lock pointer in a callee-saved register across the trace call.
>
> Binary size impact (x86_64, defconfig):
> uninlined unlock (common case): +680 bytes (+0.00%)
> inlined unlock (worst case): +83659 bytes (+0.21%)
>
> The inlined unlock case could not be achieved through Kconfig options on
> x86_64 as PREEMPT_BUILD unconditionally selects UNINLINE_SPIN_UNLOCK on
> x86_64. The UNINLINE_SPIN_UNLOCK guards were manually inverted to force
> inline the unlock path and estimate the worst case binary size increase.
>
> In practice, configurations with UNINLINE_SPIN_UNLOCK=n have already
> opted against binary size optimization, so the inlined worst case is
> unlikely to be a concern.
This is not quite accurate. You add the (5byte) NOP for the static
branch, but then you also add another 5 bytes for the CALL and at least
another 2 bytes (possibly 5) for a JMP back into the previous stream.
That is 12-15 bytes added to what was a single MOV instruction.
That is quite ludicrous.
I disagree that UNINLINE_SPIN_UNLOCK=n opts against binary size. For x86
the unlock is smaller than a function call.
I really don't see how this is worth it.
^ permalink raw reply
* Re: [PATCH v6 0/7] locking: contended_release tracepoint instrumentation
From: Peter Zijlstra @ 2026-05-13 19:26 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-kernel, linux-mips,
virtualization, linux-arch, linux-mm, linux-trace-kernel,
kernel-team
In-Reply-To: <cover.1777999826.git.d@ilvokhin.com>
On Tue, May 05, 2026 at 05:09:29PM +0000, Dmitry Ilvokhin wrote:
> This series adds a contended_release tracepoint that fires on the
> holder side when a lock with waiters is released. This provides:
>
> - Hold time estimation: when the holder's own acquisition was
> contended, its contention_end (acquisition) and contended_release
> can be correlated to measure how long the lock was held under
> contention.
>
> - The holder's stack at release time, which may differ from what perf lock
> contention --lock-owner captures if the holder does significant work between
> the waiter's arrival and the unlock.
>
> Note: for reader/writer locks, the tracepoint fires for every reader
> releasing while a writer is waiting, not only for the last reader.
And for qspinlock.
I am really not sure this is worth the overhead for qspinlock / rwlock.
^ permalink raw reply
* [RFC PATCH 2/2] virtio-balloon: add stats push mode
From: Gregory Price @ 2026-05-13 16:50 UTC (permalink / raw)
To: virtualization
Cc: linux-kernel, kernel-team, mst, david, jasowang, xuanzhuo,
eperezma, hannes, surenb, peterz, mingo, juri.lelli,
vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, kprateek.nayak
In-Reply-To: <20260513165006.2790857-1-gourry@gourry.net>
When doing aggressive overcommit of VMs on a single host, a pull
model of stat retrieval is problematic if a guest becomes some form
of unresponsive. In particular, it's difficult to discern the
difference between a hung guest and a slow guest - and why the
guest is experiencing that.
Add VIRTIO_BALLOON_F_STATS_PUSH feature that allows the host to
configure the guest to push stats on a timer instead of the default
pull model.
The host sets stats_push_interval_ms in the balloon config space:
0 = disabled (pull-only, default)
N > 0 = guest pushes stats every N milliseconds
The push mode reuses the existing stats VQ, same buffer format,
same tags. The host can change the interval at runtime by updating
the config field.
Push mode provides two advantages over pull:
1. Guest liveness detection: in pull mode, the host cannot
distinguish a slow guest from a hung guest without implementing
its own timeout tracking. In push mode, the absence of expected
stats buffers is an implicit liveness signal; if the guest
fails to push within the expected interval, the host can
conclude it is unresponsive.
2. Latency-sensitive consumers (e.g., memory pressure response
loops) receive fresh stats at a guaranteed cadence without
the host needing to poll.
STATS_PUSH requires STATS_VQ; the driver clears STATS_PUSH during
feature validation if STATS_VQ is absent. When push mode is active,
the pull callback is suppressed to avoid racing on buffer submission.
The pull model remains available and is the default.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Gregory Price <gourry@gourry.net>
---
drivers/virtio/virtio_balloon.c | 71 +++++++++++++++++++++++++++++
include/uapi/linux/virtio_balloon.h | 7 +++
2 files changed, 78 insertions(+)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 8fa33aec4ce7..47bde1d2b388 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -112,6 +112,10 @@ struct virtio_balloon {
/* Memory statistics */
struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
+ /* Stats push mode */
+ struct delayed_work stats_push_work;
+ uint32_t stats_push_interval_ms;
+
/* Shrinker to return free pages - VIRTIO_BALLOON_F_FREE_PAGE_HINT */
struct shrinker *shrinker;
@@ -463,6 +467,13 @@ static void stats_request(struct virtqueue *vq)
{
struct virtio_balloon *vb = vq->vdev->priv;
+ /*
+ * In push mode, the push timer owns the VQ. Ignore pull
+ * requests to avoid racing on buffer submission.
+ */
+ if (vb->stats_push_interval_ms)
+ return;
+
spin_lock(&vb->stop_update_lock);
if (!vb->stop_update) {
start_wakeup_event(vb, VIRTIO_BALLOON_WAKEUP_SIGNAL_STATS);
@@ -558,6 +569,20 @@ static void virtballoon_changed(struct virtio_device *vdev)
virtio_balloon_queue_free_page_work(vb);
}
spin_unlock_irqrestore(&vb->stop_update_lock, flags);
+
+ if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_STATS_PUSH)) {
+ uint32_t interval;
+
+ virtio_cread_le(vdev, struct virtio_balloon_config,
+ stats_push_interval_ms, &interval);
+ if (interval != vb->stats_push_interval_ms) {
+ vb->stats_push_interval_ms = interval;
+ cancel_delayed_work(&vb->stats_push_work);
+ if (interval)
+ schedule_delayed_work(&vb->stats_push_work,
+ msecs_to_jiffies(interval));
+ }
+ }
}
static void update_balloon_size(struct virtio_balloon *vb)
@@ -581,6 +606,32 @@ static void update_balloon_stats_func(struct work_struct *work)
finish_wakeup_event(vb);
}
+static void stats_push_func(struct work_struct *work)
+{
+ struct virtio_balloon *vb = container_of(work, struct virtio_balloon,
+ stats_push_work.work);
+ struct virtqueue *vq;
+ struct scatterlist sg;
+ unsigned int num_stats, len;
+
+ if (!vb->stats_push_interval_ms)
+ return;
+
+ vq = vb->stats_vq;
+
+ /* Reclaim previous buffer */
+ while (virtqueue_get_buf(vq, &len))
+ ;
+
+ num_stats = update_balloon_stats(vb);
+ sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
+ virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
+ virtqueue_kick(vq);
+
+ schedule_delayed_work(&vb->stats_push_work,
+ msecs_to_jiffies(vb->stats_push_interval_ms));
+}
+
static void update_balloon_size_func(struct work_struct *work)
{
struct virtio_balloon *vb;
@@ -967,6 +1018,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
}
INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
+ INIT_DELAYED_WORK(&vb->stats_push_work, stats_push_func);
INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
spin_lock_init(&vb->stop_update_lock);
mutex_init(&vb->balloon_lock);
@@ -1094,6 +1146,19 @@ static int virtballoon_probe(struct virtio_device *vdev)
if (towards_target(vb))
virtballoon_changed(vdev);
+
+ if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_STATS_PUSH)) {
+ uint32_t interval;
+
+ virtio_cread_le(vdev, struct virtio_balloon_config,
+ stats_push_interval_ms, &interval);
+ if (interval) {
+ vb->stats_push_interval_ms = interval;
+ schedule_delayed_work(&vb->stats_push_work,
+ msecs_to_jiffies(interval));
+ }
+ }
+
return 0;
out_unregister_oom:
@@ -1145,6 +1210,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
spin_unlock_irq(&vb->stop_update_lock);
cancel_work_sync(&vb->update_balloon_size_work);
cancel_work_sync(&vb->update_balloon_stats_work);
+ cancel_delayed_work_sync(&vb->stats_push_work);
if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
cancel_work_sync(&vb->report_free_page_work);
@@ -1199,6 +1265,10 @@ 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);
+ if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_STATS_PUSH) &&
+ !virtio_has_feature(vdev, VIRTIO_BALLOON_F_STATS_VQ))
+ __virtio_clear_bit(vdev, VIRTIO_BALLOON_F_STATS_PUSH);
+
__virtio_clear_bit(vdev, VIRTIO_F_ACCESS_PLATFORM);
return 0;
}
@@ -1210,6 +1280,7 @@ static unsigned int features[] = {
VIRTIO_BALLOON_F_FREE_PAGE_HINT,
VIRTIO_BALLOON_F_PAGE_POISON,
VIRTIO_BALLOON_F_REPORTING,
+ VIRTIO_BALLOON_F_STATS_PUSH,
};
static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 37ec8a8466c4..90e9b5247e5e 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -37,6 +37,7 @@
#define VIRTIO_BALLOON_F_FREE_PAGE_HINT 3 /* VQ to report free pages */
#define VIRTIO_BALLOON_F_PAGE_POISON 4 /* Guest is using page poisoning */
#define VIRTIO_BALLOON_F_REPORTING 5 /* Page reporting virtqueue */
+#define VIRTIO_BALLOON_F_STATS_PUSH 6 /* Guest pushes stats on a timer */
/* Size of a PFN in the balloon interface. */
#define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -59,6 +60,12 @@ struct virtio_balloon_config {
};
/* Stores PAGE_POISON if page poisoning is in use */
__le32 poison_val;
+ /*
+ * Stats push interval in milliseconds. 0 = disabled (pull only).
+ * Valid with VIRTIO_BALLOON_F_STATS_PUSH. Host-writable, can change
+ * at runtime via config updates.
+ */
+ __le32 stats_push_interval_ms;
};
#define VIRTIO_BALLOON_S_SWAP_IN 0 /* Amount of memory swapped in */
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 1/2] virtio-balloon: extend stats with memory composition and pressure data
From: Gregory Price @ 2026-05-13 16:50 UTC (permalink / raw)
To: virtualization
Cc: linux-kernel, kernel-team, mst, david, jasowang, xuanzhuo,
eperezma, hannes, surenb, peterz, mingo, juri.lelli,
vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, kprateek.nayak
In-Reply-To: <20260513165006.2790857-1-gourry@gourry.net>
When doing proactive ballooning to reduce the size of a guest, some
additional vmstat information is useful in deciding how much pressure
to exert on the VM and when the VM starts experiencing undesirable
behavior (memory and io pressure).
Add 11 new statistics tags to the existing balloon stats virtqueue
for improved balloon sizing decisions. Old hosts ignore unknown tags,
so no feature negotiation is required.
New memory composition stats (bytes):
S_DIRTY: dirty pages awaiting writeback
S_WRITEBACK: pages under active writeback
S_ANON: anonymous pages (for balloon ceiling calculation)
S_INACTIVE_FILE: inactive file LRU (safely reclaimable subset of cache)
S_SLAB_RECLAIM: reclaimable slab memory
New workingset stats (counts):
S_WS_REFAULT_A: anon workingset refaults
S_WS_REFAULT_F: file workingset refaults
New PSI stats (microseconds, cumulative):
S_PSI_MEM_SOME: memory pressure (some stalled)
S_PSI_MEM_FULL: memory pressure (all stalled)
S_PSI_IO_SOME: IO pressure (some stalled)
S_PSI_IO_FULL: IO pressure (all stalled)
Export psi_system for module builds (CONFIG_VIRTIO_BALLOON=m with
CONFIG_PSI=y).
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Gregory Price <gourry@gourry.net>
---
drivers/virtio/virtio_balloon.c | 33 +++++++++++++++++++++++++++++
include/uapi/linux/virtio_balloon.h | 26 +++++++++++++++++++++--
kernel/sched/psi.c | 1 +
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index f6c2dff33f8a..8fa33aec4ce7 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -18,6 +18,8 @@
#include <linux/wait.h>
#include <linux/mm.h>
#include <linux/page_reporting.h>
+#include <linux/vmstat.h>
+#include <linux/psi.h>
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -414,6 +416,37 @@ static unsigned int update_balloon_stats(struct virtio_balloon *vb)
update_stat(vb, idx++, VIRTIO_BALLOON_S_CACHES,
pages_to_bytes(caches));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_DIRTY,
+ pages_to_bytes(global_node_page_state(NR_FILE_DIRTY)));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_WRITEBACK,
+ pages_to_bytes(global_node_page_state(NR_WRITEBACK)));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_ANON,
+ pages_to_bytes(global_node_page_state(NR_ANON_MAPPED)));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_INACTIVE_FILE,
+ pages_to_bytes(global_node_page_state(NR_INACTIVE_FILE)));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_SLAB_RECLAIM,
+ pages_to_bytes(
+ global_node_page_state_pages(NR_SLAB_RECLAIMABLE_B)));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_WS_REFAULT_A,
+ global_node_page_state(WORKINGSET_REFAULT_ANON));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_WS_REFAULT_F,
+ global_node_page_state(WORKINGSET_REFAULT_FILE));
+
+#ifdef CONFIG_PSI
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_PSI_MEM_SOME,
+ div_u64(psi_system.total[PSI_AVGS][PSI_MEM_SOME],
+ NSEC_PER_USEC));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_PSI_MEM_FULL,
+ div_u64(psi_system.total[PSI_AVGS][PSI_MEM_FULL],
+ NSEC_PER_USEC));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_PSI_IO_SOME,
+ div_u64(psi_system.total[PSI_AVGS][PSI_IO_SOME],
+ NSEC_PER_USEC));
+ update_stat(vb, idx++, VIRTIO_BALLOON_S_PSI_IO_FULL,
+ div_u64(psi_system.total[PSI_AVGS][PSI_IO_FULL],
+ NSEC_PER_USEC));
+#endif
+
return idx;
}
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index ee35a372805d..37ec8a8466c4 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -77,7 +77,18 @@ struct virtio_balloon_config {
#define VIRTIO_BALLOON_S_DIRECT_SCAN 13 /* Amount of memory scanned directly */
#define VIRTIO_BALLOON_S_ASYNC_RECLAIM 14 /* Amount of memory reclaimed asynchronously */
#define VIRTIO_BALLOON_S_DIRECT_RECLAIM 15 /* Amount of memory reclaimed directly */
-#define VIRTIO_BALLOON_S_NR 16
+#define VIRTIO_BALLOON_S_DIRTY 16 /* Dirty pages (bytes) */
+#define VIRTIO_BALLOON_S_WRITEBACK 17 /* Pages under writeback (bytes) */
+#define VIRTIO_BALLOON_S_ANON 18 /* Anonymous pages (bytes) */
+#define VIRTIO_BALLOON_S_INACTIVE_FILE 19 /* Inactive file LRU pages (bytes) */
+#define VIRTIO_BALLOON_S_SLAB_RECLAIM 20 /* Reclaimable slab (bytes) */
+#define VIRTIO_BALLOON_S_WS_REFAULT_A 21 /* Workingset refaults anon (count) */
+#define VIRTIO_BALLOON_S_WS_REFAULT_F 22 /* Workingset refaults file (count) */
+#define VIRTIO_BALLOON_S_PSI_MEM_SOME 23 /* PSI memory some total (us) */
+#define VIRTIO_BALLOON_S_PSI_MEM_FULL 24 /* PSI memory full total (us) */
+#define VIRTIO_BALLOON_S_PSI_IO_SOME 25 /* PSI IO some total (us) */
+#define VIRTIO_BALLOON_S_PSI_IO_FULL 26 /* PSI IO full total (us) */
+#define VIRTIO_BALLOON_S_NR 27
#define VIRTIO_BALLOON_S_NAMES_WITH_PREFIX(VIRTIO_BALLOON_S_NAMES_prefix) { \
VIRTIO_BALLOON_S_NAMES_prefix "swap-in", \
@@ -95,7 +106,18 @@ struct virtio_balloon_config {
VIRTIO_BALLOON_S_NAMES_prefix "async-scans", \
VIRTIO_BALLOON_S_NAMES_prefix "direct-scans", \
VIRTIO_BALLOON_S_NAMES_prefix "async-reclaims", \
- VIRTIO_BALLOON_S_NAMES_prefix "direct-reclaims" \
+ VIRTIO_BALLOON_S_NAMES_prefix "direct-reclaims", \
+ VIRTIO_BALLOON_S_NAMES_prefix "dirty", \
+ VIRTIO_BALLOON_S_NAMES_prefix "writeback", \
+ VIRTIO_BALLOON_S_NAMES_prefix "anon-pages", \
+ VIRTIO_BALLOON_S_NAMES_prefix "inactive-file", \
+ VIRTIO_BALLOON_S_NAMES_prefix "slab-reclaimable", \
+ VIRTIO_BALLOON_S_NAMES_prefix "ws-refault-anon", \
+ VIRTIO_BALLOON_S_NAMES_prefix "ws-refault-file", \
+ VIRTIO_BALLOON_S_NAMES_prefix "psi-mem-some-us", \
+ VIRTIO_BALLOON_S_NAMES_prefix "psi-mem-full-us", \
+ VIRTIO_BALLOON_S_NAMES_prefix "psi-io-some-us", \
+ VIRTIO_BALLOON_S_NAMES_prefix "psi-io-full-us" \
}
#define VIRTIO_BALLOON_S_NAMES VIRTIO_BALLOON_S_NAMES_WITH_PREFIX("")
diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index d9c9d9480a45..8ab3aa1c4ef5 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -175,6 +175,7 @@ static DEFINE_PER_CPU(struct psi_group_cpu, system_group_pcpu);
struct psi_group psi_system = {
.pcpu = &system_group_pcpu,
};
+EXPORT_SYMBOL_GPL(psi_system);
static DEFINE_PER_CPU(seqcount_t, psi_seq) = SEQCNT_ZERO(psi_seq);
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 0/2] virtio-balloon: extended stats and push mode
From: Gregory Price @ 2026-05-13 16:50 UTC (permalink / raw)
To: virtualization
Cc: linux-kernel, kernel-team, mst, david, jasowang, xuanzhuo,
eperezma, hannes, surenb, peterz, mingo, juri.lelli,
vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, kprateek.nayak
This series extends the virtio-balloon stats virtqueue with new tags
for memory composition and pressure data, and adds an optional push
mode for guest-initiated stat reporting.
Patch 1 adds 11 new stat tags to the existing stats VQ.
These use the same tag/value format and require no feature negotiation.
Old hosts ignore unknown tags.
The new tags provide data needed for better balloon sizing:
a) dirty/writeback pages (more accurate reclaimable estimates)
b) workingset refault counters (balloon overshoot before PSI spikes)
c) PSI pressure stats
Also exports psi_system for module builds.
Patch 2 adds VIRTIO_BALLOON_F_STATS_PUSH.
Stats push is a new feature that lets the host configure the guest
to push stats on a timer. The host sets stats_push_interval_ms in
the balloon config space; when non-zero, the guest pushes stats at
that cadence without waiting for the host to return the buffer.
This serves two purposes:
a) latency-sensitive consumers get stats at a guaranteed cadence
b) absence of expected stats provides guest liveness detection.
STATS_PUSH requires STATS_VQ and suppresses the pull callback when
active to avoid racing on buffer submission.
Gregory Price (2):
virtio-balloon: extend stats with memory composition and pressure data
virtio-balloon: add stats push mode
drivers/virtio/virtio_balloon.c | 104 ++++++++++++++++++++++++++++
include/uapi/linux/virtio_balloon.h | 33 ++++++++-
kernel/sched/psi.c | 1 +
3 files changed, 136 insertions(+), 2 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH v7 00/31] mm/virtio: skip redundant zeroing of host-zeroed pages
From: Gregory Price @ 2026-05-13 16:34 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Michael S. Tsirkin, linux-kernel, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Muchun Song, Oscar Salvador, Andrew Morton,
Lorenzo Stoakes, Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
Johannes Weiner, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Hugh Dickins, Matthew Brost,
Joshua Hahn, Rakie Kim, Byungchul Park, Ying Huang,
Alistair Popple, Christoph Lameter, David Rientjes,
Roman Gushchin, Harry Yoo, Axel Rasmussen, Yuanchu Xie, Wei Xu,
Chris Li, Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He,
virtualization, linux-mm, Andrea Arcangeli
In-Reply-To: <e7e9b30f-ee5c-44c4-b377-d70156be41e9@kernel.org>
On Wed, May 13, 2026 at 09:36:06AM +0200, David Hildenbrand (Arm) wrote:
>
> Please wait at least a week before sending out new versions of this size.
>
> It's just a lot of noise in people's inbox, and as raised, a lot of people won't
> manage to review this before next month.
>
Also I wasn't even done reviewing what you had previously sent lol.
I can only spare so much time each week on upstream patch review :]
(This is also the pot calling the kettle black, I am guilty of doing
this same thing so uh... lesson learned, sorry David and Andrew!)
~Gregory
^ permalink raw reply
* Re: [PATCH v6 7/7] locking: Add contended_release tracepoint to qrwlock
From: Steven Rostedt @ 2026-05-13 15:43 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-mips, virtualization,
linux-arch, linux-mm, linux-trace-kernel, kernel-team
In-Reply-To: <b67fda8e847fff72da05eff7f799019f8d17ce21.1777999826.git.d@ilvokhin.com>
On Tue, 5 May 2026 17:09:36 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> Extend the contended_release tracepoint to queued rwlocks, using the
> same out-of-line traced unlock approach as queued spinlocks.
>
> Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> ---
> include/asm-generic/qrwlock.h | 22 ++++++++++++++++++++++
> kernel/locking/qrwlock.c | 16 ++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/include/asm-generic/qrwlock.h b/include/asm-generic/qrwlock.h
> index 4b627bafba8b..274c19006125 100644
> --- a/include/asm-generic/qrwlock.h
> +++ b/include/asm-generic/qrwlock.h
> @@ -14,6 +14,7 @@
> #define __ASM_GENERIC_QRWLOCK_H
>
> #include <linux/atomic.h>
> +#include <linux/tracepoint-defs.h>
> #include <asm/barrier.h>
> #include <asm/processor.h>
>
> @@ -35,6 +36,10 @@
> */
> extern void queued_read_lock_slowpath(struct qrwlock *lock);
> extern void queued_write_lock_slowpath(struct qrwlock *lock);
> +extern void queued_read_unlock_traced(struct qrwlock *lock);
> +extern void queued_write_unlock_traced(struct qrwlock *lock);
> +
> +DECLARE_TRACEPOINT(contended_release);
>
> /**
> * queued_read_trylock - try to acquire read lock of a queued rwlock
> @@ -115,6 +120,17 @@ static __always_inline void __queued_read_unlock(struct qrwlock *lock)
> */
> static inline void queued_read_unlock(struct qrwlock *lock)
> {
> + /*
> + * Trace and unlock are combined in the traced unlock variant so
> + * the compiler does not need to preserve the lock pointer across
> + * the function call, avoiding callee-saved register save/restore
> + * on the hot path.
> + */
> + if (tracepoint_enabled(contended_release)) {
> + queued_read_unlock_traced(lock);
Same issue here about duplicating the code.
> + return;
> + }
> +
> __queued_read_unlock(lock);
> }
>
> @@ -129,6 +145,12 @@ static __always_inline void __queued_write_unlock(struct qrwlock *lock)
> */
> static inline void queued_write_unlock(struct qrwlock *lock)
> {
> + /* See comment in queued_read_unlock(). */
> + if (tracepoint_enabled(contended_release)) {
> + queued_write_unlock_traced(lock);
And here.
> + return;
> + }
> +
> __queued_write_unlock(lock);
> }
>
> diff --git a/kernel/locking/qrwlock.c b/kernel/locking/qrwlock.c
> index d2ef312a8611..5ae4b0372719 100644
> --- a/kernel/locking/qrwlock.c
> +++ b/kernel/locking/qrwlock.c
> @@ -90,3 +90,19 @@ void __lockfunc queued_write_lock_slowpath(struct qrwlock *lock)
> trace_contention_end(lock, 0);
> }
> EXPORT_SYMBOL(queued_write_lock_slowpath);
> +
> +void __lockfunc queued_read_unlock_traced(struct qrwlock *lock)
> +{
> + if (queued_rwlock_is_contended(lock))
> + trace_call__contended_release(lock);
Just have this trace and not actually do any locking.
> + __queued_read_unlock(lock);
> +}
> +EXPORT_SYMBOL(queued_read_unlock_traced);
> +
> +void __lockfunc queued_write_unlock_traced(struct qrwlock *lock)
> +{
> + if (queued_rwlock_is_contended(lock))
> + trace_call__contended_release(lock);
Ditto.
-- Steve
> + __queued_write_unlock(lock);
> +}
> +EXPORT_SYMBOL(queued_write_unlock_traced);
^ permalink raw reply
* Re: [PATCH v6 6/7] locking: Factor out __queued_read_unlock()/__queued_write_unlock()
From: Steven Rostedt @ 2026-05-13 15:41 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-mips, virtualization,
linux-arch, linux-mm, linux-trace-kernel, kernel-team,
Paul E. McKenney
In-Reply-To: <8e88613c73f0603c4440ba3a62eb604a5dddc57b.1777999826.git.d@ilvokhin.com>
On Tue, 5 May 2026 17:09:35 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> This is a preparatory refactoring for the next commit, which adds
Same thing about using "next commit" in change logs.
-- Steve
> contended_release tracepoint instrumentation and needs to call the
> unlock from both traced and non-traced paths.
>
> No functional change.
>
> Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> Acked-by: Paul E. McKenney <paulmck@kernel.org>
> ---
> include/asm-generic/qrwlock.h | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
^ permalink raw reply
* Re: [PATCH v6 5/7] locking: Add contended_release tracepoint to qspinlock
From: Steven Rostedt @ 2026-05-13 15:41 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-mips, virtualization,
linux-arch, linux-mm, linux-trace-kernel, kernel-team,
Paul E. McKenney
In-Reply-To: <5d7ea75ffe74a785e6b234ada9f23c6373d4b4c1.1777999826.git.d@ilvokhin.com>
On Tue, 5 May 2026 17:09:34 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> Use the arch-overridable queued_spin_release(), introduced in the
> previous commit, to ensure the tracepoint works correctly across all
Remove the ", introduced in the previous commit," That's useless in git
change logs.
> architectures, including those with custom unlock implementations (e.g.
> x86 paravirt).
>
> When the tracepoint is disabled, the only addition to the hot path is a
> single NOP instruction (the static branch). When enabled, the contention
> check, trace call, and unlock are combined in an out-of-line function to
> minimize hot path impact, avoiding the compiler needing to preserve the
> lock pointer in a callee-saved register across the trace call.
>
> Binary size impact (x86_64, defconfig):
> uninlined unlock (common case): +680 bytes (+0.00%)
> inlined unlock (worst case): +83659 bytes (+0.21%)
>
> The inlined unlock case could not be achieved through Kconfig options on
> x86_64 as PREEMPT_BUILD unconditionally selects UNINLINE_SPIN_UNLOCK on
> x86_64. The UNINLINE_SPIN_UNLOCK guards were manually inverted to force
> inline the unlock path and estimate the worst case binary size increase.
>
> In practice, configurations with UNINLINE_SPIN_UNLOCK=n have already
> opted against binary size optimization, so the inlined worst case is
> unlikely to be a concern.
>
> Architectures with fully custom qspinlock implementations (e.g.
> PowerPC) are not covered by this change.
>
> Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> Acked-by: Paul E. McKenney <paulmck@kernel.org>
> ---
> include/asm-generic/qspinlock.h | 18 ++++++++++++++++++
> kernel/locking/qspinlock.c | 8 ++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
> index df76f34645a0..915a4c2777f6 100644
> --- a/include/asm-generic/qspinlock.h
> +++ b/include/asm-generic/qspinlock.h
> @@ -41,6 +41,7 @@
>
> #include <asm-generic/qspinlock_types.h>
> #include <linux/atomic.h>
> +#include <linux/tracepoint-defs.h>
>
> #ifndef queued_spin_is_locked
> /**
> @@ -129,12 +130,29 @@ static __always_inline void queued_spin_release(struct qspinlock *lock)
> }
> #endif
>
> +DECLARE_TRACEPOINT(contended_release);
> +
> +extern void queued_spin_release_traced(struct qspinlock *lock);
> +
> /**
> * queued_spin_unlock - unlock a queued spinlock
> * @lock : Pointer to queued spinlock structure
> + *
> + * Generic tracing wrapper around the arch-overridable
> + * queued_spin_release().
> */
> static __always_inline void queued_spin_unlock(struct qspinlock *lock)
> {
> + /*
> + * Trace and release are combined in queued_spin_release_traced() so
> + * the compiler does not need to preserve the lock pointer across the
> + * function call, avoiding callee-saved register save/restore on the
> + * hot path.
> + */
> + if (tracepoint_enabled(contended_release)) {
> + queued_spin_release_traced(lock);
> + return;
Get rid of the "return;". What does it save you? It just makes it that you
need to duplicate the code. Even though it's a one liner, it can cause bugs
in the future if this changes. You could call the function:
do_trace_queued_spin_release_traced(lock);
> + }
> queued_spin_release(lock);
> }
>
> diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
> index af8d122bb649..649fdca69288 100644
> --- a/kernel/locking/qspinlock.c
> +++ b/kernel/locking/qspinlock.c
> @@ -104,6 +104,14 @@ static __always_inline u32 __pv_wait_head_or_lock(struct qspinlock *lock,
> #define queued_spin_lock_slowpath native_queued_spin_lock_slowpath
> #endif
>
> +void __lockfunc queued_spin_release_traced(struct qspinlock *lock)
> +{
> + if (queued_spin_is_contended(lock))
> + trace_call__contended_release(lock);
> + queued_spin_release(lock);
And then remove the duplicate call of "queued_spin_release()" here.
-- Steve
> +}
> +EXPORT_SYMBOL(queued_spin_release_traced);
> +
> #endif /* _GEN_PV_LOCK_SLOWPATH */
>
> /**
^ permalink raw reply
* Re: [PATCH v6 4/7] locking: Factor out queued_spin_release()
From: Steven Rostedt @ 2026-05-13 15:37 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
Thomas Bogendoerfer, Juergen Gross, Ajay Kaher, Alexey Makhalov,
Broadcom internal kernel review list, Thomas Gleixner,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Arnd Bergmann,
Dennis Zhou, Tejun Heo, Christoph Lameter, Masami Hiramatsu,
Mathieu Desnoyers, linux-kernel, linux-mips, virtualization,
linux-arch, linux-mm, linux-trace-kernel, kernel-team,
Paul E. McKenney
In-Reply-To: <64c202b8a76a7d98515cf10cc1f99ecb0a9a7ccf.1777999826.git.d@ilvokhin.com>
On Tue, 5 May 2026 17:09:33 +0000
Dmitry Ilvokhin <d@ilvokhin.com> wrote:
> Introduce queued_spin_release() as an arch-overridable unlock primitive,
> and make queued_spin_unlock() a generic wrapper around it.
>
> This is a preparatory refactoring for the next commit, which adds
> contended_release tracepoint instrumentation to queued_spin_unlock().
In change logs, do not use "next commit". Instead say something like:
In preparation for adding contended_release tracepoint instrumentation to
queued_spin_unlock(), refactor the code to allow out of line calls when
the tracepoint is enabled.
Or something like that.
-- Steve
>
> Rename the existing arch-specific queued_spin_unlock() overrides on
> x86 (paravirt) and MIPS to queued_spin_release().
>
> No functional change.
>
> Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
^ permalink raw reply
* [PATCH v2] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Polina Vishneva @ 2026-05-13 14:50 UTC (permalink / raw)
To: linux-kernel
Cc: netdev, virtualization, kvm, Eugenio Pérez, Jason Wang,
Michael S. Tsirkin, Stefano Garzarella, Stefan Hajnoczi,
Polina Vishneva, Denis V . Lunev
From: "Denis V. Lunev" <den@openvz.org>
When the host initiates an AF_VSOCK connect() to a guest that has not
yet loaded the virtio-vsock transport (i.e. still booting), the caller
blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT.
A caller that wants to know if the guest is up yet instead of waiting
could theoretically tune SO_VM_SOCKETS_CONNECT_TIMEOUT, but it's tricky
to find the right timeout, if not impossible: there's no way to
distinguish "guest won't reply because it's not up yet" vs "guest is up
and tried to reply, but was too slow".
Furthermore, this delay is pointless:
- If the guest doesn't initialize within this timeout, connect()
returns ETIMEDOUT.
- If the guest **does** initialize, it'll reply with RST immediately,
because there won't be a listener on the port yet; connect() returns
ECONNRESET.
That's also inconsistent with the behavior at other initialization
stages: if a connection is attempted when the guest driver is already
loaded, but nothing is listening yet, we return ECONNRESET immediately
without waiting.
Fix this by checking the RX virtqueue backend in
vhost_transport_send_pkt() before queuing. If it's NULL, return
-EHOSTUNREACH immediately.
Callers that used to get ETIMEDOUT will now usually get EHOSTUNREACH.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
---
v2:
- ECONNREFUSED -> EHOSTUNREACH.
- Use vhost_transport_do_send_pkt() instead of raw .private_data access.
- Removed READ_ONCE().
- Wrapped the condition with unlikely().
- Updated the comment and the commit message.
v1: https://lore.kernel.org/netdev/20260511145610.413210-1-polina.vishneva@virtuozzo.com
drivers/vhost/vsock.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 1d8ec6bed53e..9aaab6bb8061 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -302,6 +302,22 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
return -ENODEV;
}
+ /* Fast-fail if the guest hasn't enabled the RX vq yet. Queuing the packet
+ * and making the caller wait is pointless: even if the guest manages to init
+ * within the timeout, it'll immediately reply with RST, because there's no
+ * listener on the port yet.
+ *
+ * vhost_vq_get_backend() without vq->mutex is acceptable here: locking
+ * the mutex would be too expensive in this hot path, and we already have
+ * all the outcomes covered: if the backend becomes NULL right after the check,
+ * vhost_transport_do_send_pkt() will check it under the mutex anyway.
+ */
+ if (unlikely(!data_race(vhost_vq_get_backend(&vsock->vqs[VSOCK_VQ_RX])))) {
+ rcu_read_unlock();
+ kfree_skb(skb);
+ return -EHOSTUNREACH;
+ }
+
if (virtio_vsock_skb_reply(skb))
atomic_inc(&vsock->queued_replies);
base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] virtio_pci_modern: Use GFP_ATOMIC with spin_lock_irqsave held in virtqueue_exec_admin_cmd()
From: Michael S. Tsirkin @ 2026-05-13 12:34 UTC (permalink / raw)
To: Jinhui Guo
Cc: David Laight, Eugenio Pérez, Jason Wang, Jiri Pirko,
Xuan Zhuo, linux-kernel, stable, virtualization
In-Reply-To: <20260413122244.534-1-guojinhui.liam@bytedance.com>
On Mon, Apr 13, 2026 at 08:22:44PM +0800, Jinhui Guo wrote:
> On Mon, Apr 13, 2026 at 10:17:59 +0100, David Laight wrote:
> > Or do the allocate before acquiring the lock (and free it not used
> > in the error path).
>
> Hi David,
>
> Thanks for the suggestion.
>
> Pre-allocating the memory outside the lock is indeed a good practice,
> but unfortunately it doesn't work in this specific virtqueue context.
>
> The kmalloc() in question is not happening at the virtqueue_exec_admin_cmd()
> level. Instead, it is deeply embedded inside virtqueue_add_sgs()
> (specifically, in functions like alloc_indirect_split() or
> virtqueue_add_indirect_packed()) to allocate indirect descriptors when
> multiple SG elements are provided.
>
> As a caller, we have no mechanism to pre-allocate this indirect descriptor
> memory and pass it down to virtqueue_add_sgs(). Furthermore, virtqueue_add_sgs()
> needs to atomically check the queue's num_free status, allocate the indirect
> table if necessary, and update the queue pointers. All these operations
> must be protected by admin_vq->lock to prevent concurrent admin command
> submissions from corrupting the virtqueue state.
>
> Therefore, allocating before acquiring the lock isn't feasible here, and
> replacing GFP_KERNEL with GFP_ATOMIC (with a proper sleepable retry upon
> failure) seems to be the more viable fix.
>
> Does this make sense?
>
> Thanks,
> Jinhui
it might be quite ok. what is missing is the analysis of whether we
can actually get this error and what happens then.
^ permalink raw reply
* Re: [PATCH] virtio_pci_modern: Use GFP_ATOMIC with spin_lock_irqsave held in virtqueue_exec_admin_cmd()
From: Michael S. Tsirkin @ 2026-05-13 12:10 UTC (permalink / raw)
To: Jinhui Guo
Cc: Eugenio Pérez, Jason Wang, Jiri Pirko, Xuan Zhuo,
linux-kernel, stable, virtualization
In-Reply-To: <20260413100013.32399-1-guojinhui.liam@bytedance.com>
On Mon, Apr 13, 2026 at 06:00:13PM +0800, Jinhui Guo wrote:
> On Mon, Apr 13, 2026 at 03:45:20 -0400, "Michael S. Tsirkin" wrote:
> > GFP_ATOMIC allocations can and will fail. If using them, one must
> > retry, not just propagate failures.
> > Or just switch admin_vq->lock to a mutex?
>
> Hi Michael,
>
> Thank you for the review.
>
> Regarding the suggestion to switch admin_vq->lock to a mutex:
>
> The virtqueue callback vp_modern_avq_done() holds admin_vq->lock and
> runs in an interrupt handler context, making it impractical to replace
> the spinlock with a mutex directly.
>
> I considered deferring the completion to a workqueue so we could safely
> use a mutex, but since this is a bug fix destined for stable@vger.kernel.org,
> doing so would introduce significant code churn (e.g., handling INIT_WORK,
> cancel_work_sync during cleanup, etc.) and increase the risk for backports.
This is not how we do kernel development here. Please fix the bug
upstream first then we will consider backporting strategies.
> Therefore, using GFP_ATOMIC with the existing spinlock seems to be the most
> minimal and safest approach for a fix.
>
> However, just replacing GFP_KERNEL with GFP_ATOMIC isn't entirely safe
> because of how virtqueue_add_sgs() handles allocation failures. If kmalloc()
> fails under memory pressure with GFP_ATOMIC, the function falls back to using
> direct descriptors. If there are not enough free direct descriptors, it
> ultimately returns -ENOSPC.
>
> In the current code, -ENOSPC is handled with a busy loop:
>
> if (ret == -ENOSPC) {
> spin_unlock_irqrestore(&admin_vq->lock, flags);
> cpu_relax();
> goto again;
> }
>
> If the -ENOSPC is actually caused by a GFP_ATOMIC allocation failure under
> memory pressure, this cpu_relax() loop will never yield the CPU to memory
> reclaim mechanisms (like kswapd), potentially leading to a soft lockup.
>
> To properly handle both actual queue-full conditions and GFP_ATOMIC failures,
> I propose replacing cpu_relax() with a sleep (e.g., usleep_range(10, 100)).
> This allows memory reclaim to run while we wait.
>
> I plan to send out a v2 patch with this modification:
>
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -101,11 +101,11 @@ static int virtqueue_exec_admin_cmd(struct virtio_pci_admin_vq *admin_vq,
> return -EIO;
>
> spin_lock_irqsave(&admin_vq->lock, flags);
> - ret = virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, GFP_KERNEL);
> + ret = virtqueue_add_sgs(vq, sgs, out_num, in_num, cmd, GFP_ATOMIC);
> if (ret < 0) {
> if (ret == -ENOSPC) {
> spin_unlock_irqrestore(&admin_vq->lock, flags);
> - cpu_relax();
> + usleep_range(10, 100);
> goto again;
> }
> goto unlock_err;
>
> Does this approach align with your expectations for the fix?
>
> Thanks,
> Jinhui
Nope.
I think we need to get out of peephole mode where we are just looking
at the warnings and "fix" them by 1 line tweaks and actually analyze the
codepaths. GFP is just for indirect allocations and VQ already
falls back to using direct when that fails.
The question is:
- what is going on with VQ ring state, can we actually get into a situation
where indirect would succeed but direct fails?
- how can callers either prevent failures or get notified when buffers
have been used?
And it is quite possible that the fix in the end is exactly your v1 but
with the analysis in the commit log explaining why this fixes the
problem and does not paper over it.
--
MST
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Polina Vishneva @ 2026-05-13 11:18 UTC (permalink / raw)
To: sgarzare@redhat.com
Cc: den@openvz.org, virtualization@lists.linux.dev,
stefanha@redhat.com, eperezma@redhat.com,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
kvm@vger.kernel.org, mst@redhat.com, jasowang@redhat.com
In-Reply-To: <agM8cxqUCJNjBDlm@sgarzare-redhat>
On Tue, 2026-05-12 at 17:39 +0200, Stefano Garzarella wrote:
> On Tue, May 12, 2026 at 02:32:14PM +0000, Polina Vishneva wrote:
> > On Mon, 2026-05-11 at 17:56 +0200, Stefano Garzarella wrote:
> > > On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
> > > > From: "Denis V. Lunev" <den@openvz.org>
> > > >
> > > > When the host initiates an AF_VSOCK connect() to a guest that has not
> > > > yet loaded the virtio-vsock transport (i.e. still booting), the caller
> > > > blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
> > > > vhost_transport_do_send_pkt() silently exits when
> > > > vhost_vq_get_backend(vq) returns NULL.
> > >
> > > Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
> >
> > It can, but it might be difficult to find a correct timeout.
> >
> > And, generally, there's no way to distinguish "the guest hasn't yet initialized
> > the vq" from "the guest is up and running, but didn't reply to connect() in
> > time". That's exactly what this patch is attempting to fix.
>
> Okay, so please mention this in the commit message, I mean why
> SO_VM_SOCKETS_CONNECT_TIMEOUT can't really help.
Will do.
>
> >
> > >
> > > >
> > > > If the guest doesn't start listening within this timeout, connect()
> > > > returns ETIMEDOUT.
> > > >
> > > > This delay is usually pointless and it doesn't well align with our
>
> I still don't understand why this is pointless. If an application wants
> to wait while sleeping, it can simply increase the timeout long enough
> to wait for the VM to start up and use a single `connect()` call,
> instead of continuing to try and wasting CPU cycles unnecessarily.
>
> Hmm, or maybe not, because the driver will definitely be initialized
> before the application that wants to listen on that port, so it will
> respond that no one is listening, and the `connect()` call will fail
> with an `ECONNRESET` error in any case. Right?
That's the case indeed.
>
> If it is the case, is the following line in the commit description
> correct?
>
> If the guest doesn't start listening within this timeout, connect()
> returns ETIMEDOUT.
>
> I mean, also if the application starts to listen within the timeout, I
> think the connect() will fail in any case as I pointed out above (this
> should be another point in favour of this change)
Yes, the commit message should be updated, as well as the code comment.
>
>
> BTW, I think we should explain this more clearly both here and briefly
> in the code as well.
Definitely.
>
> > > > behavior at other initialization stages: for example, if a connection is
> > > > attempted when the guest driver is already loaded, but when nothing is
> > > > listening yet, it returns ECONNRESET immediately without any wait.
> > > >
> > > > Fix this by checking the RX virtqueue backend in
> > > > vhost_transport_send_pkt() before queuing. If the backend is NULL,
> > > > return -ECONNREFUSED immediately.
> > > >
> > > > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > > > Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > > > Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > > > ---
> > > > drivers/vhost/vsock.c | 10 ++++++++++
> > > > 1 file changed, 10 insertions(+)
> > > >
> > > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > > index 1d8ec6bed53e..a3f218292c3a 100644
> > > > --- a/drivers/vhost/vsock.c
> > > > +++ b/drivers/vhost/vsock.c
> > > > @@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> > > > return -ENODEV;
> > > > }
> > > >
> > > > + /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
> > > > + * private_data without vq->mutex is deliberate: even if the backend becomes
> > > > + * NULL right after that check, do_send_pkt() checks it under the mutex.
> > > > + */
> > > > + if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
> > >
> > > Why not using vhost_vq_get_backend() ?
> >
> > Because it locks the mutex, which is slow and unacceptable in this hot
> > path.
>
> ehm, sorry, which mutex are you talking about?
>
> I see just a comment about the mutex to be acquired by the caller, but I
> don't see any lock there.
Apparently the comment in vhost.h says "Context: Need to call with vq->mutex
acquired.", but I guess we're safe to ignore this and use it instead of
accessing private_data manually, thanks for pointing this out.
>
> >
> > >
> > > Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
> >
> > It's racy, but as described here in the comment and in the commit message,
> > any possible race outcome is covered by the subsequent checks.
>
> Okay, so what is the point to call READ_ONCE()?
Probably none, it was just there in the initial patch version, and I've decided
not to drop it when adding data_race(). Will drop.
>
> >
> > > > {
> > > > + rcu_read_unlock();
> > > > + kfree_skb(skb);
> > > > + return -ECONNREFUSED;
> > >
> > > This is a generic send_pkt, is it okay to return ECONNREFUSED in any
> > > case?
> >
> > EHOSTUNREACH would probably be better.
> > All the current send_pkt functions only return ENODEV, but it has different
> > semantics: they mean that the local device isn't yet ready, while there we're
> > dealing with the opposite end not being ready.
>
> In the AF_VSOCK prespective, I see ENODEV like the transport is not
> ready, so I think it can eventually fit here too, but also EHOSTUNREACH
> is fine, for sure better than ECONNREFUSED.
EHOSTUNREACH is indeed a better fit, agreed.
>
> Thanks,
> Stefano
>
> >
> > Best regards, Polina.
> >
> > >
> > > Thanks,
> > > Stefano
> > >
> > > > + }
> > > > +
> > > > if (virtio_vsock_skb_reply(skb))
> > > > atomic_inc(&vsock->queued_replies);
> > > >
> > > >
> > > > base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
> > > > --
> > > > 2.53.0
> > > >
^ permalink raw reply
* [PATCH net v3 2/2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-13 10:54 UTC (permalink / raw)
To: netdev
Cc: Xuan Zhuo, Michael S. Tsirkin, Eugenio Pérez, linux-kernel,
Simon Horman, Paolo Abeni, Jakub Kicinski, Stefano Garzarella,
Jason Wang, kvm, Stefan Hajnoczi, virtualization, Eric Dumazet,
David S. Miller
In-Reply-To: <20260513105417.56761-1-sgarzare@redhat.com>
From: Stefano Garzarella <sgarzare@redhat.com>
After commit 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb
queue"), virtio_transport_inc_rx_pkt() subtracts per-skb overhead from
buf_alloc when checking whether a new packet fits. This reduces the
effective receive buffer below what the user configured via
SO_VM_SOCKETS_BUFFER_SIZE, causing legitimate data packets to be
silently dropped and applications that rely on the full buffer size
to deadlock.
Also, the reduced space is not communicated to the remote peer, so
its credit calculation accounts more credit than the receiver will
actually accept, causing data loss (there is no retransmission).
With this approach we currently have failures in
tools/testing/vsock/vsock_test.c. Test 18 sometimes fails, while
test 22 always fails in this way:
18 - SOCK_STREAM MSG_ZEROCOPY...hash mismatch
22 - SOCK_STREAM virtio credit update + SO_RCVLOWAT...send failed:
Resource temporarily unavailable
Fix this by using `buf_alloc * 2` as the total budget for payload plus
skb overhead in virtio_transport_inc_rx_pkt(), similar to how SO_RCVBUF
is doubled to reserve space for sk_buff metadata. This preserves the
full buf_alloc for payload under normal operation, while still bounding
the skb queue growth.
With this patch, all tests in tools/testing/vsock/vsock_test.c are
now passing again.
Fixes: 059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 4a4ac69d1ad1..e22117bf5dcd 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -434,7 +434,10 @@ static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
{
u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) * SKB_TRUESIZE(0);
- if (skb_overhead + vvs->buf_used + len > vvs->buf_alloc)
+ /* Use buf_alloc * 2 as total budget (payload + overhead), similar to
+ * how SO_RCVBUF is doubled to reserve space for sk_buff metadata.
+ */
+ if (skb_overhead + vvs->buf_used + len > (u64)vvs->buf_alloc * 2)
return false;
vvs->rx_bytes += len;
--
2.54.0
^ permalink raw reply related
* [PATCH net v3 1/2] vsock/virtio: reset connection on receiving queue overflow
From: Stefano Garzarella @ 2026-05-13 10:54 UTC (permalink / raw)
To: netdev
Cc: Xuan Zhuo, Michael S. Tsirkin, Eugenio Pérez, linux-kernel,
Simon Horman, Paolo Abeni, Jakub Kicinski, Stefano Garzarella,
Jason Wang, kvm, Stefan Hajnoczi, virtualization, Eric Dumazet,
David S. Miller
In-Reply-To: <20260513105417.56761-1-sgarzare@redhat.com>
From: Stefano Garzarella <sgarzare@redhat.com>
When there is no more space to queue an incoming packet, the packet is
silently dropped. This causes data loss without any notification to
either peer, since there is no retransmission.
Under normal circumstances, this should never happen. However, it could
happen if the other peer doesn't respect the credit, or if the skb
overhead, which we recently began to take into account with commit
059b7dbd20a6 ("vsock/virtio: fix potential unbounded skb queue"),
is too high.
Fix this by resetting the connection and setting the local socket error
to ENOBUFS when virtio_transport_recv_enqueue() can no longer queue a
packet, so both peers are explicitly notified of the failure rather than
silently losing data.
Fixes: ae6fcfbf5f03 ("vsock/virtio: discard packets if credit is not respected")
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
net/vmw_vsock/virtio_transport_common.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 989cc252d3d3..4a4ac69d1ad1 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1350,7 +1350,7 @@ virtio_transport_recv_connecting(struct sock *sk,
return err;
}
-static void
+static bool
virtio_transport_recv_enqueue(struct vsock_sock *vsk,
struct sk_buff *skb)
{
@@ -1365,10 +1365,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
spin_lock_bh(&vvs->rx_lock);
can_enqueue = virtio_transport_inc_rx_pkt(vvs, len);
- if (!can_enqueue) {
- free_pkt = true;
+ if (!can_enqueue)
goto out;
- }
if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SEQ_EOM)
vvs->msg_count++;
@@ -1408,6 +1406,8 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
spin_unlock_bh(&vvs->rx_lock);
if (free_pkt)
kfree_skb(skb);
+
+ return can_enqueue;
}
static int
@@ -1420,7 +1420,16 @@ virtio_transport_recv_connected(struct sock *sk,
switch (le16_to_cpu(hdr->op)) {
case VIRTIO_VSOCK_OP_RW:
- virtio_transport_recv_enqueue(vsk, skb);
+ if (!virtio_transport_recv_enqueue(vsk, skb)) {
+ /* There is no more space to queue the packet, so let's
+ * close the connection; otherwise, we'll lose data.
+ */
+ (void)virtio_transport_reset(vsk, skb);
+ sk->sk_state = TCP_CLOSE;
+ sk->sk_err = ENOBUFS;
+ sk_error_report(sk);
+ break;
+ }
vsock_data_ready(sk);
return err;
case VIRTIO_VSOCK_OP_CREDIT_REQUEST:
--
2.54.0
^ permalink raw reply related
* [PATCH net v3 0/2] vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
From: Stefano Garzarella @ 2026-05-13 10:54 UTC (permalink / raw)
To: netdev
Cc: Xuan Zhuo, Michael S. Tsirkin, Eugenio Pérez, linux-kernel,
Simon Horman, Paolo Abeni, Jakub Kicinski, Stefano Garzarella,
Jason Wang, kvm, Stefan Hajnoczi, virtualization, Eric Dumazet,
David S. Miller
The code is exactly the same as in v2, but split into two patches as
Michael suggested in v2:
- Patch 1 resets the connection when we can no longer queue packets,
this prevents silent data loss, and both peers are notified.
- Patch 2 increases the total budget to `buf_alloc * 2` for payload
plus skb overhead similar to how SO_RCVBUF is doubled to reserve
space for sk_buff metadata. This preserves the full buf_alloc for
payload under normal operation, while still bounding the skb queue
growth.
v3:
- Split in 2 patches [MST]
v2: https://lore.kernel.org/netdev/20260512080737.36787-1-sgarzare@redhat.com/
- Close the connection when we can no longer queue new packets instead
of losing data.
- No longer announce the reduced buf_alloc to avoid violating the
spec. [MST]
v1: https://lore.kernel.org/netdev/20260508092330.69690-1-sgarzare@redhat.com/
Stefano Garzarella (2):
vsock/virtio: reset connection on receiving queue overflow
vsock/virtio: fix skb overhead accounting to preserve full buf_alloc
net/vmw_vsock/virtio_transport_common.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Denis V. Lunev @ 2026-05-13 10:34 UTC (permalink / raw)
To: Michael S. Tsirkin, Polina Vishneva
Cc: sgarzare@redhat.com, den@openvz.org,
virtualization@lists.linux.dev, stefanha@redhat.com,
eperezma@redhat.com, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, kvm@vger.kernel.org, jasowang@redhat.com
In-Reply-To: <20260513060124-mutt-send-email-mst@kernel.org>
On 5/13/26 12:03, Michael S. Tsirkin wrote:
> On Wed, May 13, 2026 at 09:44:49AM +0000, Polina Vishneva wrote:
>> On Tue, 2026-05-12 at 12:02 -0400, Michael S. Tsirkin wrote:
>>> On Tue, May 12, 2026 at 05:39:48PM +0200, Stefano Garzarella wrote:
>>>> On Tue, May 12, 2026 at 02:32:14PM +0000, Polina Vishneva wrote:
>>>>> On Mon, 2026-05-11 at 17:56 +0200, Stefano Garzarella wrote:
>>>>>> On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
>>>>>>> From: "Denis V. Lunev" <den@openvz.org>
>>>>>>>
>>>>>>> When the host initiates an AF_VSOCK connect() to a guest that has not
>>>>>>> yet loaded the virtio-vsock transport (i.e. still booting), the caller
>>>>>>> blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
>>>>>>> vhost_transport_do_send_pkt() silently exits when
>>>>>>> vhost_vq_get_backend(vq) returns NULL.
>>>>>> Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
>>>>> It can, but it might be difficult to find a correct timeout.
>>>>>
>>>>> And, generally, there's no way to distinguish "the guest hasn't yet initialized
>>>>> the vq" from "the guest is up and running, but didn't reply to connect() in
>>>>> time". That's exactly what this patch is attempting to fix.
>>>> Okay, so please mention this in the commit message, I mean why
>>>> SO_VM_SOCKETS_CONNECT_TIMEOUT can't really help.
>>>>
>>>>>>> If the guest doesn't start listening within this timeout, connect()
>>>>>>> returns ETIMEDOUT.
>>>>>>>
>>>>>>> This delay is usually pointless and it doesn't well align with our
>>>> I still don't understand why this is pointless. If an application wants to
>>>> wait while sleeping, it can simply increase the timeout long enough to wait
>>>> for the VM to start up and use a single `connect()` call, instead of
>>>> continuing to try and wasting CPU cycles unnecessarily.
>>>>
>>>> Hmm, or maybe not, because the driver will definitely be initialized before
>>>> the application that wants to listen on that port, so it will respond that
>>>> no one is listening, and the `connect()` call will fail with an `ECONNRESET`
>>>> error in any case. Right?
>>>>
>>>> If it is the case, is the following line in the commit description correct?
>>>>
>>>> If the guest doesn't start listening within this timeout, connect()
>>>> returns ETIMEDOUT.
>>>>
>>>> I mean, also if the application starts to listen within the timeout, I think
>>>> the connect() will fail in any case as I pointed out above (this should be
>>>> another point in favour of this change)
>>>>
>>>>
>>>> BTW, I think we should explain this more clearly both here and briefly in
>>>> the code as well.
>>>>
>>>>>>> behavior at other initialization stages: for example, if a connection is
>>>>>>> attempted when the guest driver is already loaded, but when nothing is
>>>>>>> listening yet, it returns ECONNRESET immediately without any wait.
>>>>>>>
>>>>>>> Fix this by checking the RX virtqueue backend in
>>>>>>> vhost_transport_send_pkt() before queuing. If the backend is NULL,
>>>>>>> return -ECONNREFUSED immediately.
>>>>>>>
>>>>>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>>>>>> Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>>>>>>> Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
>>>>>>> ---
>>>>>>> drivers/vhost/vsock.c | 10 ++++++++++
>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>>>>>>> index 1d8ec6bed53e..a3f218292c3a 100644
>>>>>>> --- a/drivers/vhost/vsock.c
>>>>>>> +++ b/drivers/vhost/vsock.c
>>>>>>> @@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
>>>>>>> return -ENODEV;
>>>>>>> }
>>>>>>>
>>>>>>> + /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
>>>>>>> + * private_data without vq->mutex is deliberate: even if the backend becomes
>>>>>>> + * NULL right after that check, do_send_pkt() checks it under the mutex.
>>>>>>> + */
>>>>>>> + if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
>>>>>> Why not using vhost_vq_get_backend() ?
>>>>> Because it locks the mutex, which is slow and unacceptable in this hot
>>>>> path.
>>>> ehm, sorry, which mutex are you talking about?
>>>>
>>>> I see just a comment about the mutex to be acquired by the caller, but I
>>>> don't see any lock there.
>>>>
>>>>>> Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
>>>>> It's racy, but as described here in the comment and in the commit message,
>>>>> any possible race outcome is covered by the subsequent checks.
>>>> Okay, so what is the point to call READ_ONCE()?
>>>>
>>>>>>> {
>>>>>>> + rcu_read_unlock();
>>>>>>> + kfree_skb(skb);
>>>>>>> + return -ECONNREFUSED;
>>>>>> This is a generic send_pkt, is it okay to return ECONNREFUSED in any
>>>>>> case?
>>>>> EHOSTUNREACH would probably be better.
>>>>> All the current send_pkt functions only return ENODEV, but it has different
>>>>> semantics: they mean that the local device isn't yet ready, while there we're
>>>>> dealing with the opposite end not being ready.
>>>> In the AF_VSOCK prespective, I see ENODEV like the transport is not ready,
>>>> so I think it can eventually fit here too, but also EHOSTUNREACH is fine,
>>>> for sure better than ECONNREFUSED.
>>>>
>>>> Thanks,
>>>> Stefano
>>> I think it's worth trying to do the same thing with e.g. TCP
>>> and see what error, if any, we get. Match that.
>> This case is not directly applicable to TCP: in TCP, there's no out-of-band way
>> to detect the "host up, but not initialized yet and not ready for connections"
>> state: this could theoretically be ENOPROTOOPT, but no real TCP stack implement
>> this, because replying with ICMP_PROT_UNREACH requires a TCP stack, which is
>> exactly the thing that isn't up.
>>
>> So, in real world, a similar situation with TCP would result in ETIMEDOUT.
> Then it just might be best to keep the current behaviour which seems
> to match that pretty closely?
My motivation on this is very simple.
1. The guest is not configured rings yet
2. Connection request arrives, waiting is started
3. Guest has configured rings, assuming initialization is performed.
Important - there is no process listening in guest on the socket
yet, even in Linux when dependency will start next service this
will require some time
4. The queued connection request in host revives and gets -ECONNREFUSED
in my understanding. The destiny of the packet arrived just in this
window would be the same
5. Guest listener starts
At my opinion packets coming in the moment 2 should have the same
resolution as packets arriving in 4, without timeout as if these
packets will be served in the moment 3 they should get this resolution.
No need to wait.
Den
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Michael S. Tsirkin @ 2026-05-13 10:03 UTC (permalink / raw)
To: Polina Vishneva
Cc: sgarzare@redhat.com, den@openvz.org,
virtualization@lists.linux.dev, stefanha@redhat.com,
eperezma@redhat.com, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, kvm@vger.kernel.org, jasowang@redhat.com
In-Reply-To: <8ae7e443034026eda016322d22da52700e432f09.camel@virtuozzo.com>
On Wed, May 13, 2026 at 09:44:49AM +0000, Polina Vishneva wrote:
> On Tue, 2026-05-12 at 12:02 -0400, Michael S. Tsirkin wrote:
> > On Tue, May 12, 2026 at 05:39:48PM +0200, Stefano Garzarella wrote:
> > > On Tue, May 12, 2026 at 02:32:14PM +0000, Polina Vishneva wrote:
> > > > On Mon, 2026-05-11 at 17:56 +0200, Stefano Garzarella wrote:
> > > > > On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
> > > > > > From: "Denis V. Lunev" <den@openvz.org>
> > > > > >
> > > > > > When the host initiates an AF_VSOCK connect() to a guest that has not
> > > > > > yet loaded the virtio-vsock transport (i.e. still booting), the caller
> > > > > > blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
> > > > > > vhost_transport_do_send_pkt() silently exits when
> > > > > > vhost_vq_get_backend(vq) returns NULL.
> > > > >
> > > > > Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
> > > >
> > > > It can, but it might be difficult to find a correct timeout.
> > > >
> > > > And, generally, there's no way to distinguish "the guest hasn't yet initialized
> > > > the vq" from "the guest is up and running, but didn't reply to connect() in
> > > > time". That's exactly what this patch is attempting to fix.
> > >
> > > Okay, so please mention this in the commit message, I mean why
> > > SO_VM_SOCKETS_CONNECT_TIMEOUT can't really help.
> > >
> > > >
> > > > >
> > > > > >
> > > > > > If the guest doesn't start listening within this timeout, connect()
> > > > > > returns ETIMEDOUT.
> > > > > >
> > > > > > This delay is usually pointless and it doesn't well align with our
> > >
> > > I still don't understand why this is pointless. If an application wants to
> > > wait while sleeping, it can simply increase the timeout long enough to wait
> > > for the VM to start up and use a single `connect()` call, instead of
> > > continuing to try and wasting CPU cycles unnecessarily.
> > >
> > > Hmm, or maybe not, because the driver will definitely be initialized before
> > > the application that wants to listen on that port, so it will respond that
> > > no one is listening, and the `connect()` call will fail with an `ECONNRESET`
> > > error in any case. Right?
> > >
> > > If it is the case, is the following line in the commit description correct?
> > >
> > > If the guest doesn't start listening within this timeout, connect()
> > > returns ETIMEDOUT.
> > >
> > > I mean, also if the application starts to listen within the timeout, I think
> > > the connect() will fail in any case as I pointed out above (this should be
> > > another point in favour of this change)
> > >
> > >
> > > BTW, I think we should explain this more clearly both here and briefly in
> > > the code as well.
> > >
> > > > > > behavior at other initialization stages: for example, if a connection is
> > > > > > attempted when the guest driver is already loaded, but when nothing is
> > > > > > listening yet, it returns ECONNRESET immediately without any wait.
> > > > > >
> > > > > > Fix this by checking the RX virtqueue backend in
> > > > > > vhost_transport_send_pkt() before queuing. If the backend is NULL,
> > > > > > return -ECONNREFUSED immediately.
> > > > > >
> > > > > > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > > > > > Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > > > > > Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > > > > > ---
> > > > > > drivers/vhost/vsock.c | 10 ++++++++++
> > > > > > 1 file changed, 10 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > > > > index 1d8ec6bed53e..a3f218292c3a 100644
> > > > > > --- a/drivers/vhost/vsock.c
> > > > > > +++ b/drivers/vhost/vsock.c
> > > > > > @@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> > > > > > return -ENODEV;
> > > > > > }
> > > > > >
> > > > > > + /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
> > > > > > + * private_data without vq->mutex is deliberate: even if the backend becomes
> > > > > > + * NULL right after that check, do_send_pkt() checks it under the mutex.
> > > > > > + */
> > > > > > + if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
> > > > >
> > > > > Why not using vhost_vq_get_backend() ?
> > > >
> > > > Because it locks the mutex, which is slow and unacceptable in this hot
> > > > path.
> > >
> > > ehm, sorry, which mutex are you talking about?
> > >
> > > I see just a comment about the mutex to be acquired by the caller, but I
> > > don't see any lock there.
> > >
> > > >
> > > > >
> > > > > Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
> > > >
> > > > It's racy, but as described here in the comment and in the commit message,
> > > > any possible race outcome is covered by the subsequent checks.
> > >
> > > Okay, so what is the point to call READ_ONCE()?
> > >
> > > >
> > > > > > {
> > > > > > + rcu_read_unlock();
> > > > > > + kfree_skb(skb);
> > > > > > + return -ECONNREFUSED;
> > > > >
> > > > > This is a generic send_pkt, is it okay to return ECONNREFUSED in any
> > > > > case?
> > > >
> > > > EHOSTUNREACH would probably be better.
> > > > All the current send_pkt functions only return ENODEV, but it has different
> > > > semantics: they mean that the local device isn't yet ready, while there we're
> > > > dealing with the opposite end not being ready.
> > >
> > > In the AF_VSOCK prespective, I see ENODEV like the transport is not ready,
> > > so I think it can eventually fit here too, but also EHOSTUNREACH is fine,
> > > for sure better than ECONNREFUSED.
> > >
> > > Thanks,
> > > Stefano
> >
> > I think it's worth trying to do the same thing with e.g. TCP
> > and see what error, if any, we get. Match that.
>
> This case is not directly applicable to TCP: in TCP, there's no out-of-band way
> to detect the "host up, but not initialized yet and not ready for connections"
> state: this could theoretically be ENOPROTOOPT, but no real TCP stack implement
> this, because replying with ICMP_PROT_UNREACH requires a TCP stack, which is
> exactly the thing that isn't up.
>
> So, in real world, a similar situation with TCP would result in ETIMEDOUT.
Then it just might be best to keep the current behaviour which seems
to match that pretty closely?
> >
> >
> > > >
> > > > Best regards, Polina.
> > > >
> > > > >
> > > > > Thanks,
> > > > > Stefano
> > > > >
> > > > > > + }
> > > > > > +
> > > > > > if (virtio_vsock_skb_reply(skb))
> > > > > > atomic_inc(&vsock->queued_replies);
> > > > > >
> > > > > >
> > > > > > base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
> > > > > > --
> > > > > > 2.53.0
> > > > > >
^ permalink raw reply
* Re: [PATCH] vhost/vsock: Refuse the connection immediately when guest isn't ready
From: Polina Vishneva @ 2026-05-13 9:44 UTC (permalink / raw)
To: sgarzare@redhat.com, mst@redhat.com
Cc: den@openvz.org, virtualization@lists.linux.dev,
stefanha@redhat.com, eperezma@redhat.com,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
kvm@vger.kernel.org, jasowang@redhat.com
In-Reply-To: <20260512120019-mutt-send-email-mst@kernel.org>
On Tue, 2026-05-12 at 12:02 -0400, Michael S. Tsirkin wrote:
> On Tue, May 12, 2026 at 05:39:48PM +0200, Stefano Garzarella wrote:
> > On Tue, May 12, 2026 at 02:32:14PM +0000, Polina Vishneva wrote:
> > > On Mon, 2026-05-11 at 17:56 +0200, Stefano Garzarella wrote:
> > > > On Mon, May 11, 2026 at 04:56:10PM +0200, Polina Vishneva wrote:
> > > > > From: "Denis V. Lunev" <den@openvz.org>
> > > > >
> > > > > When the host initiates an AF_VSOCK connect() to a guest that has not
> > > > > yet loaded the virtio-vsock transport (i.e. still booting), the caller
> > > > > blocks for VSOCK_DEFAULT_CONNECT_TIMEOUT (2 seconds), because
> > > > > vhost_transport_do_send_pkt() silently exits when
> > > > > vhost_vq_get_backend(vq) returns NULL.
> > > >
> > > > Can SO_VM_SOCKETS_CONNECT_TIMEOUT helps on this?
> > >
> > > It can, but it might be difficult to find a correct timeout.
> > >
> > > And, generally, there's no way to distinguish "the guest hasn't yet initialized
> > > the vq" from "the guest is up and running, but didn't reply to connect() in
> > > time". That's exactly what this patch is attempting to fix.
> >
> > Okay, so please mention this in the commit message, I mean why
> > SO_VM_SOCKETS_CONNECT_TIMEOUT can't really help.
> >
> > >
> > > >
> > > > >
> > > > > If the guest doesn't start listening within this timeout, connect()
> > > > > returns ETIMEDOUT.
> > > > >
> > > > > This delay is usually pointless and it doesn't well align with our
> >
> > I still don't understand why this is pointless. If an application wants to
> > wait while sleeping, it can simply increase the timeout long enough to wait
> > for the VM to start up and use a single `connect()` call, instead of
> > continuing to try and wasting CPU cycles unnecessarily.
> >
> > Hmm, or maybe not, because the driver will definitely be initialized before
> > the application that wants to listen on that port, so it will respond that
> > no one is listening, and the `connect()` call will fail with an `ECONNRESET`
> > error in any case. Right?
> >
> > If it is the case, is the following line in the commit description correct?
> >
> > If the guest doesn't start listening within this timeout, connect()
> > returns ETIMEDOUT.
> >
> > I mean, also if the application starts to listen within the timeout, I think
> > the connect() will fail in any case as I pointed out above (this should be
> > another point in favour of this change)
> >
> >
> > BTW, I think we should explain this more clearly both here and briefly in
> > the code as well.
> >
> > > > > behavior at other initialization stages: for example, if a connection is
> > > > > attempted when the guest driver is already loaded, but when nothing is
> > > > > listening yet, it returns ECONNRESET immediately without any wait.
> > > > >
> > > > > Fix this by checking the RX virtqueue backend in
> > > > > vhost_transport_send_pkt() before queuing. If the backend is NULL,
> > > > > return -ECONNREFUSED immediately.
> > > > >
> > > > > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > > > > Co-developed-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > > > > Signed-off-by: Polina Vishneva <polina.vishneva@virtuozzo.com>
> > > > > ---
> > > > > drivers/vhost/vsock.c | 10 ++++++++++
> > > > > 1 file changed, 10 insertions(+)
> > > > >
> > > > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > > > index 1d8ec6bed53e..a3f218292c3a 100644
> > > > > --- a/drivers/vhost/vsock.c
> > > > > +++ b/drivers/vhost/vsock.c
> > > > > @@ -302,6 +302,16 @@ vhost_transport_send_pkt(struct sk_buff *skb, struct net *net)
> > > > > return -ENODEV;
> > > > > }
> > > > >
> > > > > + /* Fast-fail if the guest hasn't enabled the RX vq yet. Reading
> > > > > + * private_data without vq->mutex is deliberate: even if the backend becomes
> > > > > + * NULL right after that check, do_send_pkt() checks it under the mutex.
> > > > > + */
> > > > > + if (!data_race(READ_ONCE(vsock->vqs[VSOCK_VQ_RX].private_data)))
> > > >
> > > > Why not using vhost_vq_get_backend() ?
> > >
> > > Because it locks the mutex, which is slow and unacceptable in this hot
> > > path.
> >
> > ehm, sorry, which mutex are you talking about?
> >
> > I see just a comment about the mutex to be acquired by the caller, but I
> > don't see any lock there.
> >
> > >
> > > >
> > > > Also is READ_ONCE() okay without WRITE_ONCE() where it is set ?
> > >
> > > It's racy, but as described here in the comment and in the commit message,
> > > any possible race outcome is covered by the subsequent checks.
> >
> > Okay, so what is the point to call READ_ONCE()?
> >
> > >
> > > > > {
> > > > > + rcu_read_unlock();
> > > > > + kfree_skb(skb);
> > > > > + return -ECONNREFUSED;
> > > >
> > > > This is a generic send_pkt, is it okay to return ECONNREFUSED in any
> > > > case?
> > >
> > > EHOSTUNREACH would probably be better.
> > > All the current send_pkt functions only return ENODEV, but it has different
> > > semantics: they mean that the local device isn't yet ready, while there we're
> > > dealing with the opposite end not being ready.
> >
> > In the AF_VSOCK prespective, I see ENODEV like the transport is not ready,
> > so I think it can eventually fit here too, but also EHOSTUNREACH is fine,
> > for sure better than ECONNREFUSED.
> >
> > Thanks,
> > Stefano
>
> I think it's worth trying to do the same thing with e.g. TCP
> and see what error, if any, we get. Match that.
This case is not directly applicable to TCP: in TCP, there's no out-of-band way
to detect the "host up, but not initialized yet and not ready for connections"
state: this could theoretically be ENOPROTOOPT, but no real TCP stack implement
this, because replying with ICMP_PROT_UNREACH requires a TCP stack, which is
exactly the thing that isn't up.
So, in real world, a similar situation with TCP would result in ETIMEDOUT.
>
>
> > >
> > > Best regards, Polina.
> > >
> > > >
> > > > Thanks,
> > > > Stefano
> > > >
> > > > > + }
> > > > > +
> > > > > if (virtio_vsock_skb_reply(skb))
> > > > > atomic_inc(&vsock->queued_replies);
> > > > >
> > > > >
> > > > > base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
> > > > > --
> > > > > 2.53.0
> > > > >
^ permalink raw reply
* Re: [PATCH v2] drm/virtio: move cursor resv lock acquisition to prepare_fb
From: Dmitry Osipenko @ 2026-05-13 9:10 UTC (permalink / raw)
To: Deepanshu Kartikey
Cc: airlied, kraxel, gurchetansingh, olvaffe, maarten.lankhorst,
mripard, tzimmermann, simona, sumit.semwal, christian.koenig,
dri-devel, virtualization, linux-kernel, linux-media,
linaro-mm-sig, syzbot+72bd3dd3a5d5f39a0271, stable
In-Reply-To: <CADhLXY7N0eLpA30eV4Rb=F4vzCf9XYtDjMpxBSJtGeMWNi6Cwg@mail.gmail.com>
On 5/13/26 04:55, Deepanshu Kartikey wrote:
> On Tue, May 12, 2026 at 12:04 PM Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
>>
>> I'm getting lockup with this patch applied and now see that
>> virtio_gpu_resource_flush() also locks BO.
>>
>> Easiest option might be to add uninterruptible variant of
>> virtio_gpu_array_lock_resv(). Could you please try it for v3?
>>
>> --
>> Best regards,
>> Dmitry
>
> Hi Dmitry,
>
> Thanks for testing and catching the lockup. Before I send v3, want
> to confirm the approach:
>
> 1. Revert v2's prepare_fb / cleanup_fb / plane_state changes;
> keep the lock acquisition inside cursor_plane_update like
> the original code.
>
> 2. Add virtio_gpu_array_lock_resv_uninterruptible() in
> virtgpu_gem.c, mirroring the existing helper but using
> dma_resv_lock() instead of dma_resv_lock_interruptible() on
> the nents==1 path. Declare it in virtgpu_drv.h.
>
> 3. In cursor_plane_update, call the new helper and check its
> return. The signal path is closed; -ENOMEM from
> dma_resv_reserve_fences() remains and is handled by freeing
> objs and skipping the cursor update for that frame.
>
> A skipped cursor frame on ENOMEM is the remaining failure mode in
> .atomic_update; this avoids the lockup with virtio_gpu_resource_flush()
> that v2's broader lock scope caused.
>
> Does that match what you had in mind?
Sounds good. The virtio_gpu_resource_flush() also should be updated to
use uninterruptible() variant.
--
Best regards,
Dmitry
^ 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