From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: peterx@redhat.com, "Alex Bennée" <alex.bennee@linaro.org>,
"Thomas Huth" <thuth@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>,
"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
"Juan Quintela" <quintela@redhat.com>,
"Daniel P . Berrangé" <berrange@redhat.com>
Subject: [PATCH] migration: Wait on preempt channel in preempt thread
Date: Tue, 14 Mar 2023 16:54:29 -0400 [thread overview]
Message-ID: <20230314205429.1174295-1-peterx@redhat.com> (raw)
QEMU main thread will wait until dest preempt channel established during
processing the LISTEN command (within the whole postcopy PACKAGED data), by
waiting on the semaphore postcopy_qemufile_dst_done.
That's racy, because it's possible that the dest QEMU main thread hasn't
yet accept()ed the new connection when processing the LISTEN event. The
sem_wait() will yield the main thread without being able to run anything
else including the accept() of the new socket, which can cause deadlock
within the main thread.
To avoid the race, move the "wait channel" from main thread to the preempt
thread right at the start.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 5655aab079 ("migration: Postpone postcopy preempt channel to be after main")
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
PS: This patch is supposed to fix the test breakage reported here:
https://lore.kernel.org/r/ZBBIaX+cZD5Ud2wQ@work-vm
---
migration/postcopy-ram.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index f54f44d899..41c0713650 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1197,11 +1197,6 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
}
if (migrate_postcopy_preempt()) {
- /*
- * The preempt channel is established in asynchronous way. Wait
- * for its completion.
- */
- qemu_sem_wait(&mis->postcopy_qemufile_dst_done);
/*
* This thread needs to be created after the temp pages because
* it'll fetch RAM_CHANNEL_POSTCOPY PostcopyTmpPage immediately.
@@ -1668,6 +1663,12 @@ void *postcopy_preempt_thread(void *opaque)
qemu_sem_post(&mis->thread_sync_sem);
+ /*
+ * The preempt channel is established in asynchronous way. Wait
+ * for its completion.
+ */
+ qemu_sem_wait(&mis->postcopy_qemufile_dst_done);
+
/* Sending RAM_SAVE_FLAG_EOS to terminate this thread */
qemu_mutex_lock(&mis->postcopy_prio_thread_mutex);
while (1) {
--
2.39.1
next reply other threads:[~2023-03-14 20:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-14 20:54 Peter Xu [this message]
2023-03-15 20:34 ` [PATCH] migration: Wait on preempt channel in preempt thread Juan Quintela
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=20230314205429.1174295-1-peterx@redhat.com \
--to=peterx@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=berrange@redhat.com \
--cc=dgilbert@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=thuth@redhat.com \
/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).