On Tue, Apr 20, 2021 at 10:07:16AM +0200, Paolo Bonzini wrote: > On 18/04/21 14:43, Peter Xu wrote: > > ----8<----- > > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c > > index 25230e799bc4..d3050d1c2cd0 100644 > > --- a/tools/testing/selftests/kvm/dirty_log_test.c > > +++ b/tools/testing/selftests/kvm/dirty_log_test.c > > @@ -377,7 +377,7 @@ static void dirty_ring_after_vcpu_run(struct kvm_vm *vm, int ret, int err) > > /* A ucall-sync or ring-full event is allowed */ > > if (get_ucall(vm, VCPU_ID, NULL) == UCALL_SYNC) { > > /* We should allow this to continue */ > > - ; > > + vcpu_handle_sync_stop(); > > } else if (run->exit_reason == KVM_EXIT_DIRTY_RING_FULL || > > (ret == -1 && err == EINTR)) { > > /* Update the flag first before pause */ > > ----8<----- > > > > That's my intention when I introduce vcpu_handle_sync_stop(), but forgot to > > add... > > And possibly even this (untested though): > > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c > index ffa4e2791926..918954f01cef 100644 > --- a/tools/testing/selftests/kvm/dirty_log_test.c > +++ b/tools/testing/selftests/kvm/dirty_log_test.c > @@ -383,6 +383,7 @@ static void dirty_ring_after_vcpu_run(struct kvm_vm *vm, int ret, int err) > /* Update the flag first before pause */ > WRITE_ONCE(dirty_ring_vcpu_ring_full, > run->exit_reason == KVM_EXIT_DIRTY_RING_FULL); > + atomic_set(&vcpu_sync_stop_requested, false); > sem_post(&sem_vcpu_stop); > pr_info("vcpu stops because %s...\n", > dirty_ring_vcpu_ring_full ? > @@ -804,8 +805,7 @@ static void run_test(enum vm_guest_mode mode, void *arg) > * the flush of the last page, and since we handle the last > * page specially verification will succeed anyway. > */ > - assert(host_log_mode == LOG_MODE_DIRTY_RING || > - atomic_read(&vcpu_sync_stop_requested) == false); > + assert(atomic_read(&vcpu_sync_stop_requested) == false); > vm_dirty_log_verify(mode, bmap); > sem_post(&sem_vcpu_cont); > > You can submit all these as a separate patch. But it could race, then? main thread vcpu thread ----------- ----------- ring full vcpu_sync_stop_requested=0 sem_post(&sem_vcpu_stop) vcpu_sync_stop_requested=1 sem_wait(&sem_vcpu_stop) assert(vcpu_sync_stop_requested==0) <---- Thanks, -- Peter Xu