| Message ID | 20210417143602.215059-3-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: fix races in dirty log test | expand |
On 17/04/21 16:36, Peter Xu wrote: > The main thread could start to send SIG_IPI at any time, even before signal > blocked on vcpu thread. Reuse the sem_vcpu_stop to sync on that, so when > SIG_IPI is sent the signal will always land correctly as an -EINTR. > > Without this patch, on very busy cores the dirty_log_test could fail directly > on receiving a SIG_USR1 without a handler (when vcpu runs far slower than main). > > Signed-off-by: Peter Xu <peterx@redhat.com> This should be a better way to do the same: ----------- 8< ------------ From: Paolo Bonzini <pbonzini@redhat.com> Subject: [PATCH] KVM: selftests: Wait for vcpu thread before signal setup The main thread could start to send SIG_IPI at any time, even before signal blocked on vcpu thread. Therefore, start the vcpu thread with the signal blocked. Without this patch, on very busy cores the dirty_log_test could fail directly on receiving a SIGUSR1 without a handler (when vcpu runs far slower than main). Reported-by: Peter Xu <peterx@redhat.com> Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c index ffa4e2791926..048973d50a45 100644 --- a/tools/testing/selftests/kvm/dirty_log_test.c +++ b/tools/testing/selftests/kvm/dirty_log_test.c @@ -527,9 +527,8 @@ static void *vcpu_worker(void *data) */ sigmask->len = 8; pthread_sigmask(0, NULL, sigset); + sigdelset(sigset, SIG_IPI); vcpu_ioctl(vm, VCPU_ID, KVM_SET_SIGNAL_MASK, sigmask); - sigaddset(sigset, SIG_IPI); - pthread_sigmask(SIG_BLOCK, sigset, NULL); sigemptyset(sigset); sigaddset(sigset, SIG_IPI); @@ -858,6 +857,7 @@ int main(int argc, char *argv[]) .interval = TEST_HOST_LOOP_INTERVAL, }; int opt, i; + sigset_t sigset; sem_init(&sem_vcpu_stop, 0, 0); sem_init(&sem_vcpu_cont, 0, 0); @@ -916,6 +916,11 @@ int main(int argc, char *argv[]) srandom(time(0)); + /* Ensure that vCPU threads start with SIG_IPI blocked. */ + sigemptyset(&sigset); + sigaddset(&sigset, SIG_IPI); + pthread_sigmask(SIG_BLOCK, sigset, NULL); + if (host_log_mode_option == LOG_MODE_ALL) { /* Run each log mode */ for (i = 0; i < LOG_MODE_NUM; i++) { > --- > tools/testing/selftests/kvm/dirty_log_test.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c > index 510884f0eab8..25230e799bc4 100644 > --- a/tools/testing/selftests/kvm/dirty_log_test.c > +++ b/tools/testing/selftests/kvm/dirty_log_test.c > @@ -534,6 +534,12 @@ static void *vcpu_worker(void *data) > sigemptyset(sigset); > sigaddset(sigset, SIG_IPI); > > + /* > + * Tell the main thread that signals are setup already; let's borrow > + * sem_vcpu_stop even if it's not for it. > + */ > + sem_post(&sem_vcpu_stop); > + > guest_array = addr_gva2hva(vm, (vm_vaddr_t)random_array); > > while (!READ_ONCE(host_quit)) { > @@ -785,6 +791,8 @@ static void run_test(enum vm_guest_mode mode, void *arg) > > pthread_create(&vcpu_thread, NULL, vcpu_worker, vm); > > + sem_wait_until(&sem_vcpu_stop); > + > while (iteration < p->iterations) { > /* Give the vcpu thread some time to dirty some pages */ > usleep(p->interval * 1000); >
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c index 510884f0eab8..25230e799bc4 100644 --- a/tools/testing/selftests/kvm/dirty_log_test.c +++ b/tools/testing/selftests/kvm/dirty_log_test.c @@ -534,6 +534,12 @@ static void *vcpu_worker(void *data) sigemptyset(sigset); sigaddset(sigset, SIG_IPI); + /* + * Tell the main thread that signals are setup already; let's borrow + * sem_vcpu_stop even if it's not for it. + */ + sem_post(&sem_vcpu_stop); + guest_array = addr_gva2hva(vm, (vm_vaddr_t)random_array); while (!READ_ONCE(host_quit)) { @@ -785,6 +791,8 @@ static void run_test(enum vm_guest_mode mode, void *arg) pthread_create(&vcpu_thread, NULL, vcpu_worker, vm); + sem_wait_until(&sem_vcpu_stop); + while (iteration < p->iterations) { /* Give the vcpu thread some time to dirty some pages */ usleep(p->interval * 1000);
The main thread could start to send SIG_IPI at any time, even before signal blocked on vcpu thread. Reuse the sem_vcpu_stop to sync on that, so when SIG_IPI is sent the signal will always land correctly as an -EINTR. Without this patch, on very busy cores the dirty_log_test could fail directly on receiving a SIG_USR1 without a handler (when vcpu runs far slower than main). Signed-off-by: Peter Xu <peterx@redhat.com> --- tools/testing/selftests/kvm/dirty_log_test.c | 8 ++++++++ 1 file changed, 8 insertions(+)