From: Fabiano Rosas <farosas@suse.de>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Juan Quintela <quintela@redhat.com>,
Jiang Jiacheng <jiangjiacheng@huawei.com>,
Peter Xu <peterx@redhat.com>, Leonardo Bras <leobras@redhat.com>
Subject: [PATCH 2/3] migration/multifd: Protect accesses to migration_threads
Date: Tue, 6 Jun 2023 11:45:50 -0300 [thread overview]
Message-ID: <20230606144551.24367-3-farosas@suse.de> (raw)
In-Reply-To: <20230606144551.24367-1-farosas@suse.de>
This doubly linked list is common for all the multifd and migration
threads so we need to avoid concurrent access.
Add a mutex to protect the data from concurrent access. This fixes a
crash when removing two MigrationThread objects from the list at the
same time during cleanup of multifd threads.
To avoid destroying the mutex before the last element has been
removed, move calls to qmp_migration_thread_remove so they run before
multifd_save_cleanup joins the threads.
Fixes: 671326201d ("migration: Introduce interface query-migrationthreads")
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
migration/migration.c | 5 ++++-
migration/multifd.c | 3 ++-
migration/threadinfo.c | 19 ++++++++++++++++++-
migration/threadinfo.h | 5 +++--
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index e731fc98a1..b3b8345eb2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1146,6 +1146,7 @@ static void migrate_fd_cleanup(MigrationState *s)
qemu_mutex_lock_iothread();
multifd_save_cleanup();
+ qmp_migration_threads_cleanup();
qemu_mutex_lock(&s->qemu_file_lock);
tmp = s->to_dst_file;
s->to_dst_file = NULL;
@@ -1405,6 +1406,8 @@ void migrate_init(MigrationState *s)
s->vm_old_state = -1;
s->iteration_initial_bytes = 0;
s->threshold_size = 0;
+
+ qmp_migration_threads_init();
}
int migrate_add_blocker_internal(Error *reason, Error **errp)
@@ -2997,10 +3000,10 @@ static void *migration_thread(void *opaque)
}
trace_migration_thread_after_loop();
+ qmp_migration_threads_remove(thread);
migration_iteration_finish(s);
object_unref(OBJECT(s));
rcu_unregister_thread();
- qmp_migration_threads_remove(thread);
return NULL;
}
diff --git a/migration/multifd.c b/migration/multifd.c
index 5ec1ac5c64..ee7944560a 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -762,12 +762,13 @@ out:
qemu_sem_post(&multifd_send_state->channels_ready);
}
+ qmp_migration_threads_remove(thread);
+
qemu_mutex_lock(&p->mutex);
p->running = false;
qemu_mutex_unlock(&p->mutex);
rcu_unregister_thread();
- qmp_migration_threads_remove(thread);
trace_multifd_send_thread_end(p->id, p->num_packets, p->total_normal_pages);
return NULL;
diff --git a/migration/threadinfo.c b/migration/threadinfo.c
index c3e85c33e8..1fe64a02dd 100644
--- a/migration/threadinfo.c
+++ b/migration/threadinfo.c
@@ -10,23 +10,40 @@
* See the COPYING file in the top-level directory.
*/
+#include "qemu/osdep.h"
+#include "qemu/queue.h"
+#include "qemu/lockable.h"
#include "threadinfo.h"
+QemuMutex migration_threads_lock;
static QLIST_HEAD(, MigrationThread) migration_threads;
+void qmp_migration_threads_init(void)
+{
+ qemu_mutex_init(&migration_threads_lock);
+}
+
+void qmp_migration_threads_cleanup(void)
+{
+ qemu_mutex_destroy(&migration_threads_lock);
+}
+
MigrationThread *qmp_migration_threads_add(const char *name, int thread_id)
{
MigrationThread *thread = g_new0(MigrationThread, 1);
thread->name = name;
thread->thread_id = thread_id;
- QLIST_INSERT_HEAD(&migration_threads, thread, node);
+ WITH_QEMU_LOCK_GUARD(&migration_threads_lock) {
+ QLIST_INSERT_HEAD(&migration_threads, thread, node);
+ }
return thread;
}
void qmp_migration_threads_remove(MigrationThread *thread)
{
+ QEMU_LOCK_GUARD(&migration_threads_lock);
if (thread) {
QLIST_REMOVE(thread, node);
g_free(thread);
diff --git a/migration/threadinfo.h b/migration/threadinfo.h
index 61b990f5e3..eb7f8e5bb2 100644
--- a/migration/threadinfo.h
+++ b/migration/threadinfo.h
@@ -10,8 +10,6 @@
* See the COPYING file in the top-level directory.
*/
-#include "qemu/queue.h"
-#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
@@ -23,5 +21,8 @@ struct MigrationThread {
QLIST_ENTRY(MigrationThread) node;
};
+void qmp_migration_threads_init(void);
+void qmp_migration_threads_cleanup(void);
+
MigrationThread *qmp_migration_threads_add(const char *name, int thread_id);
void qmp_migration_threads_remove(MigrationThread *info);
--
2.35.3
next prev parent reply other threads:[~2023-06-06 14:46 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-06 14:45 [PATCH 0/3] migration: Fix multifd cancel test Fabiano Rosas
2023-06-06 14:45 ` [PATCH 1/3] migration/multifd: Rename threadinfo.c functions Fabiano Rosas
2023-06-06 18:38 ` Peter Xu
2023-06-06 19:34 ` Fabiano Rosas
2023-06-06 20:03 ` Peter Xu
2023-06-07 6:30 ` Juan Quintela
2023-06-07 7:56 ` Philippe Mathieu-Daudé
2023-06-06 14:45 ` Fabiano Rosas [this message]
2023-06-06 18:43 ` [PATCH 2/3] migration/multifd: Protect accesses to migration_threads Peter Xu
2023-06-07 8:26 ` Juan Quintela
2023-06-07 12:00 ` Fabiano Rosas
2023-06-07 13:25 ` Peter Xu
2023-06-07 16:58 ` Juan Quintela
2023-06-06 14:45 ` [PATCH 3/3] tests/qtest: Re-enable multifd cancel test Fabiano Rosas
2023-06-07 8:27 ` Juan Quintela
2024-01-08 6:42 ` Peter Xu
2024-01-08 14:26 ` Fabiano Rosas
2024-01-09 2:12 ` Peter Xu
2024-01-09 7:21 ` Thomas Huth
2024-01-09 7:48 ` Peter Xu
2024-01-09 8:44 ` Thomas Huth
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=20230606144551.24367-3-farosas@suse.de \
--to=farosas@suse.de \
--cc=jiangjiacheng@huawei.com \
--cc=leobras@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=peterx@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@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).