[PATCH v2 0/3] Randomize memory access of dirty_log_perf_test

[PATCH v2 0/3] Randomize memory access of dirty_log_perf_test

[Colton Lewis]

This patch adds the ability to randomize parts of dirty_log_perf_test,
specifically the order pages are accessed and whether pages are read
or written. This is implemented through creating an array of random
numbers for each vCPU when the VM is created.

Colton Lewis (3):
  KVM: selftests: Create source of randomness for guest code.
  KVM: selftests: Randomize which pages are written vs read.
  KVM: selftests: Randomize page access order.

 .../selftests/kvm/access_tracking_perf_test.c |  2 +-
 .../selftests/kvm/dirty_log_perf_test.c       | 47 +++++++++-----
 .../selftests/kvm/include/perf_test_util.h    |  9 ++-
 .../selftests/kvm/lib/perf_test_util.c        | 65 +++++++++++++++++--
 4 files changed, 99 insertions(+), 24 deletions(-)

-- 
2.37.1.595.g718a3a8f04-goog

[PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[Colton Lewis]

Create for each vcpu an array of random numbers to be used as a source
of randomness when executing guest code. Randomness should be useful
for approaching more realistic conditions in dirty_log_perf_test.

Create a -r argument to specify a random seed. If no argument
is provided, the seed defaults to the current Unix timestamp.

The random arrays are allocated and filled as part of VM creation. The
additional memory used is one 32-bit number per guest VM page to
ensure one number for each iteration of the inner loop of guest_code.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/dirty_log_perf_test.c       | 11 ++++-
 .../selftests/kvm/include/perf_test_util.h    |  3 ++
 .../selftests/kvm/lib/perf_test_util.c        | 45 ++++++++++++++++++-
 3 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index f99e39a672d3..362b946019e9 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -132,6 +132,7 @@ struct test_params {
 	bool partition_vcpu_memory_access;
 	enum vm_mem_backing_src_type backing_src;
 	int slots;
+	uint32_t random_seed;
 };
 
 static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
@@ -225,6 +226,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 				 p->slots, p->backing_src,
 				 p->partition_vcpu_memory_access);
 
+	perf_test_set_random_seed(vm, p->random_seed);
 	perf_test_set_wr_fract(vm, p->wr_fract);
 
 	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
@@ -352,7 +354,7 @@ static void help(char *name)
 {
 	puts("");
 	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
-	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-s mem type]"
+	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
 	       "[-x memslots]\n", name);
 	puts("");
 	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
@@ -380,6 +382,7 @@ static void help(char *name)
 	printf(" -v: specify the number of vCPUs to run.\n");
 	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
 	       "     them into a separate region of memory for each vCPU.\n");
+	printf(" -r: specify the starting random seed.\n");
 	backing_src_help("-s");
 	printf(" -x: Split the memory region into this number of memslots.\n"
 	       "     (default: 1)\n");
@@ -396,6 +399,7 @@ int main(int argc, char *argv[])
 		.partition_vcpu_memory_access = true,
 		.backing_src = DEFAULT_VM_MEM_SRC,
 		.slots = 1,
+		.random_seed = time(NULL),
 	};
 	int opt;
 
@@ -406,7 +410,7 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
+	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:or:s:x:")) != -1) {
 		switch (opt) {
 		case 'e':
 			/* 'e' is for evil. */
@@ -442,6 +446,9 @@ int main(int argc, char *argv[])
 		case 'o':
 			p.partition_vcpu_memory_access = false;
 			break;
+		case 'r':
+			p.random_seed = atoi(optarg);
+			break;
 		case 's':
 			p.backing_src = parse_backing_src_type(optarg);
 			break;
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index eaa88df0555a..9517956424fc 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -23,6 +23,7 @@ struct perf_test_vcpu_args {
 	uint64_t gpa;
 	uint64_t gva;
 	uint64_t pages;
+	vm_vaddr_t random_array;
 
 	/* Only used by the host userspace part of the vCPU thread */
 	struct kvm_vcpu *vcpu;
@@ -35,6 +36,7 @@ struct perf_test_args {
 	uint64_t gpa;
 	uint64_t size;
 	uint64_t guest_page_size;
+	uint32_t random_seed;
 	int wr_fract;
 
 	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
@@ -52,6 +54,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 void perf_test_destroy_vm(struct kvm_vm *vm);
 
 void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract);
+void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
 
 void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
 void perf_test_join_vcpu_threads(int vcpus);
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index 9618b37c66f7..8d85923acb4e 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -70,6 +70,35 @@ void perf_test_guest_code(uint32_t vcpu_idx)
 	}
 }
 
+void perf_test_alloc_random_arrays(uint32_t nr_vcpus, uint32_t nr_randoms)
+{
+	struct perf_test_args *pta = &perf_test_args;
+
+	for (uint32_t i = 0; i < nr_vcpus; i++) {
+		pta->vcpu_args[i].random_array = vm_vaddr_alloc(
+			pta->vm,
+			nr_randoms * sizeof(uint32_t),
+			KVM_UTIL_MIN_VADDR);
+		pr_debug("Random row %d vaddr: %p.\n", i, (void *)pta->vcpu_args[i].random_array);
+	}
+}
+
+void perf_test_fill_random_arrays(uint32_t nr_vcpus, uint32_t nr_randoms)
+{
+	struct perf_test_args *pta = &perf_test_args;
+	uint32_t *host_row;
+
+	for (uint32_t i = 0; i < nr_vcpus; i++) {
+		host_row = addr_gva2hva(pta->vm, pta->vcpu_args[i].random_array);
+
+		for (uint32_t j = 0; j < nr_randoms; j++)
+			host_row[j] = random();
+
+		pr_debug("New randoms row %d: %d, %d, %d...\n",
+			 i, host_row[0], host_row[1], host_row[2]);
+	}
+}
+
 void perf_test_setup_vcpus(struct kvm_vm *vm, int nr_vcpus,
 			   struct kvm_vcpu *vcpus[],
 			   uint64_t vcpu_memory_bytes,
@@ -120,8 +149,9 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
-	/* By default vCPUs will write to memory. */
-	pta->wr_fract = 1;
+	/* Set perf_test_args defaults. */
+	pta->wr_fract = 100;
+	pta->random_seed = time(NULL);
 
 	/*
 	 * Snapshot the non-huge page size.  This is used by the guest code to
@@ -211,6 +241,11 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 
 	ucall_init(vm, NULL);
 
+	srandom(perf_test_args.random_seed);
+	pr_info("Random seed: %d\n", perf_test_args.random_seed);
+	perf_test_alloc_random_arrays(nr_vcpus, vcpu_memory_bytes >> vm->page_shift);
+	perf_test_fill_random_arrays(nr_vcpus, vcpu_memory_bytes >> vm->page_shift);
+
 	/* Export the shared variables to the guest. */
 	sync_global_to_guest(vm, perf_test_args);
 
@@ -229,6 +264,12 @@ void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract)
 	sync_global_to_guest(vm, perf_test_args);
 }
 
+void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
+{
+	perf_test_args.random_seed = random_seed;
+	sync_global_to_guest(vm, perf_test_args);
+}
+
 uint64_t __weak perf_test_nested_pages(int nr_vcpus)
 {
 	return 0;
-- 
2.37.1.595.g718a3a8f04-goog

Re: [PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[David Matlack]

On Wed, Aug 17, 2022 at 09:41:44PM +0000, Colton Lewis wrote:
> Create for each vcpu an array of random numbers to be used as a source
> of randomness when executing guest code. Randomness should be useful
> for approaching more realistic conditions in dirty_log_perf_test.
> 
> Create a -r argument to specify a random seed. If no argument
> is provided, the seed defaults to the current Unix timestamp.
> 
> The random arrays are allocated and filled as part of VM creation. The
> additional memory used is one 32-bit number per guest VM page to
> ensure one number for each iteration of the inner loop of guest_code.

nit: I find it helpful to put this in more practical terms as well. e.g.

  The random arrays are allocated and filled as part of VM creation. The
  additional memory used is one 32-bit number per guest VM page (so
  1MiB of additional memory when using the default 1GiB per-vCPU region
  size) to ensure one number for each iteration of the inner loop of
  guest_code.

Speaking of, this commit always allocates the random arrays, even if
they are not used. I think that's probably fine but it deserves to be
called out in the commit description with an explanation of why it is
the way it is.

> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/dirty_log_perf_test.c       | 11 ++++-
>  .../selftests/kvm/include/perf_test_util.h    |  3 ++
>  .../selftests/kvm/lib/perf_test_util.c        | 45 ++++++++++++++++++-
>  3 files changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index f99e39a672d3..362b946019e9 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -132,6 +132,7 @@ struct test_params {
>  	bool partition_vcpu_memory_access;
>  	enum vm_mem_backing_src_type backing_src;
>  	int slots;
> +	uint32_t random_seed;
>  };
>  
>  static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
> @@ -225,6 +226,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  				 p->slots, p->backing_src,
>  				 p->partition_vcpu_memory_access);
>  
> +	perf_test_set_random_seed(vm, p->random_seed);
>  	perf_test_set_wr_fract(vm, p->wr_fract);
>  
>  	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
> @@ -352,7 +354,7 @@ static void help(char *name)
>  {
>  	puts("");
>  	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
> -	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-s mem type]"
> +	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
>  	       "[-x memslots]\n", name);
>  	puts("");
>  	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
> @@ -380,6 +382,7 @@ static void help(char *name)
>  	printf(" -v: specify the number of vCPUs to run.\n");
>  	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
>  	       "     them into a separate region of memory for each vCPU.\n");
> +	printf(" -r: specify the starting random seed.\n");
>  	backing_src_help("-s");
>  	printf(" -x: Split the memory region into this number of memslots.\n"
>  	       "     (default: 1)\n");
> @@ -396,6 +399,7 @@ int main(int argc, char *argv[])
>  		.partition_vcpu_memory_access = true,
>  		.backing_src = DEFAULT_VM_MEM_SRC,
>  		.slots = 1,
> +		.random_seed = time(NULL),
>  	};
>  	int opt;
>  
> @@ -406,7 +410,7 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:or:s:x:")) != -1) {
>  		switch (opt) {
>  		case 'e':
>  			/* 'e' is for evil. */
> @@ -442,6 +446,9 @@ int main(int argc, char *argv[])
>  		case 'o':
>  			p.partition_vcpu_memory_access = false;
>  			break;
> +		case 'r':
> +			p.random_seed = atoi(optarg);

Putting the random seed in test_params seems unnecessary. This flag
could just set perf_test_args.randome_seed directly. Doing this would
avoid the need for duplicating the time(NULL) initialization, and allow
you to drop perf_test_set_random_seed().

> +			break;
>  		case 's':
>  			p.backing_src = parse_backing_src_type(optarg);
>  			break;
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index eaa88df0555a..9517956424fc 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -23,6 +23,7 @@ struct perf_test_vcpu_args {
>  	uint64_t gpa;
>  	uint64_t gva;
>  	uint64_t pages;
> +	vm_vaddr_t random_array;
>  
>  	/* Only used by the host userspace part of the vCPU thread */
>  	struct kvm_vcpu *vcpu;
> @@ -35,6 +36,7 @@ struct perf_test_args {
>  	uint64_t gpa;
>  	uint64_t size;
>  	uint64_t guest_page_size;
> +	uint32_t random_seed;
>  	int wr_fract;
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
> @@ -52,6 +54,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  void perf_test_destroy_vm(struct kvm_vm *vm);
>  
>  void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract);
> +void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
>  
>  void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
>  void perf_test_join_vcpu_threads(int vcpus);
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index 9618b37c66f7..8d85923acb4e 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -70,6 +70,35 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  	}
>  }
>  
> +void perf_test_alloc_random_arrays(uint32_t nr_vcpus, uint32_t nr_randoms)
> +{
> +	struct perf_test_args *pta = &perf_test_args;
> +
> +	for (uint32_t i = 0; i < nr_vcpus; i++) {
> +		pta->vcpu_args[i].random_array = vm_vaddr_alloc(
> +			pta->vm,
> +			nr_randoms * sizeof(uint32_t),
> +			KVM_UTIL_MIN_VADDR);

> +		pr_debug("Random row %d vaddr: %p.\n", i, (void *)pta->vcpu_args[i].random_array);
> +	}
> +}
> +
> +void perf_test_fill_random_arrays(uint32_t nr_vcpus, uint32_t nr_randoms)
> +{
> +	struct perf_test_args *pta = &perf_test_args;
> +	uint32_t *host_row;

"host_row" is a confusing variable name here since you are no longer
operating on a 2D array. Each vCPU has it's own array.

> +
> +	for (uint32_t i = 0; i < nr_vcpus; i++) {
> +		host_row = addr_gva2hva(pta->vm, pta->vcpu_args[i].random_array);
> +
> +		for (uint32_t j = 0; j < nr_randoms; j++)
> +			host_row[j] = random();
> +
> +		pr_debug("New randoms row %d: %d, %d, %d...\n",
> +			 i, host_row[0], host_row[1], host_row[2]);
> +	}
> +}
> +
>  void perf_test_setup_vcpus(struct kvm_vm *vm, int nr_vcpus,
>  			   struct kvm_vcpu *vcpus[],
>  			   uint64_t vcpu_memory_bytes,
> @@ -120,8 +149,9 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
> -	/* By default vCPUs will write to memory. */
> -	pta->wr_fract = 1;
> +	/* Set perf_test_args defaults. */
> +	pta->wr_fract = 100;
> +	pta->random_seed = time(NULL);

Won't this override write the random_seed provided by the test?

>  
>  	/*
>  	 * Snapshot the non-huge page size.  This is used by the guest code to
> @@ -211,6 +241,11 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  
>  	ucall_init(vm, NULL);
>  
> +	srandom(perf_test_args.random_seed);
> +	pr_info("Random seed: %d\n", perf_test_args.random_seed);
> +	perf_test_alloc_random_arrays(nr_vcpus, vcpu_memory_bytes >> vm->page_shift);
> +	perf_test_fill_random_arrays(nr_vcpus, vcpu_memory_bytes >> vm->page_shift);

I think this is broken if !partition_vcpu_memory_access. nr_randoms
(per-vCPU) should be `nr_vcpus * vcpu_memory_bytes >> vm->page_shift`.

Speaking of, this should probably just be done in
perf_test_setup_vcpus(). That way you can just use vcpu_args->pages for
nr_randoms, and you don't have to add another for-each-vcpu loop.

> +
>  	/* Export the shared variables to the guest. */
>  	sync_global_to_guest(vm, perf_test_args);
>  
> @@ -229,6 +264,12 @@ void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract)
>  	sync_global_to_guest(vm, perf_test_args);
>  }
>  
> +void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> +{
> +	perf_test_args.random_seed = random_seed;
> +	sync_global_to_guest(vm, perf_test_args);

sync_global_to_guest() is unnecessary here since the guest does not need
to access perf_test_args.random_seed (and I can't imagine it ever will).

That being said, I think you can drop this function (see earlier
comment).

> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> -- 
> 2.37.1.595.g718a3a8f04-goog
> 

Re: [PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[Colton Lewis]

David Matlack <dmatlack@google.com> writes:

> On Wed, Aug 17, 2022 at 09:41:44PM +0000, Colton Lewis wrote:
>> Create for each vcpu an array of random numbers to be used as a source
>> of randomness when executing guest code. Randomness should be useful
>> for approaching more realistic conditions in dirty_log_perf_test.

>> Create a -r argument to specify a random seed. If no argument
>> is provided, the seed defaults to the current Unix timestamp.

>> The random arrays are allocated and filled as part of VM creation. The
>> additional memory used is one 32-bit number per guest VM page to
>> ensure one number for each iteration of the inner loop of guest_code.

> nit: I find it helpful to put this in more practical terms as well. e.g.

>    The random arrays are allocated and filled as part of VM creation. The
>    additional memory used is one 32-bit number per guest VM page (so
>    1MiB of additional memory when using the default 1GiB per-vCPU region
>    size) to ensure one number for each iteration of the inner loop of
>    guest_code.

> Speaking of, this commit always allocates the random arrays, even if
> they are not used. I think that's probably fine but it deserves to be
> called out in the commit description with an explanation of why it is
> the way it is.


I'll add a concrete example and explain always allocating.

Technically, the random array will always be accessed even if it never
changes the code path when write_percent = 0 or write_percent =
100. Always allocating avoids extra code that adds complexity for no
benefit.

>> @@ -442,6 +446,9 @@ int main(int argc, char *argv[])
>>   		case 'o':
>>   			p.partition_vcpu_memory_access = false;
>>   			break;
>> +		case 'r':
>> +			p.random_seed = atoi(optarg);

> Putting the random seed in test_params seems unnecessary. This flag
> could just set perf_test_args.randome_seed directly. Doing this would
> avoid the need for duplicating the time(NULL) initialization, and allow
> you to drop perf_test_set_random_seed().


I understand there is some redundancy in this approach and had
originally done what you suggest. My reason for changing was based on
the consideration that perf_test_util.c is library code that is used for
other tests and could be used for more in the future, so it should
always initialize everything in perf_test_args rather than rely on the
test to do it. This is what is done for wr_fract (renamed write_percent
later in this patch). perf_test_set_random_seed is there for interface
consistency with the other perf_test_set* functions.

But per the point below, moving random generation to VM creation means
we are dependent on the seed being set before then. So I do need a
change here, but I'd rather keep the redundancy and
perf_test_set_random_seed.

>> +void perf_test_fill_random_arrays(uint32_t nr_vcpus, uint32_t  
>> nr_randoms)
>> +{
>> +	struct perf_test_args *pta = &perf_test_args;
>> +	uint32_t *host_row;

> "host_row" is a confusing variable name here since you are no longer
> operating on a 2D array. Each vCPU has it's own array.


Agreed.

>> @@ -120,8 +149,9 @@ struct kvm_vm *perf_test_create_vm(enum  
>> vm_guest_mode mode, int nr_vcpus,

>>   	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));

>> -	/* By default vCPUs will write to memory. */
>> -	pta->wr_fract = 1;
>> +	/* Set perf_test_args defaults. */
>> +	pta->wr_fract = 100;
>> +	pta->random_seed = time(NULL);

> Won't this override write the random_seed provided by the test?


Yes. I had thought I accounted for that by setting random_seed later in
run_test, but now realize that has no effect because I moved the
generation of all the random numbers to happen before then.


>>   	/*
>>   	 * Snapshot the non-huge page size.  This is used by the guest code to
>> @@ -211,6 +241,11 @@ struct kvm_vm *perf_test_create_vm(enum  
>> vm_guest_mode mode, int nr_vcpus,

>>   	ucall_init(vm, NULL);

>> +	srandom(perf_test_args.random_seed);
>> +	pr_info("Random seed: %d\n", perf_test_args.random_seed);
>> +	perf_test_alloc_random_arrays(nr_vcpus, vcpu_memory_bytes >>  
>> vm->page_shift);
>> +	perf_test_fill_random_arrays(nr_vcpus, vcpu_memory_bytes >>  
>> vm->page_shift);

> I think this is broken if !partition_vcpu_memory_access. nr_randoms
> (per-vCPU) should be `nr_vcpus * vcpu_memory_bytes >> vm->page_shift`.


Agree it will break then and should not. But allocating that many more
random numbers may eat too much memory. In a test with 64 vcpus, it would
try to allocate 64x64 times as many random numbers. I'll try it but may
need something different in that case.

> Speaking of, this should probably just be done in
> perf_test_setup_vcpus(). That way you can just use vcpu_args->pages for
> nr_randoms, and you don't have to add another for-each-vcpu loop.


Agreed.

>> +
>>   	/* Export the shared variables to the guest. */
>>   	sync_global_to_guest(vm, perf_test_args);

>> @@ -229,6 +264,12 @@ void perf_test_set_wr_fract(struct kvm_vm *vm, int  
>> wr_fract)
>>   	sync_global_to_guest(vm, perf_test_args);
>>   }

>> +void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
>> +{
>> +	perf_test_args.random_seed = random_seed;
>> +	sync_global_to_guest(vm, perf_test_args);

> sync_global_to_guest() is unnecessary here since the guest does not need
> to access perf_test_args.random_seed (and I can't imagine it ever will).

> That being said, I think you can drop this function (see earlier
> comment).


See earlier response about consistency.

Re: [PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[Ricardo Koller]

On Tue, Aug 30, 2022 at 07:01:57PM +0000, Colton Lewis wrote:
> David Matlack <dmatlack@google.com> writes:
> 
> > On Wed, Aug 17, 2022 at 09:41:44PM +0000, Colton Lewis wrote:
> > > Create for each vcpu an array of random numbers to be used as a source
> > > of randomness when executing guest code. Randomness should be useful
> > > for approaching more realistic conditions in dirty_log_perf_test.
> 
> > > Create a -r argument to specify a random seed. If no argument
> > > is provided, the seed defaults to the current Unix timestamp.
> 
> > > The random arrays are allocated and filled as part of VM creation. The
> > > additional memory used is one 32-bit number per guest VM page to
> > > ensure one number for each iteration of the inner loop of guest_code.
> 
> > nit: I find it helpful to put this in more practical terms as well. e.g.
> 
> >    The random arrays are allocated and filled as part of VM creation. The
> >    additional memory used is one 32-bit number per guest VM page (so
> >    1MiB of additional memory when using the default 1GiB per-vCPU region
> >    size) to ensure one number for each iteration of the inner loop of
> >    guest_code.
> 
> > Speaking of, this commit always allocates the random arrays, even if
> > they are not used. I think that's probably fine but it deserves to be
> > called out in the commit description with an explanation of why it is
> > the way it is.
> 
> 
> I'll add a concrete example and explain always allocating.
> 
> Technically, the random array will always be accessed even if it never
> changes the code path when write_percent = 0 or write_percent =
> 100. Always allocating avoids extra code that adds complexity for no
> benefit.
> 
> > > @@ -442,6 +446,9 @@ int main(int argc, char *argv[])
> > >   		case 'o':
> > >   			p.partition_vcpu_memory_access = false;
> > >   			break;
> > > +		case 'r':
> > > +			p.random_seed = atoi(optarg);
> 
> > Putting the random seed in test_params seems unnecessary. This flag
> > could just set perf_test_args.randome_seed directly. Doing this would
> > avoid the need for duplicating the time(NULL) initialization, and allow
> > you to drop perf_test_set_random_seed().
> 
> 
> I understand there is some redundancy in this approach and had
> originally done what you suggest. My reason for changing was based on
> the consideration that perf_test_util.c is library code that is used for
> other tests and could be used for more in the future, so it should
> always initialize everything in perf_test_args rather than rely on the
> test to do it. This is what is done for wr_fract (renamed write_percent
> later in this patch). perf_test_set_random_seed is there for interface
> consistency with the other perf_test_set* functions.
> 
> But per the point below, moving random generation to VM creation means
> we are dependent on the seed being set before then. So I do need a
> change here, but I'd rather keep the redundancy and
> perf_test_set_random_seed.
> 
> > > +void perf_test_fill_random_arrays(uint32_t nr_vcpus, uint32_t
> > > nr_randoms)
> > > +{
> > > +	struct perf_test_args *pta = &perf_test_args;
> > > +	uint32_t *host_row;
> 
> > "host_row" is a confusing variable name here since you are no longer
> > operating on a 2D array. Each vCPU has it's own array.
> 
> 
> Agreed.
> 
> > > @@ -120,8 +149,9 @@ struct kvm_vm *perf_test_create_vm(enum
> > > vm_guest_mode mode, int nr_vcpus,
> 
> > >   	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
> 
> > > -	/* By default vCPUs will write to memory. */
> > > -	pta->wr_fract = 1;
> > > +	/* Set perf_test_args defaults. */
> > > +	pta->wr_fract = 100;
> > > +	pta->random_seed = time(NULL);
> 
> > Won't this override write the random_seed provided by the test?
> 
> 
> Yes. I had thought I accounted for that by setting random_seed later in
> run_test, but now realize that has no effect because I moved the
> generation of all the random numbers to happen before then.
> 
> 
> > >   	/*
> > >   	 * Snapshot the non-huge page size.  This is used by the guest code to
> > > @@ -211,6 +241,11 @@ struct kvm_vm *perf_test_create_vm(enum
> > > vm_guest_mode mode, int nr_vcpus,
> 
> > >   	ucall_init(vm, NULL);
> 
> > > +	srandom(perf_test_args.random_seed);
> > > +	pr_info("Random seed: %d\n", perf_test_args.random_seed);
> > > +	perf_test_alloc_random_arrays(nr_vcpus, vcpu_memory_bytes >>
> > > vm->page_shift);
> > > +	perf_test_fill_random_arrays(nr_vcpus, vcpu_memory_bytes >>
> > > vm->page_shift);
> 
> > I think this is broken if !partition_vcpu_memory_access. nr_randoms
> > (per-vCPU) should be `nr_vcpus * vcpu_memory_bytes >> vm->page_shift`.
> 
> 
> Agree it will break then and should not. But allocating that many more
> random numbers may eat too much memory. In a test with 64 vcpus, it would
> try to allocate 64x64 times as many random numbers. I'll try it but may
> need something different in that case.

You might want to reconsider the idea of using a random number generator
inside the guest. IRC the reasons against it were: quality of the random
numbers, and that some random generators use floating-point numbers. I
don't think the first one is a big issue. The second one might be an
issue if we want to generate non-uniform distributions (e.g., poisson);
but not a problem for now.

> 
> > Speaking of, this should probably just be done in
> > perf_test_setup_vcpus(). That way you can just use vcpu_args->pages for
> > nr_randoms, and you don't have to add another for-each-vcpu loop.
> 
> 
> Agreed.
> 
> > > +
> > >   	/* Export the shared variables to the guest. */
> > >   	sync_global_to_guest(vm, perf_test_args);
> 
> > > @@ -229,6 +264,12 @@ void perf_test_set_wr_fract(struct kvm_vm *vm,
> > > int wr_fract)
> > >   	sync_global_to_guest(vm, perf_test_args);
> > >   }
> 
> > > +void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> > > +{
> > > +	perf_test_args.random_seed = random_seed;
> > > +	sync_global_to_guest(vm, perf_test_args);
> 
> > sync_global_to_guest() is unnecessary here since the guest does not need
> > to access perf_test_args.random_seed (and I can't imagine it ever will).
> 
> > That being said, I think you can drop this function (see earlier
> > comment).
> 
> 
> See earlier response about consistency.

Re: [PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[Sean Christopherson]

On Thu, Sep 01, 2022, Ricardo Koller wrote:
> On Tue, Aug 30, 2022 at 07:01:57PM +0000, Colton Lewis wrote:
> > David Matlack <dmatlack@google.com> writes:
> > > I think this is broken if !partition_vcpu_memory_access. nr_randoms
> > > (per-vCPU) should be `nr_vcpus * vcpu_memory_bytes >> vm->page_shift`.
> > 
> > 
> > Agree it will break then and should not. But allocating that many more
> > random numbers may eat too much memory. In a test with 64 vcpus, it would
> > try to allocate 64x64 times as many random numbers. I'll try it but may
> > need something different in that case.
> 
> You might want to reconsider the idea of using a random number generator
> inside the guest. IRC the reasons against it were: quality of the random
> numbers, and that some random generators use floating-point numbers. I
> don't think the first one is a big issue. The second one might be an
> issue if we want to generate non-uniform distributions (e.g., poisson);
> but not a problem for now.

I'm pretty I had coded up a pseudo-RNG framework for selftests at one point, but
I cant find the code in my morass of branches and stashes :-(
 
Whatever we do, make sure the randomness and thus any failures are easily
reproducible, i.e. the RNG needs to be seeded pseudo-RNG, not fully random.

Re: [PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[Andrew Jones]

On Thu, Sep 01, 2022 at 06:22:05PM +0000, Sean Christopherson wrote:
> On Thu, Sep 01, 2022, Ricardo Koller wrote:
> > On Tue, Aug 30, 2022 at 07:01:57PM +0000, Colton Lewis wrote:
> > > David Matlack <dmatlack@google.com> writes:
> > > > I think this is broken if !partition_vcpu_memory_access. nr_randoms
> > > > (per-vCPU) should be `nr_vcpus * vcpu_memory_bytes >> vm->page_shift`.
> > > 
> > > 
> > > Agree it will break then and should not. But allocating that many more
> > > random numbers may eat too much memory. In a test with 64 vcpus, it would
> > > try to allocate 64x64 times as many random numbers. I'll try it but may
> > > need something different in that case.
> > 
> > You might want to reconsider the idea of using a random number generator
> > inside the guest. IRC the reasons against it were: quality of the random
> > numbers, and that some random generators use floating-point numbers. I
> > don't think the first one is a big issue. The second one might be an
> > issue if we want to generate non-uniform distributions (e.g., poisson);
> > but not a problem for now.
> 
> I'm pretty I had coded up a pseudo-RNG framework for selftests at one point, but
> I cant find the code in my morass of branches and stashes :-(

Here's an option that was once proposed by Alex Bennee for kvm-unit-tests.
(Someday we need to revisit that MTTCG series...)

https://gitlab.com/rhdrjones/kvm-unit-tests/-/commit/2fc51790577c9f91214f72ef51ee5d0e8bbb1b7e

Thanks,
drew

>  
> Whatever we do, make sure the randomness and thus any failures are easily
> reproducible, i.e. the RNG needs to be seeded pseudo-RNG, not fully random.

Re: [PATCH v2 1/3] KVM: selftests: Create source of randomness for guest code.

[Ricardo Koller]

On Wed, Aug 17, 2022 at 09:41:44PM +0000, Colton Lewis wrote:
> Create for each vcpu an array of random numbers to be used as a source
> of randomness when executing guest code. Randomness should be useful
> for approaching more realistic conditions in dirty_log_perf_test.
> 
> Create a -r argument to specify a random seed. If no argument
> is provided, the seed defaults to the current Unix timestamp.
> 
> The random arrays are allocated and filled as part of VM creation. The
> additional memory used is one 32-bit number per guest VM page to
> ensure one number for each iteration of the inner loop of guest_code.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/dirty_log_perf_test.c       | 11 ++++-
>  .../selftests/kvm/include/perf_test_util.h    |  3 ++
>  .../selftests/kvm/lib/perf_test_util.c        | 45 ++++++++++++++++++-
>  3 files changed, 55 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index f99e39a672d3..362b946019e9 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -132,6 +132,7 @@ struct test_params {
>  	bool partition_vcpu_memory_access;
>  	enum vm_mem_backing_src_type backing_src;
>  	int slots;
> +	uint32_t random_seed;
>  };
>  
>  static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
> @@ -225,6 +226,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  				 p->slots, p->backing_src,
>  				 p->partition_vcpu_memory_access);
>  
> +	perf_test_set_random_seed(vm, p->random_seed);
>  	perf_test_set_wr_fract(vm, p->wr_fract);
>  
>  	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
> @@ -352,7 +354,7 @@ static void help(char *name)
>  {
>  	puts("");
>  	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
> -	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-s mem type]"
> +	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
>  	       "[-x memslots]\n", name);
>  	puts("");
>  	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
> @@ -380,6 +382,7 @@ static void help(char *name)
>  	printf(" -v: specify the number of vCPUs to run.\n");
>  	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
>  	       "     them into a separate region of memory for each vCPU.\n");
> +	printf(" -r: specify the starting random seed.\n");
>  	backing_src_help("-s");
>  	printf(" -x: Split the memory region into this number of memslots.\n"
>  	       "     (default: 1)\n");
> @@ -396,6 +399,7 @@ int main(int argc, char *argv[])
>  		.partition_vcpu_memory_access = true,
>  		.backing_src = DEFAULT_VM_MEM_SRC,
>  		.slots = 1,
> +		.random_seed = time(NULL),
>  	};
>  	int opt;
>  
> @@ -406,7 +410,7 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:or:s:x:")) != -1) {
>  		switch (opt) {
>  		case 'e':
>  			/* 'e' is for evil. */
> @@ -442,6 +446,9 @@ int main(int argc, char *argv[])
>  		case 'o':
>  			p.partition_vcpu_memory_access = false;
>  			break;
> +		case 'r':
> +			p.random_seed = atoi(optarg);
> +			break;
>  		case 's':
>  			p.backing_src = parse_backing_src_type(optarg);
>  			break;
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index eaa88df0555a..9517956424fc 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -23,6 +23,7 @@ struct perf_test_vcpu_args {
>  	uint64_t gpa;
>  	uint64_t gva;
>  	uint64_t pages;
> +	vm_vaddr_t random_array;
>  
>  	/* Only used by the host userspace part of the vCPU thread */
>  	struct kvm_vcpu *vcpu;
> @@ -35,6 +36,7 @@ struct perf_test_args {
>  	uint64_t gpa;
>  	uint64_t size;
>  	uint64_t guest_page_size;
> +	uint32_t random_seed;
>  	int wr_fract;
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
> @@ -52,6 +54,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  void perf_test_destroy_vm(struct kvm_vm *vm);
>  
>  void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract);
> +void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
>  
>  void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
>  void perf_test_join_vcpu_threads(int vcpus);
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index 9618b37c66f7..8d85923acb4e 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -70,6 +70,35 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  	}
>  }
>  
> +void perf_test_alloc_random_arrays(uint32_t nr_vcpus, uint32_t nr_randoms)
> +{
> +	struct perf_test_args *pta = &perf_test_args;
> +
> +	for (uint32_t i = 0; i < nr_vcpus; i++) {
> +		pta->vcpu_args[i].random_array = vm_vaddr_alloc(
> +			pta->vm,
> +			nr_randoms * sizeof(uint32_t),
> +			KVM_UTIL_MIN_VADDR);
> +		pr_debug("Random row %d vaddr: %p.\n", i, (void *)pta->vcpu_args[i].random_array);
> +	}
> +}
> +
> +void perf_test_fill_random_arrays(uint32_t nr_vcpus, uint32_t nr_randoms)
> +{
> +	struct perf_test_args *pta = &perf_test_args;
> +	uint32_t *host_row;
> +
> +	for (uint32_t i = 0; i < nr_vcpus; i++) {
> +		host_row = addr_gva2hva(pta->vm, pta->vcpu_args[i].random_array);
> +
> +		for (uint32_t j = 0; j < nr_randoms; j++)
> +			host_row[j] = random();
> +
> +		pr_debug("New randoms row %d: %d, %d, %d...\n",
> +			 i, host_row[0], host_row[1], host_row[2]);
> +	}
> +}
> +
>  void perf_test_setup_vcpus(struct kvm_vm *vm, int nr_vcpus,
>  			   struct kvm_vcpu *vcpus[],
>  			   uint64_t vcpu_memory_bytes,
> @@ -120,8 +149,9 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
> -	/* By default vCPUs will write to memory. */
> -	pta->wr_fract = 1;
> +	/* Set perf_test_args defaults. */
> +	pta->wr_fract = 100;

This is not needed in this commit. It's changing the behavior at the
current commit, as it's now defaulting to 0% reads (using the current
check in guest_code()).

> +	pta->random_seed = time(NULL);
>  
>  	/*
>  	 * Snapshot the non-huge page size.  This is used by the guest code to
> @@ -211,6 +241,11 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  
>  	ucall_init(vm, NULL);
>  
> +	srandom(perf_test_args.random_seed);
> +	pr_info("Random seed: %d\n", perf_test_args.random_seed);
> +	perf_test_alloc_random_arrays(nr_vcpus, vcpu_memory_bytes >> vm->page_shift);
> +	perf_test_fill_random_arrays(nr_vcpus, vcpu_memory_bytes >> vm->page_shift);
> +
>  	/* Export the shared variables to the guest. */
>  	sync_global_to_guest(vm, perf_test_args);
>  
> @@ -229,6 +264,12 @@ void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract)
>  	sync_global_to_guest(vm, perf_test_args);
>  }
>  
> +void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> +{
> +	perf_test_args.random_seed = random_seed;
> +	sync_global_to_guest(vm, perf_test_args);
> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> -- 
> 2.37.1.595.g718a3a8f04-goog
> 

[PATCH v2 2/3] KVM: selftests: Randomize which pages are written vs read.

[Colton Lewis]

Randomize which tables are written vs read using the random number
arrays. Change the variable wr_fract and associated function calls to
write_percent that now operates as a percentage from 0 to 100 where X
means each page has an X% chance of being written. Change the -f
argument to -w to reflect the new variable semantics. Keep the same
default of 100 percent writes.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/access_tracking_perf_test.c |  2 +-
 .../selftests/kvm/dirty_log_perf_test.c       | 28 ++++++++++---------
 .../selftests/kvm/include/perf_test_util.h    |  4 +--
 .../selftests/kvm/lib/perf_test_util.c        |  9 +++---
 4 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index 1c2749b1481a..e87696b60e0f 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -272,7 +272,7 @@ static void run_iteration(struct kvm_vm *vm, int nr_vcpus, const char *descripti
 static void access_memory(struct kvm_vm *vm, int nr_vcpus,
 			  enum access_type access, const char *description)
 {
-	perf_test_set_wr_fract(vm, (access == ACCESS_READ) ? INT_MAX : 1);
+	perf_test_set_write_percent(vm, (access == ACCESS_READ) ? 0 : 100);
 	iteration_work = ITERATION_ACCESS_MEMORY;
 	run_iteration(vm, nr_vcpus, description);
 }
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 362b946019e9..9226eeea79bc 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -128,10 +128,10 @@ static void vcpu_worker(struct perf_test_vcpu_args *vcpu_args)
 struct test_params {
 	unsigned long iterations;
 	uint64_t phys_offset;
-	int wr_fract;
 	bool partition_vcpu_memory_access;
 	enum vm_mem_backing_src_type backing_src;
 	int slots;
+	uint32_t write_percent;
 	uint32_t random_seed;
 };
 
@@ -227,7 +227,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 				 p->partition_vcpu_memory_access);
 
 	perf_test_set_random_seed(vm, p->random_seed);
-	perf_test_set_wr_fract(vm, p->wr_fract);
+	perf_test_set_write_percent(vm, p->write_percent);
 
 	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
 	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
@@ -355,7 +355,7 @@ static void help(char *name)
 	puts("");
 	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
 	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
-	       "[-x memslots]\n", name);
+	       "[-x memslots] [-w percentage]\n", name);
 	puts("");
 	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
 	       TEST_HOST_LOOP_N);
@@ -375,10 +375,6 @@ static void help(char *name)
 	printf(" -b: specify the size of the memory region which should be\n"
 	       "     dirtied by each vCPU. e.g. 10M or 3G.\n"
 	       "     (default: 1G)\n");
-	printf(" -f: specify the fraction of pages which should be written to\n"
-	       "     as opposed to simply read, in the form\n"
-	       "     1/<fraction of pages to write>.\n"
-	       "     (default: 1 i.e. all pages are written to.)\n");
 	printf(" -v: specify the number of vCPUs to run.\n");
 	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
 	       "     them into a separate region of memory for each vCPU.\n");
@@ -386,6 +382,11 @@ static void help(char *name)
 	backing_src_help("-s");
 	printf(" -x: Split the memory region into this number of memslots.\n"
 	       "     (default: 1)\n");
+	printf(" -w: specify the percentage of pages which should be written to\n"
+	       "     as an integer from 0-100 inclusive. This is probabalistic,\n"
+	       "     so -w X means each page has an X%% chance of writing\n"
+	       "     and a (100-X)%% chance of reading.\n"
+	       "     (default: 100 i.e. all pages are written to.)\n");
 	puts("");
 	exit(0);
 }
@@ -395,10 +396,10 @@ int main(int argc, char *argv[])
 	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
 	struct test_params p = {
 		.iterations = TEST_HOST_LOOP_N,
-		.wr_fract = 1,
 		.partition_vcpu_memory_access = true,
 		.backing_src = DEFAULT_VM_MEM_SRC,
 		.slots = 1,
+		.write_percent = 100,
 		.random_seed = time(NULL),
 	};
 	int opt;
@@ -410,7 +411,7 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:or:s:x:")) != -1) {
+	while ((opt = getopt(argc, argv, "eghi:p:m:nb:v:or:s:x:w:")) != -1) {
 		switch (opt) {
 		case 'e':
 			/* 'e' is for evil. */
@@ -433,10 +434,11 @@ int main(int argc, char *argv[])
 		case 'b':
 			guest_percpu_mem_size = parse_size(optarg);
 			break;
-		case 'f':
-			p.wr_fract = atoi(optarg);
-			TEST_ASSERT(p.wr_fract >= 1,
-				    "Write fraction cannot be less than one");
+		case 'w':
+			perf_test_args.write_percent = atoi(optarg);
+			TEST_ASSERT(perf_test_args.write_percent >= 0
+				    && perf_test_args.write_percent <= 100,
+				    "Write percentage must be between 0 and 100");
 			break;
 		case 'v':
 			nr_vcpus = atoi(optarg);
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index 9517956424fc..8da4a839c585 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -37,7 +37,7 @@ struct perf_test_args {
 	uint64_t size;
 	uint64_t guest_page_size;
 	uint32_t random_seed;
-	int wr_fract;
+	int write_percent;
 
 	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
 	bool nested;
@@ -53,7 +53,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 				   bool partition_vcpu_memory_access);
 void perf_test_destroy_vm(struct kvm_vm *vm);
 
-void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract);
+void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent);
 void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
 
 void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index 8d85923acb4e..1a6b69713337 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -48,6 +48,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
 	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_idx];
 	uint64_t gva;
 	uint64_t pages;
+	uint32_t *rnd_arr = (uint32_t *)vcpu_args->random_array;
 	int i;
 
 	gva = vcpu_args->gva;
@@ -60,7 +61,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
 		for (i = 0; i < pages; i++) {
 			uint64_t addr = gva + (i * pta->guest_page_size);
 
-			if (i % pta->wr_fract == 0)
+			if (rnd_arr[i] % 100 < pta->write_percent)
 				*(uint64_t *)addr = 0x0123456789ABCDEF;
 			else
 				READ_ONCE(*(uint64_t *)addr);
@@ -150,7 +151,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
 	/* Set perf_test_args defaults. */
-	pta->wr_fract = 100;
+	pta->write_percent = 100;
 	pta->random_seed = time(NULL);
 
 	/*
@@ -258,9 +259,9 @@ void perf_test_destroy_vm(struct kvm_vm *vm)
 	kvm_vm_free(vm);
 }
 
-void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract)
+void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent)
 {
-	perf_test_args.wr_fract = wr_fract;
+	perf_test_args.write_percent = write_percent;
 	sync_global_to_guest(vm, perf_test_args);
 }
 
-- 
2.37.1.595.g718a3a8f04-goog

Re: [PATCH v2 2/3] KVM: selftests: Randomize which pages are written vs read.

[David Matlack]

On Wed, Aug 17, 2022 at 09:41:45PM +0000, Colton Lewis wrote:
> Randomize which tables are written vs read using the random number
> arrays. Change the variable wr_fract and associated function calls to
> write_percent that now operates as a percentage from 0 to 100 where X
> means each page has an X% chance of being written. Change the -f
> argument to -w to reflect the new variable semantics. Keep the same
> default of 100 percent writes.

Doesn't the new option cause like a 1000x slowdown in "Dirty memory
time"?  I don't think we should merge this until that is understood and
addressed (and it should be at least called out here so that reviewers
can be made aware).

> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/access_tracking_perf_test.c |  2 +-
>  .../selftests/kvm/dirty_log_perf_test.c       | 28 ++++++++++---------
>  .../selftests/kvm/include/perf_test_util.h    |  4 +--
>  .../selftests/kvm/lib/perf_test_util.c        |  9 +++---
>  4 files changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> index 1c2749b1481a..e87696b60e0f 100644
> --- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
> +++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> @@ -272,7 +272,7 @@ static void run_iteration(struct kvm_vm *vm, int nr_vcpus, const char *descripti
>  static void access_memory(struct kvm_vm *vm, int nr_vcpus,
>  			  enum access_type access, const char *description)
>  {
> -	perf_test_set_wr_fract(vm, (access == ACCESS_READ) ? INT_MAX : 1);
> +	perf_test_set_write_percent(vm, (access == ACCESS_READ) ? 0 : 100);
>  	iteration_work = ITERATION_ACCESS_MEMORY;
>  	run_iteration(vm, nr_vcpus, description);
>  }
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 362b946019e9..9226eeea79bc 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -128,10 +128,10 @@ static void vcpu_worker(struct perf_test_vcpu_args *vcpu_args)
>  struct test_params {
>  	unsigned long iterations;
>  	uint64_t phys_offset;
> -	int wr_fract;
>  	bool partition_vcpu_memory_access;
>  	enum vm_mem_backing_src_type backing_src;
>  	int slots;
> +	uint32_t write_percent;
>  	uint32_t random_seed;
>  };
>  
> @@ -227,7 +227,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  				 p->partition_vcpu_memory_access);
>  
>  	perf_test_set_random_seed(vm, p->random_seed);
> -	perf_test_set_wr_fract(vm, p->wr_fract);
> +	perf_test_set_write_percent(vm, p->write_percent);
>  
>  	guest_num_pages = (nr_vcpus * guest_percpu_mem_size) >> vm->page_shift;
>  	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
> @@ -355,7 +355,7 @@ static void help(char *name)
>  	puts("");
>  	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
>  	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
> -	       "[-x memslots]\n", name);
> +	       "[-x memslots] [-w percentage]\n", name);
>  	puts("");
>  	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
>  	       TEST_HOST_LOOP_N);
> @@ -375,10 +375,6 @@ static void help(char *name)
>  	printf(" -b: specify the size of the memory region which should be\n"
>  	       "     dirtied by each vCPU. e.g. 10M or 3G.\n"
>  	       "     (default: 1G)\n");
> -	printf(" -f: specify the fraction of pages which should be written to\n"
> -	       "     as opposed to simply read, in the form\n"
> -	       "     1/<fraction of pages to write>.\n"
> -	       "     (default: 1 i.e. all pages are written to.)\n");
>  	printf(" -v: specify the number of vCPUs to run.\n");
>  	printf(" -o: Overlap guest memory accesses instead of partitioning\n"
>  	       "     them into a separate region of memory for each vCPU.\n");
> @@ -386,6 +382,11 @@ static void help(char *name)
>  	backing_src_help("-s");
>  	printf(" -x: Split the memory region into this number of memslots.\n"
>  	       "     (default: 1)\n");
> +	printf(" -w: specify the percentage of pages which should be written to\n"
> +	       "     as an integer from 0-100 inclusive. This is probabalistic,\n"
> +	       "     so -w X means each page has an X%% chance of writing\n"
> +	       "     and a (100-X)%% chance of reading.\n"
> +	       "     (default: 100 i.e. all pages are written to.)\n");
>  	puts("");
>  	exit(0);
>  }
> @@ -395,10 +396,10 @@ int main(int argc, char *argv[])
>  	int max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
>  	struct test_params p = {
>  		.iterations = TEST_HOST_LOOP_N,
> -		.wr_fract = 1,
>  		.partition_vcpu_memory_access = true,
>  		.backing_src = DEFAULT_VM_MEM_SRC,
>  		.slots = 1,
> +		.write_percent = 100,
>  		.random_seed = time(NULL),
>  	};
>  	int opt;
> @@ -410,7 +411,7 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:or:s:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "eghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
>  		case 'e':
>  			/* 'e' is for evil. */
> @@ -433,10 +434,11 @@ int main(int argc, char *argv[])
>  		case 'b':
>  			guest_percpu_mem_size = parse_size(optarg);
>  			break;
> -		case 'f':
> -			p.wr_fract = atoi(optarg);
> -			TEST_ASSERT(p.wr_fract >= 1,
> -				    "Write fraction cannot be less than one");
> +		case 'w':
> +			perf_test_args.write_percent = atoi(optarg);
> +			TEST_ASSERT(perf_test_args.write_percent >= 0
> +				    && perf_test_args.write_percent <= 100,
> +				    "Write percentage must be between 0 and 100");

perf_test_create_vm() overwrites this with 100. Did you mean
p.write_percent?

>  			break;
>  		case 'v':
>  			nr_vcpus = atoi(optarg);
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index 9517956424fc..8da4a839c585 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -37,7 +37,7 @@ struct perf_test_args {
>  	uint64_t size;
>  	uint64_t guest_page_size;
>  	uint32_t random_seed;
> -	int wr_fract;
> +	int write_percent;
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
>  	bool nested;
> @@ -53,7 +53,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  				   bool partition_vcpu_memory_access);
>  void perf_test_destroy_vm(struct kvm_vm *vm);
>  
> -void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract);
> +void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent);
>  void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
>  
>  void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index 8d85923acb4e..1a6b69713337 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -48,6 +48,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_idx];
>  	uint64_t gva;
>  	uint64_t pages;
> +	uint32_t *rnd_arr = (uint32_t *)vcpu_args->random_array;
>  	int i;
>  
>  	gva = vcpu_args->gva;
> @@ -60,7 +61,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  		for (i = 0; i < pages; i++) {
>  			uint64_t addr = gva + (i * pta->guest_page_size);
>  
> -			if (i % pta->wr_fract == 0)
> +			if (rnd_arr[i] % 100 < pta->write_percent)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;
>  			else
>  				READ_ONCE(*(uint64_t *)addr);
> @@ -150,7 +151,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
>  	/* Set perf_test_args defaults. */
> -	pta->wr_fract = 100;
> +	pta->write_percent = 100;
>  	pta->random_seed = time(NULL);
>  
>  	/*
> @@ -258,9 +259,9 @@ void perf_test_destroy_vm(struct kvm_vm *vm)
>  	kvm_vm_free(vm);
>  }
>  
> -void perf_test_set_wr_fract(struct kvm_vm *vm, int wr_fract)
> +void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent)
>  {
> -	perf_test_args.wr_fract = wr_fract;
> +	perf_test_args.write_percent = write_percent;
>  	sync_global_to_guest(vm, perf_test_args);
>  }
>  
> -- 
> 2.37.1.595.g718a3a8f04-goog
> 

Re: [PATCH v2 2/3] KVM: selftests: Randomize which pages are written vs read.

[Colton Lewis]

David Matlack <dmatlack@google.com> writes:

> On Wed, Aug 17, 2022 at 09:41:45PM +0000, Colton Lewis wrote:
>> Randomize which tables are written vs read using the random number
>> arrays. Change the variable wr_fract and associated function calls to
>> write_percent that now operates as a percentage from 0 to 100 where X
>> means each page has an X% chance of being written. Change the -f
>> argument to -w to reflect the new variable semantics. Keep the same
>> default of 100 percent writes.

> Doesn't the new option cause like a 1000x slowdown in "Dirty memory
> time"?  I don't think we should merge this until that is understood and
> addressed (and it should be at least called out here so that reviewers
> can be made aware).


I'm guessing you got that from my internally posted tests. This option
itself does not cause the slowdown. If this option is set to 0% or 100%
(the default), there is no slowdown at all. The slowdown I measured was
at 50%, probably because that makes branch prediction impossible because
it has an equal chance of doing a read or a write each time. This is a
good thing. It's much more realistic than predictably alternating read
and write.

I can see this would be worth mentioning.

>> @@ -433,10 +434,11 @@ int main(int argc, char *argv[])
>>   		case 'b':
>>   			guest_percpu_mem_size = parse_size(optarg);
>>   			break;
>> -		case 'f':
>> -			p.wr_fract = atoi(optarg);
>> -			TEST_ASSERT(p.wr_fract >= 1,
>> -				    "Write fraction cannot be less than one");
>> +		case 'w':
>> +			perf_test_args.write_percent = atoi(optarg);
>> +			TEST_ASSERT(perf_test_args.write_percent >= 0
>> +				    && perf_test_args.write_percent <= 100,
>> +				    "Write percentage must be between 0 and 100");

> perf_test_create_vm() overwrites this with 100. Did you mean
> p.write_percent?


I did.

Re: [PATCH v2 2/3] KVM: selftests: Randomize which pages are written vs read.

[David Matlack]

On Tue, Aug 30, 2022 at 07:02:10PM +0000, Colton Lewis wrote:
> David Matlack <dmatlack@google.com> writes:
> 
> > On Wed, Aug 17, 2022 at 09:41:45PM +0000, Colton Lewis wrote:
> > > Randomize which tables are written vs read using the random number
> > > arrays. Change the variable wr_fract and associated function calls to
> > > write_percent that now operates as a percentage from 0 to 100 where X
> > > means each page has an X% chance of being written. Change the -f
> > > argument to -w to reflect the new variable semantics. Keep the same
> > > default of 100 percent writes.
> 
> > Doesn't the new option cause like a 1000x slowdown in "Dirty memory
> > time"?  I don't think we should merge this until that is understood and
> > addressed (and it should be at least called out here so that reviewers
> > can be made aware).
> 
> 
> I'm guessing you got that from my internally posted tests. This option
> itself does not cause the slowdown. If this option is set to 0% or 100%
> (the default), there is no slowdown at all. The slowdown I measured was
> at 50%, probably because that makes branch prediction impossible because
> it has an equal chance of doing a read or a write each time. This is a
> good thing. It's much more realistic than predictably alternating read
> and write.

I found it hard to believe that branch prediction could affect
performance by 1000x (and why wouldn't random access order show the same
effect?) so I looked into it further.

The cause of the slowdown is actually MMU lock contention:

-   82.62%  [k] queued_spin_lock_slowpath
   - 82.09% queued_spin_lock_slowpath
      - 48.36% queued_write_lock_slowpath
         - _raw_write_lock
            - 22.18% kvm_mmu_notifier_invalidate_range_start
                 __mmu_notifier_invalidate_range_start
                 wp_page_copy
                 do_wp_page
                 __handle_mm_fault
                 handle_mm_fault
                 __get_user_pages
                 get_user_pages_unlocked
                 hva_to_pfn
                 __gfn_to_pfn_memslot
                 kvm_faultin_pfn
                 direct_page_fault
                 kvm_tdp_page_fault
                 kvm_mmu_page_fault
                 handle_ept_violation

I think the bug is due to the following:

 1. Randomized reads/writes were being applied to the Populate phase,
    which (when using anonymous memory) results in the guest memory being
    mapped to the Zero Page.
 2. The random access order changed across each iteration (Population
    phase included) which means that some pages were written to during each
    iteration for the first time. Those pages resulted in a copy-on-write
    in the host MM fault handler, which invokes the invalidate range
    notifier and acquires the MMU lock in write-mode.
 3. All vCPUs are doing this in parallel which results in a ton of lock
    contention.

Your internal test results also showed that performance got better
during each iteration. That's because more and more of the guest memory
has been faulted in during each iteration (less and less copy-on-write
faults that need to acquire the MMU lock in write-mode).

I see this issue in your v1 patchset but not v3 (I did not check v2).
But I think that's just because of the bug Ricardo pointed out
(write_percent is always 100 no matter what the user passes in on the
command line).

The proper fix for v4 would be to set write-percent to 100 during the
populate phase so all memory actually gets populated. Then only use the
provided write-percent for testing dirty logging.

Now you might wonder why the old version of the code didn't have this
problem despite also doing reads during the populate phase. That's
because the non-randomized version always read/wrote the same pages
during every pass. With your code every iteratoin reads/writes to a
random set of pages.

Re: [PATCH v2 2/3] KVM: selftests: Randomize which pages are written vs read.

[Colton Lewis]

David Matlack <dmatlack@google.com> writes:

> On Tue, Aug 30, 2022 at 07:02:10PM +0000, Colton Lewis wrote:
>> David Matlack <dmatlack@google.com> writes:

>> > On Wed, Aug 17, 2022 at 09:41:45PM +0000, Colton Lewis wrote:
>> > > Randomize which tables are written vs read using the random number
>> > > arrays. Change the variable wr_fract and associated function calls to
>> > > write_percent that now operates as a percentage from 0 to 100 where X
>> > > means each page has an X% chance of being written. Change the -f
>> > > argument to -w to reflect the new variable semantics. Keep the same
>> > > default of 100 percent writes.

>> > Doesn't the new option cause like a 1000x slowdown in "Dirty memory
>> > time"?  I don't think we should merge this until that is understood and
>> > addressed (and it should be at least called out here so that reviewers
>> > can be made aware).


>> I'm guessing you got that from my internally posted tests. This option
>> itself does not cause the slowdown. If this option is set to 0% or 100%
>> (the default), there is no slowdown at all. The slowdown I measured was
>> at 50%, probably because that makes branch prediction impossible because
>> it has an equal chance of doing a read or a write each time. This is a
>> good thing. It's much more realistic than predictably alternating read
>> and write.

> I found it hard to believe that branch prediction could affect
> performance by 1000x (and why wouldn't random access order show the same
> effect?) so I looked into it further.

> The cause of the slowdown is actually MMU lock contention:

> -   82.62%  [k] queued_spin_lock_slowpath
>     - 82.09% queued_spin_lock_slowpath
>        - 48.36% queued_write_lock_slowpath
>           - _raw_write_lock
>              - 22.18% kvm_mmu_notifier_invalidate_range_start
>                   __mmu_notifier_invalidate_range_start
>                   wp_page_copy
>                   do_wp_page
>                   __handle_mm_fault
>                   handle_mm_fault
>                   __get_user_pages
>                   get_user_pages_unlocked
>                   hva_to_pfn
>                   __gfn_to_pfn_memslot
>                   kvm_faultin_pfn
>                   direct_page_fault
>                   kvm_tdp_page_fault
>                   kvm_mmu_page_fault
>                   handle_ept_violation

> I think the bug is due to the following:

>   1. Randomized reads/writes were being applied to the Populate phase,
>      which (when using anonymous memory) results in the guest memory being
>      mapped to the Zero Page.
>   2. The random access order changed across each iteration (Population
>      phase included) which means that some pages were written to during  
> each
>      iteration for the first time. Those pages resulted in a copy-on-write
>      in the host MM fault handler, which invokes the invalidate range
>      notifier and acquires the MMU lock in write-mode.
>   3. All vCPUs are doing this in parallel which results in a ton of lock
>      contention.

> Your internal test results also showed that performance got better
> during each iteration. That's because more and more of the guest memory
> has been faulted in during each iteration (less and less copy-on-write
> faults that need to acquire the MMU lock in write-mode).


Thanks for the analysis David. I had wondered about the effects of
randomized reads/writes during the populate phase.

> The proper fix for v4 would be to set write-percent to 100 during the
> populate phase so all memory actually gets populated. Then only use the
> provided write-percent for testing dirty logging.


Will do that.

Re: [PATCH v2 2/3] KVM: selftests: Randomize which pages are written vs read.

[David Matlack]

On Thu, Sep 8, 2022 at 12:46 PM Colton Lewis <coltonlewis@google.com> wrote:
>
> David Matlack <dmatlack@google.com> writes:
>
> > On Tue, Aug 30, 2022 at 07:02:10PM +0000, Colton Lewis wrote:
> >> David Matlack <dmatlack@google.com> writes:
>
> >> > On Wed, Aug 17, 2022 at 09:41:45PM +0000, Colton Lewis wrote:
> >> > > Randomize which tables are written vs read using the random number
> >> > > arrays. Change the variable wr_fract and associated function calls to
> >> > > write_percent that now operates as a percentage from 0 to 100 where X
> >> > > means each page has an X% chance of being written. Change the -f
> >> > > argument to -w to reflect the new variable semantics. Keep the same
> >> > > default of 100 percent writes.
>
> >> > Doesn't the new option cause like a 1000x slowdown in "Dirty memory
> >> > time"?  I don't think we should merge this until that is understood and
> >> > addressed (and it should be at least called out here so that reviewers
> >> > can be made aware).
>
>
> >> I'm guessing you got that from my internally posted tests. This option
> >> itself does not cause the slowdown. If this option is set to 0% or 100%
> >> (the default), there is no slowdown at all. The slowdown I measured was
> >> at 50%, probably because that makes branch prediction impossible because
> >> it has an equal chance of doing a read or a write each time. This is a
> >> good thing. It's much more realistic than predictably alternating read
> >> and write.
>
> > I found it hard to believe that branch prediction could affect
> > performance by 1000x (and why wouldn't random access order show the same
> > effect?) so I looked into it further.
>
> > The cause of the slowdown is actually MMU lock contention:
>
> > -   82.62%  [k] queued_spin_lock_slowpath
> >     - 82.09% queued_spin_lock_slowpath
> >        - 48.36% queued_write_lock_slowpath
> >           - _raw_write_lock
> >              - 22.18% kvm_mmu_notifier_invalidate_range_start
> >                   __mmu_notifier_invalidate_range_start
> >                   wp_page_copy
> >                   do_wp_page
> >                   __handle_mm_fault
> >                   handle_mm_fault
> >                   __get_user_pages
> >                   get_user_pages_unlocked
> >                   hva_to_pfn
> >                   __gfn_to_pfn_memslot
> >                   kvm_faultin_pfn
> >                   direct_page_fault
> >                   kvm_tdp_page_fault
> >                   kvm_mmu_page_fault
> >                   handle_ept_violation
>
> > I think the bug is due to the following:
>
> >   1. Randomized reads/writes were being applied to the Populate phase,
> >      which (when using anonymous memory) results in the guest memory being
> >      mapped to the Zero Page.
> >   2. The random access order changed across each iteration (Population
> >      phase included) which means that some pages were written to during
> > each
> >      iteration for the first time. Those pages resulted in a copy-on-write
> >      in the host MM fault handler, which invokes the invalidate range
> >      notifier and acquires the MMU lock in write-mode.
> >   3. All vCPUs are doing this in parallel which results in a ton of lock
> >      contention.
>
> > Your internal test results also showed that performance got better
> > during each iteration. That's because more and more of the guest memory
> > has been faulted in during each iteration (less and less copy-on-write
> > faults that need to acquire the MMU lock in write-mode).
>
>
> Thanks for the analysis David. I had wondered about the effects of
> randomized reads/writes during the populate phase.
>
> > The proper fix for v4 would be to set write-percent to 100 during the
> > populate phase so all memory actually gets populated. Then only use the
> > provided write-percent for testing dirty logging.
>
>
> Will do that.

Sounds good, thanks. I do think that this is an interesting discovery
though that we wouldn't have known about without your random access
series. So I am somewhat tempted to say let's leave the behavior as
is. But doing that will end up conflating two different tests.
dirty_log_perf_test should really isolate the performance of dirty
logging independent of memory population.

It would be interesting though to create a generalized memory access
test that randomizes reads, writes and even execution (to exercise NX
Huge Pages) so that we do have a test that exercises the copy-on-write
behavior. But I think that falls outside the scope of your series.

[PATCH v2 3/3] KVM: selftests: Randomize page access order.

[Colton Lewis]

Create the ability to randomize page access order with the -a
argument, including the possibility that the same pages may be hit
multiple times during an iteration or not at all.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/dirty_log_perf_test.c   | 10 +++++++++-
 .../testing/selftests/kvm/include/perf_test_util.h  |  2 ++
 tools/testing/selftests/kvm/lib/perf_test_util.c    | 13 ++++++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 9226eeea79bc..af9754bda0a4 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -133,6 +133,7 @@ struct test_params {
 	int slots;
 	uint32_t write_percent;
 	uint32_t random_seed;
+	bool random_access;
 };
 
 static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
@@ -271,6 +272,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
 		ts_diff.tv_sec, ts_diff.tv_nsec);
 
+	/* Set random access here, after population phase. */
+	perf_test_set_random_access(vm, p->random_access);
+
 	while (iteration < p->iterations) {
 		/*
 		 * Incrementing the iteration number will start the vCPUs
@@ -353,10 +357,11 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 static void help(char *name)
 {
 	puts("");
-	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
+	printf("usage: %s [-h] [-a] [-i iterations] [-p offset] [-g] "
 	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
 	       "[-x memslots] [-w percentage]\n", name);
 	puts("");
+	printf(" -a: access memory randomly rather than in order.\n");
 	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
 	       TEST_HOST_LOOP_N);
 	printf(" -g: Do not enable KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2. This\n"
@@ -413,6 +418,9 @@ int main(int argc, char *argv[])
 
 	while ((opt = getopt(argc, argv, "eghi:p:m:nb:v:or:s:x:w:")) != -1) {
 		switch (opt) {
+		case 'a':
+			p.random_access = true;
+			break;
 		case 'e':
 			/* 'e' is for evil. */
 			run_vcpus_while_disabling_dirty_logging = true;
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index 8da4a839c585..237899f3f4fe 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -41,6 +41,7 @@ struct perf_test_args {
 
 	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
 	bool nested;
+	bool random_access;
 
 	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
 };
@@ -55,6 +56,7 @@ void perf_test_destroy_vm(struct kvm_vm *vm);
 
 void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent);
 void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
+void perf_test_set_random_access(struct kvm_vm *vm, bool random_access);
 
 void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
 void perf_test_join_vcpu_threads(int vcpus);
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index 1a6b69713337..84e442a028c0 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -48,6 +48,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
 	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_idx];
 	uint64_t gva;
 	uint64_t pages;
+	uint64_t addr;
 	uint32_t *rnd_arr = (uint32_t *)vcpu_args->random_array;
 	int i;
 
@@ -59,7 +60,11 @@ void perf_test_guest_code(uint32_t vcpu_idx)
 
 	while (true) {
 		for (i = 0; i < pages; i++) {
-			uint64_t addr = gva + (i * pta->guest_page_size);
+			if (pta->random_access)
+				addr = gva +
+					((rnd_arr[i] % pages) * pta->guest_page_size);
+			else
+				addr = gva + (i * pta->guest_page_size);
 
 			if (rnd_arr[i] % 100 < pta->write_percent)
 				*(uint64_t *)addr = 0x0123456789ABCDEF;
@@ -271,6 +276,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
 	sync_global_to_guest(vm, perf_test_args);
 }
 
+void perf_test_set_random_access(struct kvm_vm *vm, bool random_access)
+{
+	perf_test_args.random_access = random_access;
+	sync_global_to_guest(vm, perf_test_args);
+}
+
 uint64_t __weak perf_test_nested_pages(int nr_vcpus)
 {
 	return 0;
-- 
2.37.1.595.g718a3a8f04-goog

Re: [PATCH v2 3/3] KVM: selftests: Randomize page access order.

[Colton Lewis]

On Wed, Aug 17, 2022 at 09:41:46PM +0000, Colton Lewis wrote:
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 9226eeea79bc..af9754bda0a4 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
>  	while ((opt = getopt(argc, argv, "eghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
> +		case 'a':
> +			p.random_access = true;
> +			break;
>  		case 'e':
>  			/* 'e' is for evil. */
>  			run_vcpus_while_disabling_dirty_logging = true;

Was double checking this and forgot to add an "a" to the getopt
string. This is small enough that I will wait for the rest to get more
eyes before rerolling the patch.

Re: [PATCH v2 3/3] KVM: selftests: Randomize page access order.

[David Matlack]

On Wed, Aug 17, 2022 at 09:41:46PM +0000, Colton Lewis wrote:
> Create the ability to randomize page access order with the -a
> argument, including the possibility that the same pages may be hit
> multiple times during an iteration or not at all.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c   | 10 +++++++++-
>  .../testing/selftests/kvm/include/perf_test_util.h  |  2 ++
>  tools/testing/selftests/kvm/lib/perf_test_util.c    | 13 ++++++++++++-
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 9226eeea79bc..af9754bda0a4 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -133,6 +133,7 @@ struct test_params {
>  	int slots;
>  	uint32_t write_percent;
>  	uint32_t random_seed;
> +	bool random_access;
>  };
>  
>  static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
> @@ -271,6 +272,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
>  		ts_diff.tv_sec, ts_diff.tv_nsec);
>  
> +	/* Set random access here, after population phase. */
> +	perf_test_set_random_access(vm, p->random_access);

Optional suggestion: We don't have any use-case for disabling random
access, so perhaps this would be simpler as:

  if (p->random_access)
          perf_test_enable_random_access(vm);

And then:

  void perf_test_enable_random_access(struct kvm_vm *vm)
  {
          perf_test_args.random_access = true;
          sync_global_to_guest(vm, perf_test_args);
  }

> +
>  	while (iteration < p->iterations) {
>  		/*
>  		 * Incrementing the iteration number will start the vCPUs
> @@ -353,10 +357,11 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  static void help(char *name)
>  {
>  	puts("");
> -	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
> +	printf("usage: %s [-h] [-a] [-i iterations] [-p offset] [-g] "
>  	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
>  	       "[-x memslots] [-w percentage]\n", name);
>  	puts("");
> +	printf(" -a: access memory randomly rather than in order.\n");
>  	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
>  	       TEST_HOST_LOOP_N);
>  	printf(" -g: Do not enable KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2. This\n"
> @@ -413,6 +418,9 @@ int main(int argc, char *argv[])
>  
>  	while ((opt = getopt(argc, argv, "eghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
> +		case 'a':
> +			p.random_access = true;
> +			break;
>  		case 'e':
>  			/* 'e' is for evil. */
>  			run_vcpus_while_disabling_dirty_logging = true;
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index 8da4a839c585..237899f3f4fe 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -41,6 +41,7 @@ struct perf_test_args {
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
>  	bool nested;
> +	bool random_access;
>  
>  	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
>  };
> @@ -55,6 +56,7 @@ void perf_test_destroy_vm(struct kvm_vm *vm);
>  
>  void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent);
>  void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
> +void perf_test_set_random_access(struct kvm_vm *vm, bool random_access);
>  
>  void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
>  void perf_test_join_vcpu_threads(int vcpus);
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index 1a6b69713337..84e442a028c0 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -48,6 +48,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_idx];
>  	uint64_t gva;
>  	uint64_t pages;
> +	uint64_t addr;
>  	uint32_t *rnd_arr = (uint32_t *)vcpu_args->random_array;
>  	int i;
>  
> @@ -59,7 +60,11 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  
>  	while (true) {
>  		for (i = 0; i < pages; i++) {
> -			uint64_t addr = gva + (i * pta->guest_page_size);
> +			if (pta->random_access)
> +				addr = gva +
> +					((rnd_arr[i] % pages) * pta->guest_page_size);
> +			else
> +				addr = gva + (i * pta->guest_page_size);
>  
>  			if (rnd_arr[i] % 100 < pta->write_percent)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;
> @@ -271,6 +276,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
>  	sync_global_to_guest(vm, perf_test_args);
>  }
>  
> +void perf_test_set_random_access(struct kvm_vm *vm, bool random_access)
> +{
> +	perf_test_args.random_access = random_access;
> +	sync_global_to_guest(vm, perf_test_args);
> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> -- 
> 2.37.1.595.g718a3a8f04-goog
> 

Re: [PATCH v2 3/3] KVM: selftests: Randomize page access order.

[Colton Lewis]

David Matlack <dmatlack@google.com> writes:

> On Wed, Aug 17, 2022 at 09:41:46PM +0000, Colton Lewis wrote:
>> @@ -271,6 +272,9 @@ static void run_test(enum vm_guest_mode mode, void  
>> *arg)
>>   	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
>>   		ts_diff.tv_sec, ts_diff.tv_nsec);

>> +	/* Set random access here, after population phase. */
>> +	perf_test_set_random_access(vm, p->random_access);

> Optional suggestion: We don't have any use-case for disabling random
> access, so perhaps this would be simpler as:

>    if (p->random_access)
>            perf_test_enable_random_access(vm);

> And then:

>    void perf_test_enable_random_access(struct kvm_vm *vm)
>    {
>            perf_test_args.random_access = true;
>            sync_global_to_guest(vm, perf_test_args);
>    }


I don't think it's simpler and it's less flexible.

Re: [PATCH v2 3/3] KVM: selftests: Randomize page access order.

[Ricardo Koller]

Hey Colton,

On Wed, Aug 17, 2022 at 09:41:46PM +0000, Colton Lewis wrote:
> Create the ability to randomize page access order with the -a
> argument, including the possibility that the same pages may be hit
> multiple times during an iteration or not at all.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c   | 10 +++++++++-
>  .../testing/selftests/kvm/include/perf_test_util.h  |  2 ++
>  tools/testing/selftests/kvm/lib/perf_test_util.c    | 13 ++++++++++++-
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 9226eeea79bc..af9754bda0a4 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -133,6 +133,7 @@ struct test_params {
>  	int slots;
>  	uint32_t write_percent;
>  	uint32_t random_seed;
> +	bool random_access;
>  };
>  
>  static void toggle_dirty_logging(struct kvm_vm *vm, int slots, bool enable)
> @@ -271,6 +272,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  	pr_info("Enabling dirty logging time: %ld.%.9lds\n\n",
>  		ts_diff.tv_sec, ts_diff.tv_nsec);
>  
> +	/* Set random access here, after population phase. */
> +	perf_test_set_random_access(vm, p->random_access);
> +
>  	while (iteration < p->iterations) {
>  		/*
>  		 * Incrementing the iteration number will start the vCPUs
> @@ -353,10 +357,11 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  static void help(char *name)
>  {
>  	puts("");
> -	printf("usage: %s [-h] [-i iterations] [-p offset] [-g] "
> +	printf("usage: %s [-h] [-a] [-i iterations] [-p offset] [-g] "
>  	       "[-m mode] [-n] [-b vcpu bytes] [-v vcpus] [-o] [-r random seed ] [-s mem type]"
>  	       "[-x memslots] [-w percentage]\n", name);
>  	puts("");
> +	printf(" -a: access memory randomly rather than in order.\n");
>  	printf(" -i: specify iteration counts (default: %"PRIu64")\n",
>  	       TEST_HOST_LOOP_N);
>  	printf(" -g: Do not enable KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2. This\n"
> @@ -413,6 +418,9 @@ int main(int argc, char *argv[])
>  
>  	while ((opt = getopt(argc, argv, "eghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
> +		case 'a':
> +			p.random_access = true;
> +			break;
>  		case 'e':
>  			/* 'e' is for evil. */
>  			run_vcpus_while_disabling_dirty_logging = true;
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index 8da4a839c585..237899f3f4fe 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -41,6 +41,7 @@ struct perf_test_args {
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
>  	bool nested;
> +	bool random_access;
>  
>  	struct perf_test_vcpu_args vcpu_args[KVM_MAX_VCPUS];
>  };
> @@ -55,6 +56,7 @@ void perf_test_destroy_vm(struct kvm_vm *vm);
>  
>  void perf_test_set_write_percent(struct kvm_vm *vm, uint32_t write_percent);
>  void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed);
> +void perf_test_set_random_access(struct kvm_vm *vm, bool random_access);
>  
>  void perf_test_start_vcpu_threads(int vcpus, void (*vcpu_fn)(struct perf_test_vcpu_args *));
>  void perf_test_join_vcpu_threads(int vcpus);
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index 1a6b69713337..84e442a028c0 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -48,6 +48,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_idx];
>  	uint64_t gva;
>  	uint64_t pages;
> +	uint64_t addr;
>  	uint32_t *rnd_arr = (uint32_t *)vcpu_args->random_array;
>  	int i;
>  
> @@ -59,7 +60,11 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  
>  	while (true) {
>  		for (i = 0; i < pages; i++) {
> -			uint64_t addr = gva + (i * pta->guest_page_size);
> +			if (pta->random_access)
> +				addr = gva +
> +					((rnd_arr[i] % pages) * pta->guest_page_size);
> +			else
> +				addr = gva + (i * pta->guest_page_size);
>  
>  			if (rnd_arr[i] % 100 < pta->write_percent)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;

I think addr and write_percent need two different random numbers.
Otherwise, you will end up with a situation where all addresses where
(rnd_arr[i] % 100 < pta->write_percent) will get a write (always).
Something like this:

	012345678    <= address
	wwwrrrwww
	837561249    <= access order

I think the best way to fix this is to abstract the random number
reading into something like get_next_rand(), and use it twice per
iteration.

> @@ -271,6 +276,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
>  	sync_global_to_guest(vm, perf_test_args);
>  }
>  
> +void perf_test_set_random_access(struct kvm_vm *vm, bool random_access)
> +{
> +	perf_test_args.random_access = random_access;
> +	sync_global_to_guest(vm, perf_test_args);
> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> -- 
> 2.37.1.595.g718a3a8f04-goog
> 

Thanks,
Ricardo