Skip to content

Commit 0a818b3

Browse files
committed
KVM: selftests: Keep dirty_log_test vCPU in guest until it needs to stop
In the dirty_log_test guest code, exit to userspace only when the vCPU is explicitly told to stop. Periodically exiting just to check if a flag has been set is unnecessary, weirdly complex, and wastes time handling exits that could be used to dirty memory. Opportunistically convert 'i' to a uint64_t to guard against the unlikely scenario that guest_num_pages exceeds the storage of an int. Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Link: https://lore.kernel.org/r/20250111003004.1235645-11-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent f3629c0 commit 0a818b3

File tree

1 file changed

+22
-21
lines changed

1 file changed

+22
-21
lines changed

tools/testing/selftests/kvm/dirty_log_test.c

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@
3131
/* Default guest test virtual memory offset */
3232
#define DEFAULT_GUEST_TEST_MEM 0xc0000000
3333

34-
/* How many pages to dirty for each guest loop */
35-
#define TEST_PAGES_PER_LOOP 1024
36-
3734
/* How many host loops to run (one KVM_GET_DIRTY_LOG for each loop) */
3835
#define TEST_HOST_LOOP_N 32UL
3936

@@ -75,6 +72,7 @@ static uint64_t host_page_size;
7572
static uint64_t guest_page_size;
7673
static uint64_t guest_num_pages;
7774
static uint64_t iteration;
75+
static bool vcpu_stop;
7876

7977
/*
8078
* Guest physical memory offset of the testing memory slot.
@@ -96,9 +94,10 @@ static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
9694
static void guest_code(void)
9795
{
9896
uint64_t addr;
99-
int i;
10097

10198
#ifdef __s390x__
99+
uint64_t i;
100+
102101
/*
103102
* On s390x, all pages of a 1M segment are initially marked as dirty
104103
* when a page of the segment is written to for the very first time.
@@ -112,7 +111,7 @@ static void guest_code(void)
112111
#endif
113112

114113
while (true) {
115-
for (i = 0; i < TEST_PAGES_PER_LOOP; i++) {
114+
while (!READ_ONCE(vcpu_stop)) {
116115
addr = guest_test_virt_mem;
117116
addr += (guest_random_u64(&guest_rng) % guest_num_pages)
118117
* guest_page_size;
@@ -140,14 +139,7 @@ static uint64_t host_track_next_count;
140139
/* Whether dirty ring reset is requested, or finished */
141140
static sem_t sem_vcpu_stop;
142141
static sem_t sem_vcpu_cont;
143-
/*
144-
* This is only set by main thread, and only cleared by vcpu thread. It is
145-
* used to request vcpu thread to stop at the next GUEST_SYNC, since GUEST_SYNC
146-
* is the only place that we'll guarantee both "dirty bit" and "dirty data"
147-
* will match. E.g., SIG_IPI won't guarantee that if the vcpu is interrupted
148-
* after setting dirty bit but before the data is written.
149-
*/
150-
static atomic_t vcpu_sync_stop_requested;
142+
151143
/*
152144
* This is updated by the vcpu thread to tell the host whether it's a
153145
* ring-full event. It should only be read until a sem_wait() of
@@ -272,9 +264,7 @@ static void clear_log_collect_dirty_pages(struct kvm_vcpu *vcpu, int slot,
272264
/* Should only be called after a GUEST_SYNC */
273265
static void vcpu_handle_sync_stop(void)
274266
{
275-
if (atomic_read(&vcpu_sync_stop_requested)) {
276-
/* It means main thread is sleeping waiting */
277-
atomic_set(&vcpu_sync_stop_requested, false);
267+
if (READ_ONCE(vcpu_stop)) {
278268
sem_post(&sem_vcpu_stop);
279269
sem_wait(&sem_vcpu_cont);
280270
}
@@ -801,20 +791,31 @@ static void run_test(enum vm_guest_mode mode, void *arg)
801791
}
802792

803793
/*
804-
* See vcpu_sync_stop_requested definition for details on why
805-
* we need to stop vcpu when verify data.
794+
* Stop the vCPU prior to collecting and verifying the dirty
795+
* log. If the vCPU is allowed to run during collection, then
796+
* pages that are written during this iteration may be missed,
797+
* i.e. collected in the next iteration. And if the vCPU is
798+
* writing memory during verification, pages that this thread
799+
* sees as clean may be written with this iteration's value.
806800
*/
807-
atomic_set(&vcpu_sync_stop_requested, true);
801+
WRITE_ONCE(vcpu_stop, true);
802+
sync_global_to_guest(vm, vcpu_stop);
808803
sem_wait(&sem_vcpu_stop);
804+
805+
/*
806+
* Clear vcpu_stop after the vCPU thread has acknowledge the
807+
* stop request and is waiting, i.e. is definitely not running!
808+
*/
809+
WRITE_ONCE(vcpu_stop, false);
810+
sync_global_to_guest(vm, vcpu_stop);
811+
809812
/*
810813
* NOTE: for dirty ring, it's possible that we didn't stop at
811814
* GUEST_SYNC but instead we stopped because ring is full;
812815
* that's okay too because ring full means we're only missing
813816
* the flush of the last page, and since we handle the last
814817
* page specially verification will succeed anyway.
815818
*/
816-
assert(host_log_mode == LOG_MODE_DIRTY_RING ||
817-
atomic_read(&vcpu_sync_stop_requested) == false);
818819
vm_dirty_log_verify(mode, bmap);
819820

820821
/*

0 commit comments

Comments
 (0)