From: Iris Johnson <iris@modwiz.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"open list:All patches CC here" <qemu-devel@nongnu.org>,
"Iris Johnson" <iris@modwiz.com>
Subject: [PATCH] chardev/char-io: Fix polling by not removing polls when buffers are full
Date: Fri, 29 Jan 2021 19:56:31 +0000 [thread overview]
Message-ID: <20210129195631.1577922-1-iris@modwiz.com> (raw)
Currently, the chardev backend code will prepare for IO polling to occur
by potentially adding or removing a watch of the backing channel for the
chardev. The chardev poll is added if the fd_can_read() function reports
more than 0 byte of buffer space, if a poll handler is already setup and
the bufer is now empty, the poll handler is removed.
This causes a bug where the device buffer becomes ready, but the poll is
blocking on a sleep (potentially forever), because the buffer is small
and fills up immediately, while the backend channel has more data. This
leads to a stall condition or potentially a deadlock in the guest.
The guest is looping, waiting for data to be reported as ready to read,
the host sees that the buffer is ready for reading and adds the poll,
the poll returns since data is available and data is made available to
the guest. Before the guest code is able to retrieve the data and clear
the full buffer, the poll code runs again, sees that the buffer is now
full, and removes the poll. At this point only a timeout from another
polled source, or another source having it's poll complete will result
in the loop running again to see that the buffer is now ready and to
add the poll again.
We solve this issue by removing the logic that removes the poll, keeping
the existing logic to only create the poll once there's space for the
first read.
Buglink: https://bugs.launchpad.net/qemu/+bug/1913341
Signed-off-by: Iris Johnson <iris@modwiz.com>
---
chardev/char-io.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index 8ced184160..fa9e222f78 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -50,16 +50,14 @@ static gboolean io_watch_poll_prepare(GSource *source,
return FALSE;
}
- if (now_active) {
+ if (now_active && !was_active) {
iwp->src = qio_channel_create_watch(
iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
g_source_add_child_source(source, iwp->src);
g_source_unref(iwp->src);
- } else {
- g_source_remove_child_source(source, iwp->src);
- iwp->src = NULL;
}
+
return FALSE;
}
--
2.25.1
next reply other threads:[~2021-01-29 19:58 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-29 19:56 Iris Johnson [this message]
2021-01-30 10:06 ` [PATCH] chardev/char-io: Fix polling by not removing polls when buffers are full Marc-André Lureau
2021-01-30 15:54 ` Iris Johnson
2021-01-30 17:55 ` Iris Johnson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210129195631.1577922-1-iris@modwiz.com \
--to=iris@modwiz.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).