Re: [PATCH 5/5] KVM: selftests: Introduce the dirty log perf test

[Peter Xu <peterx@redhat.com>]

On Tue, Oct 27, 2020 at 04:37:33PM -0700, Ben Gardon wrote:
> The dirty log perf test will time verious dirty logging operations
> (enabling dirty logging, dirtying memory, getting the dirty log,
> clearing the dirty log, and disabling dirty logging) in order to
> quantify dirty logging performance. This test can be used to inform
> future performance improvements to KVM's dirty logging infrastructure.

One thing to mention is that there're a few patches in the kvm dirty ring
series that reworked the dirty log test quite a bit (to add similar test for
dirty ring).  For example:

  https://lore.kernel.org/kvm/20201023183358.50607-11-peterx@redhat.com/

Just a FYI if we're going to use separate test programs.  Merging this tests
should benefit in many ways, of course (e.g., dirty ring may directly runnable
with the perf tests too; so we can manually enable this "perf mode" as a new
parameter in dirty_log_test, if possible?), however I don't know how hard -
maybe there's some good reason to keep them separate...

[...]

> +static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> +		     uint64_t phys_offset, int vcpus,
> +		     uint64_t vcpu_memory_bytes, int wr_fract)
> +{

[...]

> +	/* Start the iterations */
> +	iteration = 0;
> +	host_quit = false;
> +
> +	clock_gettime(CLOCK_MONOTONIC, &start);
> +	for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
> +		pthread_create(&vcpu_threads[vcpu_id], NULL, vcpu_worker,
> +			       &perf_test_args.vcpu_args[vcpu_id]);
> +	}
> +
> +	/* Allow the vCPU to populate memory */
> +	pr_debug("Starting iteration %lu - Populating\n", iteration);
> +	while (READ_ONCE(vcpu_last_completed_iteration[vcpu_id]) != iteration)
> +		pr_debug("Waiting for vcpu_last_completed_iteration == %lu\n",
> +			iteration);

Isn't array vcpu_last_completed_iteration[] initialized to all zeros?  If so, I
feel like this "while" won't run as expected to wait for populating mem.

The flooding pr_debug() seems a bit scary too if the mem size is huge..  How
about a pr_debug() after the loop (so if we don't see that it means it hanged)?

(There's another similar pr_debug() after this point too within a loop)

Thanks,

-- 
Peter Xu