Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Oct 2023 18:32:32 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 7de582874eb9 - main - bhyve: Remove init_snapshot() and initialize static vars
Message-ID:  <202310171832.39HIWWiG095927@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=7de582874eb9d08f3f87d11ed9e2b9ce8306db79

commit 7de582874eb9d08f3f87d11ed9e2b9ce8306db79
Author:     Vitaliy Gusev <gusev.vitaliy@gmail.com>
AuthorDate: 2023-10-17 14:16:08 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-10-17 18:26:51 +0000

    bhyve: Remove init_snapshot() and initialize static vars
    
    vCPU threads are starting before init_snapshot() is called. That can lead
    to corruption of vcpu_lock userspace mutex (snapshot.c) and then VM hangs
    in acquiring that mutex.
    
    init_snapshot() initializes only static variables (mutex, cv) and that
    code can be optimized and removed.
    
    Fixes:          9a9a248964696 ("bhyve: init checkput before caph_enter")
    Reviewed by:    markj
    MFC after:      1 week
    Sponsored by:   vStack
---
 usr.sbin/bhyve/bhyverun.c |  3 ---
 usr.sbin/bhyve/snapshot.c | 21 +++------------------
 usr.sbin/bhyve/snapshot.h |  1 -
 3 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/usr.sbin/bhyve/bhyverun.c b/usr.sbin/bhyve/bhyverun.c
index 0d7f58509244..8147dcd3872b 100644
--- a/usr.sbin/bhyve/bhyverun.c
+++ b/usr.sbin/bhyve/bhyverun.c
@@ -1021,9 +1021,6 @@ main(int argc, char *argv[])
 	setproctitle("%s", vmname);
 
 #ifdef BHYVE_SNAPSHOT
-	/* initialize mutex/cond variables */
-	init_snapshot();
-
 	/*
 	 * checkpointing thread for communication with bhyvectl
 	 */
diff --git a/usr.sbin/bhyve/snapshot.c b/usr.sbin/bhyve/snapshot.c
index 5f643c9ceb50..edce55c03eae 100644
--- a/usr.sbin/bhyve/snapshot.c
+++ b/usr.sbin/bhyve/snapshot.c
@@ -137,8 +137,9 @@ static const struct vm_snapshot_kern_info snapshot_kern_structs[] = {
 };
 
 static cpuset_t vcpus_active, vcpus_suspended;
-static pthread_mutex_t vcpu_lock;
-static pthread_cond_t vcpus_idle, vcpus_can_run;
+static pthread_mutex_t vcpu_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t vcpus_idle = PTHREAD_COND_INITIALIZER;
+static pthread_cond_t vcpus_can_run = PTHREAD_COND_INITIALIZER;
 static bool checkpoint_active;
 
 /*
@@ -1395,22 +1396,6 @@ vm_do_checkpoint(struct vmctx *ctx, const nvlist_t *nvl)
 }
 IPC_COMMAND(ipc_cmd_set, checkpoint, vm_do_checkpoint);
 
-void
-init_snapshot(void)
-{
-	int err;
-
-	err = pthread_mutex_init(&vcpu_lock, NULL);
-	if (err != 0)
-		errc(1, err, "checkpoint mutex init");
-	err = pthread_cond_init(&vcpus_idle, NULL);
-	if (err != 0)
-		errc(1, err, "checkpoint cv init (vcpus_idle)");
-	err = pthread_cond_init(&vcpus_can_run, NULL);
-	if (err != 0)
-		errc(1, err, "checkpoint cv init (vcpus_can_run)");
-}
-
 /*
  * Create the listening socket for IPC with bhyvectl
  */
diff --git a/usr.sbin/bhyve/snapshot.h b/usr.sbin/bhyve/snapshot.h
index 179aafb6471d..8bebdafd6117 100644
--- a/usr.sbin/bhyve/snapshot.h
+++ b/usr.sbin/bhyve/snapshot.h
@@ -100,7 +100,6 @@ int vm_resume_devices(void);
 int get_checkpoint_msg(int conn_fd, struct vmctx *ctx);
 void *checkpoint_thread(void *param);
 int init_checkpoint_thread(struct vmctx *ctx);
-void init_snapshot(void);
 
 int load_restore_file(const char *filename, struct restore_state *rstate);
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202310171832.39HIWWiG095927>