From patchwork Fri Apr 5 07:40:07 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 234061 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id D39BB2C00A4 for ; Fri, 5 Apr 2013 18:40:37 +1100 (EST) Received: from localhost ([::1]:55564 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UO1GF-0004Sl-QM for incoming@patchwork.ozlabs.org; Fri, 05 Apr 2013 03:40:35 -0400 Received: from eggs.gnu.org ([208.118.235.92]:49398) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UO1Fy-0004SW-Bu for qemu-devel@nongnu.org; Fri, 05 Apr 2013 03:40:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UO1Fw-00051M-K9 for qemu-devel@nongnu.org; Fri, 05 Apr 2013 03:40:18 -0400 Received: from mail-wg0-f54.google.com ([74.125.82.54]:33124) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UO1Fw-00050n-C9 for qemu-devel@nongnu.org; Fri, 05 Apr 2013 03:40:16 -0400 Received: by mail-wg0-f54.google.com with SMTP id a12so3408128wgh.21 for ; Fri, 05 Apr 2013 00:40:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:sender:from:to:cc:subject:date:message-id:x-mailer; bh=e3cUNTMWnUrGgWL+GHSaqo996iPV0gpgYOiAnKf21m0=; b=i2CGwsMIRd2WR/CHLsPntfknkix7JoKsydgbMUKIxIWYp7IbVvWm+kaILOIcFtNZNq +c+V6AmMYLK62i7lcw6CmuWQypUAUubxS8Xm3k//rZO0w3TGb7lxaaukf448WUrL9S1n nkzecJwKSYEryKZqR46RLxwDVApW448+KgDqDYpLpyRXIU0qTVY2U0pROT8FjBJYN1FE 932hHAKaa78ck29n/vqD4GETc6VXJNO51ihgbBqcP1iEuhGAPUugwQgfXWwBI+VzKJco 22nt/LqYtpWkCuxYYoSHjKGSaeFthVn9wYIHrnF1pRzbQpJXcy7yxViQdiPUueRkA/cy Nhig== X-Received: by 10.194.104.168 with SMTP id gf8mr13812685wjb.58.1365147615148; Fri, 05 Apr 2013 00:40:15 -0700 (PDT) Received: from yakj.lan (93-34-176-20.ip50.fastwebnet.it. [93.34.176.20]) by mx.google.com with ESMTPS id s2sm2292095wib.4.2013.04.05.00.40.12 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 05 Apr 2013 00:40:14 -0700 (PDT) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Fri, 5 Apr 2013 09:40:07 +0200 Message-Id: <1365147607-18298-1-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.8.2 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 74.125.82.54 Cc: amit.shah@redhat.com, aliguori@us.ibm.com, peter.crosthwaite@xilinx.com, gson@gson.org Subject: [Qemu-devel] [PATCH] qemu-char: eliminate busy waiting on can_read returning zero X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The character backend refactoring introduced an undesirable busy wait. The busy wait happens if can_read returns zero and there is data available on the character device's file descriptor. Then, the I/O watch will fire continuously and, with TCG, the CPU thread will never run. 1) Char backend asks front end if it can write 2) Front end says no 3) poll() finds the char backend's descriptor is available 4) Goto (1) What we really want is this (note that step 3 avoids the busy wait): 1) Char backend asks front end if it can write 2) Front end says no 3) poll() goes on without char backend's descriptor 4) Goto (1) until qemu_chr_accept_input() called 5) Char backend asks front end if it can write 6) Front end says yes 7) poll() finds the char backend's descriptor is available 8) Backend handler called After this patch, the IOWatchPoll source and the watch source are separated. The IOWatchPoll is simply a hook that runs during the prepare phase on each main loop iteration. The hook adds/removes the actual source depending on the return value from can_read. A simple reproducer is qemu-system-i386 -serial mon:stdio ... followed by banging on the terminal as much as you can. :) Without this patch, emulation will hang. Signed-off-by: Paolo Bonzini Reviewed-by: Anthony Liguori --- This supersedes Peter's patch. qemu-char.c | 64 +++++++++++++++++++++------------------------------------------- 1 file changed, 21 insertions(+), 43 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index e5eb8dd..d4239b5 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -594,65 +594,52 @@ int recv_all(int fd, void *_buf, int len1, bool single_read) typedef struct IOWatchPoll { + GSource parent; + GSource *src; - int max_size; + bool active; IOCanReadHandler *fd_can_read; void *opaque; - - QTAILQ_ENTRY(IOWatchPoll) node; } IOWatchPoll; -static QTAILQ_HEAD(, IOWatchPoll) io_watch_poll_list = - QTAILQ_HEAD_INITIALIZER(io_watch_poll_list); - static IOWatchPoll *io_watch_poll_from_source(GSource *source) { - IOWatchPoll *i; - - QTAILQ_FOREACH(i, &io_watch_poll_list, node) { - if (i->src == source) { - return i; - } - } - - return NULL; + return container_of(source, IOWatchPoll, parent); } static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_) { IOWatchPoll *iwp = io_watch_poll_from_source(source); - - iwp->max_size = iwp->fd_can_read(iwp->opaque); - if (iwp->max_size == 0) { + bool active = iwp->fd_can_read(iwp->opaque) > 0; + if (iwp->active == active) { return FALSE; } - return g_io_watch_funcs.prepare(source, timeout_); + iwp->active = active; + if (active) { + g_source_attach(iwp->src, NULL); + } else { + g_source_remove(g_source_get_id(iwp->src)); + } + return FALSE; } static gboolean io_watch_poll_check(GSource *source) { - IOWatchPoll *iwp = io_watch_poll_from_source(source); - - if (iwp->max_size == 0) { - return FALSE; - } - - return g_io_watch_funcs.check(source); + return FALSE; } static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback, gpointer user_data) { - return g_io_watch_funcs.dispatch(source, callback, user_data); + abort(); } static void io_watch_poll_finalize(GSource *source) { IOWatchPoll *iwp = io_watch_poll_from_source(source); - QTAILQ_REMOVE(&io_watch_poll_list, iwp, node); - g_io_watch_funcs.finalize(source); + g_source_unref(iwp->src); } static GSourceFuncs io_watch_poll_funcs = { @@ -669,24 +657,15 @@ static guint io_add_watch_poll(GIOChannel *channel, gpointer user_data) { IOWatchPoll *iwp; - GSource *src; - guint tag; - - src = g_io_create_watch(channel, G_IO_IN | G_IO_ERR | G_IO_HUP); - g_source_set_funcs(src, &io_watch_poll_funcs); - g_source_set_callback(src, (GSourceFunc)fd_read, user_data, NULL); - tag = g_source_attach(src, NULL); - g_source_unref(src); - iwp = g_malloc0(sizeof(*iwp)); - iwp->src = src; - iwp->max_size = 0; + iwp = (IOWatchPoll *) g_source_new(&io_watch_poll_funcs, sizeof(IOWatchPoll)); + iwp->active = FALSE; iwp->fd_can_read = fd_can_read; iwp->opaque = user_data; + iwp->src = g_io_create_watch(channel, G_IO_IN | G_IO_ERR | G_IO_HUP); + g_source_set_callback(iwp->src, (GSourceFunc)fd_read, user_data, NULL); - QTAILQ_INSERT_HEAD(&io_watch_poll_list, iwp, node); - - return tag; + return g_source_attach(&iwp->parent, NULL); } #ifndef _WIN32