From a1598eaa1f229f4d81bafa4e320453991d03a029 Mon Sep 17 00:00:00 2001 Message-Id: In-Reply-To: <7d8ebc793c9bc4b5058ec1189139e7912e209e19.1375955382.git.minovotn@redhat.com> References: <7d8ebc793c9bc4b5058ec1189139e7912e209e19.1375955382.git.minovotn@redhat.com> From: Laszlo Ersek Date: Tue, 6 Aug 2013 16:31:41 +0200 Subject: [PATCH 33/35] monitor: maintain at most one G_IO_OUT watch RH-Author: Laszlo Ersek Message-id: <1375806701-27520-3-git-send-email-lersek@redhat.com> Patchwork-id: 53003 O-Subject: [RHEL-6.5 qemu-kvm PATCH 2/2] monitor: maintain at most one G_IO_OUT watch Bugzilla: 985334 RH-Acked-by: Amit Shah RH-Acked-by: Markus Armbruster RH-Acked-by: Luiz Capitulino When monitor_flush() is invoked repeatedly outside the monitor_unblocked() callback, for example from tlb_info() -> ... -> print_pte(), several watches may be added for the same event. This is no problem per se because the extra monitor_unblocked() callbacks are harmless if mon->outbuf is empty, the watches will be removed gradually. However a big number of watches can grow "gpollfds" without limit in glib_pollfds_fill(), triggering a -1/EINVAL condition in g_poll(). Keep at most one such watch, by following the pattern observable in eg. commits c874ea97 and c3d6b96e. The change has no effect when monitor_unblocked() calls monitor_flush() (when the watch can either be removed or renewed 1-for-1), but non-callback contexts won't create an additional watch when the monitor already has one. Related RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=970047 Signed-off-by: Laszlo Ersek Reviewed-by: Amit Shah Reviewed-by: Anthony Liguori Message-id: 1373998781-29561-3-git-send-email-lersek@redhat.com Signed-off-by: Anthony Liguori (cherry picked from commit 293d2a0014a0e849477413f55aaa05f2743b2e04) RHEL-6 note: the problem affects RHEL-6 qemu-kvm a bit differently (the main loop is select()-based rather than g_poll()-based), but the unlimited growth of monitor watches is still there -- after a while the watches would overflow a fixed-size array. Thankfully, they trigger an assert() in glib_select_fill() first; see bug 985334 comments 4-5. Hence the bug is there in RHEL-6, reproducibly; let's fix it. --- monitor.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-) Signed-off-by: Michal Novotny --- monitor.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index dfc2f00..b096c7f 100644 --- a/monitor.c +++ b/monitor.c @@ -166,6 +166,7 @@ struct Monitor { int flags; int suspend_cnt; QString *outbuf; + guint watch; ReadLineState *rs; MonitorControl *mc; CPUState *mon_cpu; @@ -266,7 +267,10 @@ static int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func, static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond, void *opaque) { - monitor_flush(opaque); + Monitor *mon = opaque; + + mon->watch = 0; + monitor_flush(mon); return FALSE; } @@ -293,7 +297,10 @@ void monitor_flush(Monitor *mon) QDECREF(mon->outbuf); mon->outbuf = tmp; } - qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon); + if (mon->watch == 0) { + mon->watch = qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, + monitor_unblocked, mon); + } } } -- 1.7.11.7