[PATCH] qga: don't busy-loop writing to virtio-serial when host disconnected

Damien Ferrere posted 1 patch 1 week, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260714041405.75395-1-damien@varyab.net
Maintainers: Michael Roth <michael.roth@amd.com>, Kostiantyn Kostiuk <kkostiuk@redhat.com>
qga/channel-posix.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
[PATCH] qga: don't busy-loop writing to virtio-serial when host disconnected
Posted by Damien Ferrere 1 week, 4 days ago
ga_channel_write_all() retries g_io_channel_write_chars() (and later
g_io_channel_flush()) in a tight loop while the status is
G_IO_STATUS_AGAIN, without ever waiting for the fd to become writable
again.  For a virtio-serial channel this condition is reached whenever
nothing is draining the host-side chardev: the guest port buffer fills
up, every write() returns EAGAIN, and the loop spins, pinning a CPU at
100% for as long as the host stays disconnected.

This is easy to hit in practice: as soon as the host stops reading the
channel and the agent has a response to send (e.g. the reply to a
guest-ping), qemu-ga jumps to 100% CPU.  It shows up as a thread stuck
in state R with /proc/<pid>/io reporting billions of write syscalls for
only a few MB of wchar, and fd pointing at /dev/vportNpN.

Before commit f74df9bfce6d1 the loop simply broke out of the write on a
non-normal status, so G_IO_STATUS_AGAIN could not spin.  That commit
started retrying on AGAIN to handle short writes, but retries with no
backoff or wait, turning a transient AGAIN into an unbounded busy loop.

The read path (channel_event_cb) already mitigates the same virtio spin
with a sleep on G_IO_STATUS_AGAIN.  Do the analogous thing on the write
path, but instead of sleeping a fixed amount, block on POLLOUT so no CPU
is consumed while the channel is unwritable, and return on POLLHUP/
POLLERR so a genuine disconnect surfaces as a write error (which resets
the connection) rather than an infinite wait.

Behaviour is otherwise unchanged: a response that cannot yet be written
is still delivered once the host resumes draining the channel; it just
no longer burns a CPU in the meantime.

Fixes: f74df9bfce6d1 ("qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()")
Signed-off-by: Damien Ferrere <damien@varyab.net>
---
 qga/channel-posix.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 9ccc8b7bd1..fa16c252f7 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -253,6 +253,25 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
     return true;
 }
 
+/*
+ * Wait for the channel fd to become writable again after a short write
+ * returned G_IO_STATUS_AGAIN. For a virtio-serial channel this happens when
+ * nothing drains the host-side chardev: the port buffer fills up and every
+ * write() returns EAGAIN. Without this, the caller busy-loops on write() and
+ * pins a CPU at 100% for as long as the host stays disconnected. The read
+ * path (channel_event_cb) already mitigates the same virtio spin with a
+ * sleep; this blocks on POLLOUT so we consume no CPU while waiting, and
+ * returns on HUP/ERR so a real disconnect surfaces as an error instead.
+ */
+static void ga_channel_wait_writable(GAChannel *c)
+{
+    GPollFD pfd = {
+        .fd = g_io_channel_unix_get_fd(c->client_channel),
+        .events = G_IO_OUT | G_IO_ERR | G_IO_HUP,
+    };
+    g_poll(&pfd, 1, -1);
+}
+
 GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
 {
     GError *err = NULL;
@@ -266,7 +285,9 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
         if (status == G_IO_STATUS_NORMAL) {
             size -= written;
             buf += written;
-        } else if (status != G_IO_STATUS_AGAIN) {
+        } else if (status == G_IO_STATUS_AGAIN) {
+            ga_channel_wait_writable(c);
+        } else {
             g_warning("error writing to channel: %s", err->message);
             return status;
         }
@@ -274,6 +295,9 @@ GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size)
 
     do {
         status = g_io_channel_flush(c->client_channel, &err);
+        if (status == G_IO_STATUS_AGAIN) {
+            ga_channel_wait_writable(c);
+        }
     } while (status == G_IO_STATUS_AGAIN);
 
     if (status != G_IO_STATUS_NORMAL) {
-- 
2.47.3