mirror of
https://git.proxmox.com/git/pve-qemu
synced 2025-08-15 22:25:11 +00:00
441072fc57
Commit f06b222 ("fixes for QEMU 9.0") included a revert for the QEMU
commit 2ce6cff94d ("virtio-pci: fix use of a released vector"). That
commit caused some regressions which sounded just as bad as the fix.
Those regressions have now been addressed upstream, so pick up the fix
and drop the revert. Dropping the revert fixes the original issue that
commit 2ce6cff94d ("virtio-pci: fix use of a released vector")
addressed.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
94 lines
4.6 KiB
Diff
94 lines
4.6 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Sergey Dyasli <sergey.dyasli@nutanix.com>
|
|
Date: Fri, 12 Jul 2024 09:26:59 +0000
|
|
Subject: [PATCH] Revert "qemu-char: do not operate on sources from finalize
|
|
callbacks"
|
|
|
|
This reverts commit 2b316774f60291f57ca9ecb6a9f0712c532cae34.
|
|
|
|
After 038b4217884c ("Revert "chardev: use a child source for qio input
|
|
source"") we've been observing the "iwp->src == NULL" assertion
|
|
triggering periodically during the initial capabilities querying by
|
|
libvirtd. One of possible backtraces:
|
|
|
|
Thread 1 (Thread 0x7f16cd4f0700 (LWP 43858)):
|
|
0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
|
|
1 0x00007f16c6c21e65 in __GI_abort () at abort.c:79
|
|
2 0x00007f16c6c21d39 in __assert_fail_base at assert.c:92
|
|
3 0x00007f16c6c46e86 in __GI___assert_fail (assertion=assertion@entry=0x562e9bcdaadd "iwp->src == NULL", file=file@entry=0x562e9bcdaac8 "../chardev/char-io.c", line=line@entry=99, function=function@entry=0x562e9bcdab10 <__PRETTY_FUNCTION__.20549> "io_watch_poll_finalize") at assert.c:101
|
|
4 0x0000562e9ba20c2c in io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:99
|
|
5 io_watch_poll_finalize (source=<optimized out>) at ../chardev/char-io.c:88
|
|
6 0x00007f16c904aae0 in g_source_unref_internal () from /lib64/libglib-2.0.so.0
|
|
7 0x00007f16c904baf9 in g_source_destroy_internal () from /lib64/libglib-2.0.so.0
|
|
8 0x0000562e9ba20db0 in io_remove_watch_poll (source=0x562e9d6720b0) at ../chardev/char-io.c:147
|
|
9 remove_fd_in_watch (chr=chr@entry=0x562e9d5f3800) at ../chardev/char-io.c:153
|
|
10 0x0000562e9ba23ffb in update_ioc_handlers (s=0x562e9d5f3800) at ../chardev/char-socket.c:592
|
|
11 0x0000562e9ba2072f in qemu_chr_fe_set_handlers_full at ../chardev/char-fe.c:279
|
|
12 0x0000562e9ba207a9 in qemu_chr_fe_set_handlers at ../chardev/char-fe.c:304
|
|
13 0x0000562e9ba2ca75 in monitor_qmp_setup_handlers_bh (opaque=0x562e9d4c2c60) at ../monitor/qmp.c:509
|
|
14 0x0000562e9bb6222e in aio_bh_poll (ctx=ctx@entry=0x562e9d4c2f20) at ../util/async.c:216
|
|
15 0x0000562e9bb4de0a in aio_poll (ctx=0x562e9d4c2f20, blocking=blocking@entry=true) at ../util/aio-posix.c:722
|
|
16 0x0000562e9b99dfaa in iothread_run (opaque=0x562e9d4c26f0) at ../iothread.c:63
|
|
17 0x0000562e9bb505a4 in qemu_thread_start (args=0x562e9d4c7ea0) at ../util/qemu-thread-posix.c:543
|
|
18 0x00007f16c70081ca in start_thread (arg=<optimized out>) at pthread_create.c:479
|
|
19 0x00007f16c6c398d3 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
|
|
|
|
io_remove_watch_poll(), which makes sure that iwp->src is NULL, calls
|
|
g_source_destroy() which finds that iwp->src is not NULL in the finalize
|
|
callback. This can only happen if another thread has managed to trigger
|
|
io_watch_poll_prepare() callback in the meantime.
|
|
|
|
Move iwp->src destruction back to the finalize callback to prevent the
|
|
described race, and also remove the stale comment. The deadlock glib bug
|
|
was fixed back in 2010 by b35820285668 ("gmain: move finalization of
|
|
GSource outside of context lock").
|
|
|
|
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
Signed-off-by: Sergey Dyasli <sergey.dyasli@nutanix.com>
|
|
Link: https://lore.kernel.org/r/20240712092659.216206-1-sergey.dyasli@nutanix.com
|
|
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
(cherry picked from commit e0bf95443ee9326d44031373420cf9f3513ee255)
|
|
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
|
|
---
|
|
chardev/char-io.c | 19 +++++--------------
|
|
1 file changed, 5 insertions(+), 14 deletions(-)
|
|
|
|
diff --git a/chardev/char-io.c b/chardev/char-io.c
|
|
index dab77b112e..3be17b51ca 100644
|
|
--- a/chardev/char-io.c
|
|
+++ b/chardev/char-io.c
|
|
@@ -87,16 +87,12 @@ static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
|
|
|
|
static void io_watch_poll_finalize(GSource *source)
|
|
{
|
|
- /*
|
|
- * Due to a glib bug, removing the last reference to a source
|
|
- * inside a finalize callback causes recursive locking (and a
|
|
- * deadlock). This is not a problem inside other callbacks,
|
|
- * including dispatch callbacks, so we call io_remove_watch_poll
|
|
- * to remove this source. At this point, iwp->src must
|
|
- * be NULL, or we would leak it.
|
|
- */
|
|
IOWatchPoll *iwp = io_watch_poll_from_source(source);
|
|
- assert(iwp->src == NULL);
|
|
+ if (iwp->src) {
|
|
+ g_source_destroy(iwp->src);
|
|
+ g_source_unref(iwp->src);
|
|
+ iwp->src = NULL;
|
|
+ }
|
|
}
|
|
|
|
static GSourceFuncs io_watch_poll_funcs = {
|
|
@@ -139,11 +135,6 @@ static void io_remove_watch_poll(GSource *source)
|
|
IOWatchPoll *iwp;
|
|
|
|
iwp = io_watch_poll_from_source(source);
|
|
- if (iwp->src) {
|
|
- g_source_destroy(iwp->src);
|
|
- g_source_unref(iwp->src);
|
|
- iwp->src = NULL;
|
|
- }
|
|
g_source_destroy(&iwp->parent);
|
|
}
|
|
|