| Message ID | 20241214010721.2356923-6-seanjc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/20] KVM: selftests: Support multiple write retires in dirty_log_test | expand |
On Fri, 2024-12-13 at 17:07 -0800, Sean Christopherson wrote: > Track and print the number of dirty and clear pages for each iteration. > This provides parity between all log modes, and will allow collecting the > dirty ring multiple times per iteration without spamming the console. > > Opportunistically drop the "Dirtied N pages" print, which is redundant > and wrong. For the dirty ring testcase, the vCPU isn't guaranteed to > complete a loop. And when the vCPU does complete a loot, there are no Typo > guarantees that it has *dirtied* that many pages; because the writes are > to random address, the vCPU may have written the same page over and over, > i.e. only dirtied one page. Counting how many times a vCPU wrote is also a valid statistic I think it would be the best to include it as well (e.g call it number of loops that vCPU did). Best regards, Maxim Levitsky > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > tools/testing/selftests/kvm/dirty_log_test.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c > index 55a744373c80..08cbecd1a135 100644 > --- a/tools/testing/selftests/kvm/dirty_log_test.c > +++ b/tools/testing/selftests/kvm/dirty_log_test.c > @@ -388,8 +388,6 @@ static void dirty_ring_collect_dirty_pages(struct kvm_vcpu *vcpu, int slot, > > if (READ_ONCE(dirty_ring_vcpu_ring_full)) > dirty_ring_continue_vcpu(); > - > - pr_info("Iteration %ld collected %u pages\n", iteration, count); > } > > static void dirty_ring_after_vcpu_run(struct kvm_vcpu *vcpu) > @@ -508,24 +506,20 @@ static void log_mode_after_vcpu_run(struct kvm_vcpu *vcpu) > static void *vcpu_worker(void *data) > { > struct kvm_vcpu *vcpu = data; > - uint64_t pages_count = 0; > > while (!READ_ONCE(host_quit)) { > - pages_count += TEST_PAGES_PER_LOOP; > /* Let the guest dirty the random pages */ > vcpu_run(vcpu); > log_mode_after_vcpu_run(vcpu); > } > > - pr_info("Dirtied %"PRIu64" pages\n", pages_count); > - > return NULL; > } > > static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) > { > + uint64_t page, nr_dirty_pages = 0, nr_clean_pages = 0; > uint64_t step = vm_num_host_pages(mode, 1); > - uint64_t page; > uint64_t *value_ptr; > uint64_t min_iter = 0; > > @@ -544,7 +538,7 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) > if (__test_and_clear_bit_le(page, bmap)) { > bool matched; > > - host_dirty_count++; > + nr_dirty_pages++; > > /* > * If the bit is set, the value written onto > @@ -605,7 +599,7 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) > " incorrect (iteration=%"PRIu64")", > page, *value_ptr, iteration); > } else { > - host_clear_count++; > + nr_clean_pages++; > /* > * If cleared, the value written can be any > * value smaller or equals to the iteration > @@ -639,6 +633,12 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) > } > } > } > + > + pr_info("Iteration %2ld: dirty: %-6lu clean: %-6lu\n", > + iteration, nr_dirty_pages, nr_clean_pages); > + > + host_dirty_count += nr_dirty_pages; > + host_clear_count += nr_clean_pages; > } > > static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
On Tue, Dec 17, 2024, Maxim Levitsky wrote: > On Fri, 2024-12-13 at 17:07 -0800, Sean Christopherson wrote: > > Track and print the number of dirty and clear pages for each iteration. > > This provides parity between all log modes, and will allow collecting the > > dirty ring multiple times per iteration without spamming the console. > > > > Opportunistically drop the "Dirtied N pages" print, which is redundant > > and wrong. For the dirty ring testcase, the vCPU isn't guaranteed to > > complete a loop. And when the vCPU does complete a loot, there are no > Typo > > guarantees that it has *dirtied* that many pages; because the writes are > > to random address, the vCPU may have written the same page over and over, > > i.e. only dirtied one page. > > Counting how many times a vCPU wrote is also a valid statistic > > I think it would be the best to include it as well (e.g call it number of > loops that vCPU did). Heh, I originally had it that way, but dropped it because it didn't seem all that interesting. I'll add it back.
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c index 55a744373c80..08cbecd1a135 100644 --- a/tools/testing/selftests/kvm/dirty_log_test.c +++ b/tools/testing/selftests/kvm/dirty_log_test.c @@ -388,8 +388,6 @@ static void dirty_ring_collect_dirty_pages(struct kvm_vcpu *vcpu, int slot, if (READ_ONCE(dirty_ring_vcpu_ring_full)) dirty_ring_continue_vcpu(); - - pr_info("Iteration %ld collected %u pages\n", iteration, count); } static void dirty_ring_after_vcpu_run(struct kvm_vcpu *vcpu) @@ -508,24 +506,20 @@ static void log_mode_after_vcpu_run(struct kvm_vcpu *vcpu) static void *vcpu_worker(void *data) { struct kvm_vcpu *vcpu = data; - uint64_t pages_count = 0; while (!READ_ONCE(host_quit)) { - pages_count += TEST_PAGES_PER_LOOP; /* Let the guest dirty the random pages */ vcpu_run(vcpu); log_mode_after_vcpu_run(vcpu); } - pr_info("Dirtied %"PRIu64" pages\n", pages_count); - return NULL; } static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) { + uint64_t page, nr_dirty_pages = 0, nr_clean_pages = 0; uint64_t step = vm_num_host_pages(mode, 1); - uint64_t page; uint64_t *value_ptr; uint64_t min_iter = 0; @@ -544,7 +538,7 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) if (__test_and_clear_bit_le(page, bmap)) { bool matched; - host_dirty_count++; + nr_dirty_pages++; /* * If the bit is set, the value written onto @@ -605,7 +599,7 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) " incorrect (iteration=%"PRIu64")", page, *value_ptr, iteration); } else { - host_clear_count++; + nr_clean_pages++; /* * If cleared, the value written can be any * value smaller or equals to the iteration @@ -639,6 +633,12 @@ static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap) } } } + + pr_info("Iteration %2ld: dirty: %-6lu clean: %-6lu\n", + iteration, nr_dirty_pages, nr_clean_pages); + + host_dirty_count += nr_dirty_pages; + host_clear_count += nr_clean_pages; } static struct kvm_vm *create_vm(enum vm_guest_mode mode, struct kvm_vcpu **vcpu,
Track and print the number of dirty and clear pages for each iteration. This provides parity between all log modes, and will allow collecting the dirty ring multiple times per iteration without spamming the console. Opportunistically drop the "Dirtied N pages" print, which is redundant and wrong. For the dirty ring testcase, the vCPU isn't guaranteed to complete a loop. And when the vCPU does complete a loot, there are no guarantees that it has *dirtied* that many pages; because the writes are to random address, the vCPU may have written the same page over and over, i.e. only dirtied one page. Signed-off-by: Sean Christopherson <seanjc@google.com> --- tools/testing/selftests/kvm/dirty_log_test.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)