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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201102222102.GE20600@xz-x1>
Date:   Mon, 2 Nov 2020 17:21:02 -0500
From:   Peter Xu <peterx@...hat.com>
To:     Ben Gardon <bgardon@...gle.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-kselftest@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Andrew Jones <drjones@...hat.com>,
        Peter Shier <pshier@...gle.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        Thomas Huth <thuth@...hat.com>,
        Peter Feiner <pfeiner@...gle.com>
Subject: Re: [PATCH 5/5] KVM: selftests: Introduce the dirty log perf test

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