Re: [PATCH 05/20] KVM: selftests: Precisely track number of dirty/clear pages for each iteration

Maxim Levitsky
2024-12-17T23:59:47Z

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,