vmd has a sneaky little deadlock hidden in the pause logic related to
the use of mutexes and condition variables.
When pausing, the vcpu is holding the "run" mutex. It then sleeps
waiting for the unpause condition. If the event thread is trying to
assert an irq, it will try to lock that "run" mutex in an attempt to
signal a halted vcpu that it should start running. This deadlocks the
thread.
Diff below releases the run mutex (by the vcpu thread) before sleeping
on the "pause" condition and reacquires it afterwards. This lets the
event thread advance as needed.
A simple reproducer is run `iperf3 -s` inside a guest, `iperf -c <ip> -t
60` from outside the guest, and them `vmctl pause <vm>`. vmctl will hang
as the vm can't respond to the pause request.
I'm also simplifying the offending vcpu_assert_pic_irq (called by the
event thread), so the diff should be pretty self contained for review.
ok?
diffstat /usr/src
M usr.sbin/vmd/vm.c | 6+ 8-
1 file changed, 6 insertions(+), 8 deletions(-)
diff /usr/src
commit - 89fcd96b33617c4eddd2306889179a96a934ebe8
path + /usr/src
blob - fe804b4e3b9e8b59b883e58f02e15a178c035742
file + usr.sbin/vmd/vm.c
--- usr.sbin/vmd/vm.c
+++ usr.sbin/vmd/vm.c
@@ -1564,16 +1564,20 @@ vcpu_run_loop(void *arg)
__func__, (int)ret);
return ((void *)ret);
}
+ /* i8259 may be firing as we pause, release run mtx. */
+ mutex_unlock(&vcpu_run_mtx[n]);
ret = pthread_cond_wait(&vcpu_unpause_cond[n],
&vcpu_unpause_mtx[n]);
if (ret) {
log_warnx(
"%s: can't wait on unpause cond (%d)",
__func__, (int)ret);
break;
}
+ mutex_lock(&vcpu_run_mtx[n]);
+
ret = pthread_mutex_unlock(&vcpu_unpause_mtx[n]);
if (ret) {
log_warnx("%s: can't unlock unpause mtx (%d)",
__func__, (int)ret);
@@ -2135,20 +2139,14 @@ vcpu_assert_pic_irq(uint32_t vm_id, uint32_t vcpu_id,
if (i8259_is_pending()) {
if (vcpu_pic_intr(vm_id, vcpu_id, 1))
fatalx("%s: can't assert INTR", __func__);
-
- ret = pthread_mutex_lock(&vcpu_run_mtx[vcpu_id]);
- if (ret)
- fatalx("%s: can't lock vcpu mtx (%d)", __func__, ret);
-
+ mutex_lock(&vcpu_run_mtx[vcpu_id]);
vcpu_hlt[vcpu_id] = 0;
ret = pthread_cond_signal(&vcpu_run_cond[vcpu_id]);
if (ret)
fatalx("%s: can't signal (%d)", __func__, ret);
- ret = pthread_mutex_unlock(&vcpu_run_mtx[vcpu_id]);
- if (ret)
- fatalx("%s: can't unlock vcpu mtx (%d)", __func__, ret);
+ mutex_unlock(&vcpu_run_mtx[vcpu_id]);
}
}
/*
