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-pet...@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

Reply via email to