kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] KVM: selftests: randomize memory access of dirty_log_perf_test
@ 2022-09-09 12:42 Colton Lewis
  2022-09-09 12:42 ` [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code Colton Lewis
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-09 12:42 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, maz, dmatlack, seanjc, oupton, ricarkol, andrew.jones,
	Colton Lewis

Add the ability to randomize parts of dirty_log_perf_test,
specifically the order pages are accessed and whether pages are read
or written.

v5:

Return a few lines in guest_code that got lost when rebasing for v4.

Provide the populate settings _before_ the vcpu threads start to
ensure they take effect for the whole populate phase.

Colton Lewis (3):
  KVM: selftests: implement random number generation 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       | 49 +++++++++++++------
 .../selftests/kvm/include/perf_test_util.h    |  8 ++-
 .../testing/selftests/kvm/include/test_util.h |  2 +
 .../selftests/kvm/lib/perf_test_util.c        | 36 +++++++++++---
 tools/testing/selftests/kvm/lib/test_util.c   |  9 ++++
 6 files changed, 81 insertions(+), 25 deletions(-)

--
2.37.2.789.g6183377224-goog

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code
  2022-09-09 12:42 [PATCH v5 0/3] KVM: selftests: randomize memory access of dirty_log_perf_test Colton Lewis
@ 2022-09-09 12:42 ` Colton Lewis
  2022-09-09 16:22   ` Ricardo Koller
  2022-09-09 17:08   ` David Matlack
  2022-09-09 12:42 ` [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read Colton Lewis
  2022-09-09 12:43 ` [PATCH v5 3/3] KVM: selftests: randomize page access order Colton Lewis
  2 siblings, 2 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-09 12:42 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, maz, dmatlack, seanjc, oupton, ricarkol, andrew.jones,
	Colton Lewis

Implement random number generation for guest code to randomize parts
of the test, making it less predictable and a more accurate reflection
of reality.

Create a -r argument to specify a random seed. If no argument is
provided, the seed defaults to the current Unix timestamp. The random
seed is set with perf_test_set_random_seed() and must be set before
guest_code runs to apply.

The random number generator chosen is the Park-Miller Linear
Congruential Generator, a fancy name for a basic and well-understood
random number generator entirely sufficient for this purpose. Each
vCPU calculates its own seed by adding its index to the seed provided.

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

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index d60a34cdfaee..2f91acd94130 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -126,6 +126,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)
@@ -220,6 +221,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 				 p->slots, p->backing_src,
 				 p->partition_vcpu_memory_access);
 
+	pr_info("Random seed: %u\n", p->random_seed);
+	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_get_page_shift(vm);
@@ -337,7 +340,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",
@@ -362,6 +365,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");
@@ -378,6 +382,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;
 
@@ -388,7 +393,7 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:os:x:")) != -1) {
+	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:or:s:x:")) != -1) {
 		switch (opt) {
 		case 'g':
 			dirty_log_manual_caps = 0;
@@ -421,6 +426,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 d822cb670f1c..f18530984b42 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -34,6 +34,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. */
@@ -51,6 +52,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int 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/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index 99e0dcdc923f..2dd286bcf46f 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -143,4 +143,6 @@ static inline void *align_ptr_up(void *x, size_t size)
 	return (void *)align_up((unsigned long)x, size);
 }
 
+void guest_random(uint32_t *seed);
+
 #endif /* SELFTEST_KVM_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index f989ff91f022..4d9c7d7693d9 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -47,6 +47,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
 	uint64_t gva;
 	uint64_t pages;
 	int i;
+	uint32_t rand = pta->random_seed + vcpu_id;
 
 	/* Make sure vCPU args data structure is not corrupt. */
 	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
@@ -57,6 +58,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
 	while (true) {
 		for (i = 0; i < pages; i++) {
 			uint64_t addr = gva + (i * pta->guest_page_size);
+			guest_random(&rand);
 
 			if (i % pta->wr_fract == 0)
 				*(uint64_t *)addr = 0x0123456789ABCDEF;
@@ -115,8 +117,9 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
 
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
-	/* By default vCPUs will write to memory. */
+	/* Set perf_test_args defaults. */
 	pta->wr_fract = 1;
+	pta->random_seed = time(NULL);
 
 	/*
 	 * Snapshot the non-huge page size.  This is used by the guest code to
@@ -224,6 +227,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.random_seed);
+}
+
 uint64_t __weak perf_test_nested_pages(int nr_vcpus)
 {
 	return 0;
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 6d23878bbfe1..28c895743abe 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -17,6 +17,15 @@
 
 #include "test_util.h"
 
+/*
+ * Random number generator that is usable from guest code. This is the
+ * Park-Miller LCG using standard constants.
+ */
+void guest_random(uint32_t *seed)
+{
+	*seed = (uint64_t)*seed * 48271 % ((uint32_t)(1 << 31) - 1);
+}
+
 /*
  * Parses "[0-9]+[kmgt]?".
  */
-- 
2.37.2.789.g6183377224-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read
  2022-09-09 12:42 [PATCH v5 0/3] KVM: selftests: randomize memory access of dirty_log_perf_test Colton Lewis
  2022-09-09 12:42 ` [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code Colton Lewis
@ 2022-09-09 12:42 ` Colton Lewis
  2022-09-09 17:07   ` Ricardo Koller
  2022-09-09 17:21   ` David Matlack
  2022-09-09 12:43 ` [PATCH v5 3/3] KVM: selftests: randomize page access order Colton Lewis
  2 siblings, 2 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-09 12:42 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, maz, dmatlack, seanjc, oupton, ricarkol, andrew.jones,
	Colton Lewis

Randomize which pages are written vs read using the random number
generator.

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% writes.

Population always uses 100% writes.

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

diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
index d8909032317a..d86046ef3a0b 100644
--- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
+++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
@@ -274,7 +274,7 @@ static void run_iteration(struct kvm_vm *vm, int vcpus, const char *description)
 static void access_memory(struct kvm_vm *vm, int 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, 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 2f91acd94130..c2ad299b3760 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -122,10 +122,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;
 };
 
@@ -223,7 +223,6 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 
 	pr_info("Random seed: %u\n", p->random_seed);
 	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_get_page_shift(vm);
 	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
@@ -248,6 +247,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
 		vcpu_last_completed_iteration[vcpu_id] = -1;
 
+	perf_test_set_write_percent(vm, 100);
 	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
 
 	/* Allow the vCPUs to populate memory */
@@ -269,6 +269,8 @@ 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);
 
+	perf_test_set_write_percent(vm, p->write_percent);
+
 	while (iteration < p->iterations) {
 		/*
 		 * Incrementing the iteration number will start the vCPUs
@@ -341,7 +343,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);
@@ -358,10 +360,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");
@@ -369,6 +367,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);
 }
@@ -378,10 +381,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;
@@ -393,7 +396,7 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:or:s:x:")) != -1) {
+	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
 		switch (opt) {
 		case 'g':
 			dirty_log_manual_caps = 0;
@@ -413,10 +416,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':
+			p.write_percent = atoi(optarg);
+			TEST_ASSERT(p.write_percent >= 0
+				    && p.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 f18530984b42..f93f2ea7c6a3 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -35,7 +35,7 @@ struct perf_test_args {
 	uint64_t size;
 	uint64_t guest_page_size;
 	uint32_t random_seed;
-	int wr_fract;
+	uint32_t write_percent;
 
 	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
 	bool nested;
@@ -51,7 +51,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int 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 4d9c7d7693d9..12a3597be1f9 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -60,7 +60,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
 			uint64_t addr = gva + (i * pta->guest_page_size);
 			guest_random(&rand);
 
-			if (i % pta->wr_fract == 0)
+			if (rand % 100 < pta->write_percent)
 				*(uint64_t *)addr = 0x0123456789ABCDEF;
 			else
 				READ_ONCE(*(uint64_t *)addr);
@@ -118,7 +118,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
 	/* Set perf_test_args defaults. */
-	pta->wr_fract = 1;
+	pta->write_percent = 100;
 	pta->random_seed = time(NULL);
 
 	/*
@@ -221,10 +221,10 @@ 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;
-	sync_global_to_guest(vm, perf_test_args);
+	perf_test_args.write_percent = write_percent;
+	sync_global_to_guest(vm, perf_test_args.write_percent);
 }
 
 void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
-- 
2.37.2.789.g6183377224-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 3/3] KVM: selftests: randomize page access order
  2022-09-09 12:42 [PATCH v5 0/3] KVM: selftests: randomize memory access of dirty_log_perf_test Colton Lewis
  2022-09-09 12:42 ` [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code Colton Lewis
  2022-09-09 12:42 ` [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read Colton Lewis
@ 2022-09-09 12:43 ` Colton Lewis
  2022-09-09 17:17   ` Ricardo Koller
  2022-09-09 17:26   ` David Matlack
  2 siblings, 2 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-09 12:43 UTC (permalink / raw)
  To: kvm
  Cc: pbonzini, maz, dmatlack, seanjc, oupton, ricarkol, andrew.jones,
	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.

Population sets random access to false.

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

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index c2ad299b3760..3639d5f95033 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -127,6 +127,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)
@@ -248,6 +249,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		vcpu_last_completed_iteration[vcpu_id] = -1;
 
 	perf_test_set_write_percent(vm, 100);
+	perf_test_set_random_access(vm, false);
 	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
 
 	/* Allow the vCPUs to populate memory */
@@ -270,6 +272,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
 		ts_diff.tv_sec, ts_diff.tv_nsec);
 
 	perf_test_set_write_percent(vm, p->write_percent);
+	perf_test_set_random_access(vm, p->random_access);
 
 	while (iteration < p->iterations) {
 		/*
@@ -341,10 +344,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"
@@ -396,8 +400,11 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
+	while ((opt = getopt(argc, argv, "aghi:p:m:nb:v:or:s:x:w:")) != -1) {
 		switch (opt) {
+		case 'a':
+			p.random_access = true;
+			break;
 		case 'g':
 			dirty_log_manual_caps = 0;
 			break;
diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
index f93f2ea7c6a3..d9664a31e01c 100644
--- a/tools/testing/selftests/kvm/include/perf_test_util.h
+++ b/tools/testing/selftests/kvm/include/perf_test_util.h
@@ -39,6 +39,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];
 };
@@ -53,6 +54,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 12a3597be1f9..ce657fa92f05 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -46,6 +46,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
 	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_id];
 	uint64_t gva;
 	uint64_t pages;
+	uint64_t addr;
 	int i;
 	uint32_t rand = pta->random_seed + vcpu_id;
 
@@ -57,7 +58,13 @@ void perf_test_guest_code(uint32_t vcpu_id)
 
 	while (true) {
 		for (i = 0; i < pages; i++) {
-			uint64_t addr = gva + (i * pta->guest_page_size);
+			guest_random(&rand);
+
+			if (pta->random_access)
+				addr = gva + ((rand % pages) * pta->guest_page_size);
+			else
+				addr = gva + (i * pta->guest_page_size);
+
 			guest_random(&rand);
 
 			if (rand % 100 < pta->write_percent)
@@ -233,6 +240,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
 	sync_global_to_guest(vm, perf_test_args.random_seed);
 }
 
+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.random_access);
+}
+
 uint64_t __weak perf_test_nested_pages(int nr_vcpus)
 {
 	return 0;
-- 
2.37.2.789.g6183377224-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code
  2022-09-09 12:42 ` [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code Colton Lewis
@ 2022-09-09 16:22   ` Ricardo Koller
  2022-09-09 17:08   ` David Matlack
  1 sibling, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2022-09-09 16:22 UTC (permalink / raw)
  To: Colton Lewis; +Cc: kvm, pbonzini, maz, dmatlack, seanjc, oupton, andrew.jones

On Fri, Sep 09, 2022 at 12:42:58PM +0000, Colton Lewis wrote:
> Implement random number generation for guest code to randomize parts
> of the test, making it less predictable and a more accurate reflection
> of reality.
> 
> Create a -r argument to specify a random seed. If no argument is
> provided, the seed defaults to the current Unix timestamp. The random
> seed is set with perf_test_set_random_seed() and must be set before
> guest_code runs to apply.
> 
> The random number generator chosen is the Park-Miller Linear
> Congruential Generator, a fancy name for a basic and well-understood
> random number generator entirely sufficient for this purpose. Each
> vCPU calculates its own seed by adding its index to the seed provided.
>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c    | 12 ++++++++++--
>  tools/testing/selftests/kvm/include/perf_test_util.h |  2 ++
>  tools/testing/selftests/kvm/include/test_util.h      |  2 ++
>  tools/testing/selftests/kvm/lib/perf_test_util.c     | 11 ++++++++++-
>  tools/testing/selftests/kvm/lib/test_util.c          |  9 +++++++++
>  5 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index d60a34cdfaee..2f91acd94130 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -126,6 +126,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)
> @@ -220,6 +221,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  				 p->slots, p->backing_src,
>  				 p->partition_vcpu_memory_access);
>  
> +	pr_info("Random seed: %u\n", p->random_seed);
> +	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_get_page_shift(vm);
> @@ -337,7 +340,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",
> @@ -362,6 +365,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");
> @@ -378,6 +382,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;
>  
> @@ -388,7 +393,7 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:os:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:or:s:x:")) != -1) {
>  		switch (opt) {
>  		case 'g':
>  			dirty_log_manual_caps = 0;
> @@ -421,6 +426,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 d822cb670f1c..f18530984b42 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -34,6 +34,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. */
> @@ -51,6 +52,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int 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/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index 99e0dcdc923f..2dd286bcf46f 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -143,4 +143,6 @@ static inline void *align_ptr_up(void *x, size_t size)
>  	return (void *)align_up((unsigned long)x, size);
>  }
>  
> +void guest_random(uint32_t *seed);
> +
>  #endif /* SELFTEST_KVM_TEST_UTIL_H */
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index f989ff91f022..4d9c7d7693d9 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -47,6 +47,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  	uint64_t gva;
>  	uint64_t pages;
>  	int i;
> +	uint32_t rand = pta->random_seed + vcpu_id;
>  
>  	/* Make sure vCPU args data structure is not corrupt. */
>  	GUEST_ASSERT(vcpu_args->vcpu_id == vcpu_id);
> @@ -57,6 +58,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  	while (true) {
>  		for (i = 0; i < pages; i++) {
>  			uint64_t addr = gva + (i * pta->guest_page_size);
> +			guest_random(&rand);
>  
>  			if (i % pta->wr_fract == 0)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;
> @@ -115,8 +117,9 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
>  
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
> -	/* By default vCPUs will write to memory. */
> +	/* Set perf_test_args defaults. */
>  	pta->wr_fract = 1;
> +	pta->random_seed = time(NULL);
>  
>  	/*
>  	 * Snapshot the non-huge page size.  This is used by the guest code to
> @@ -224,6 +227,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.random_seed);
> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index 6d23878bbfe1..28c895743abe 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -17,6 +17,15 @@
>  
>  #include "test_util.h"
>  
> +/*
> + * Random number generator that is usable from guest code. This is the
> + * Park-Miller LCG using standard constants.
> + */
> +void guest_random(uint32_t *seed)
> +{
> +	*seed = (uint64_t)*seed * 48271 % ((uint32_t)(1 << 31) - 1);
> +}
> +
>  /*
>   * Parses "[0-9]+[kmgt]?".
>   */
> -- 
> 2.37.2.789.g6183377224-goog
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read
  2022-09-09 12:42 ` [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read Colton Lewis
@ 2022-09-09 17:07   ` Ricardo Koller
  2022-09-09 17:21   ` David Matlack
  1 sibling, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2022-09-09 17:07 UTC (permalink / raw)
  To: Colton Lewis; +Cc: kvm, pbonzini, maz, dmatlack, seanjc, oupton, andrew.jones

On Fri, Sep 09, 2022 at 12:42:59PM +0000, Colton Lewis wrote:
> Randomize which pages are written vs read using the random number
> generator.
> 
> 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% writes.
> 
> Population always uses 100% writes.
>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/access_tracking_perf_test.c |  2 +-
>  .../selftests/kvm/dirty_log_perf_test.c       | 30 +++++++++++--------
>  .../selftests/kvm/include/perf_test_util.h    |  4 +--
>  .../selftests/kvm/lib/perf_test_util.c        | 10 +++----
>  4 files changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> index d8909032317a..d86046ef3a0b 100644
> --- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
> +++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> @@ -274,7 +274,7 @@ static void run_iteration(struct kvm_vm *vm, int vcpus, const char *description)
>  static void access_memory(struct kvm_vm *vm, int 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, 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 2f91acd94130..c2ad299b3760 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -122,10 +122,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;
>  };
>  
> @@ -223,7 +223,6 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  
>  	pr_info("Random seed: %u\n", p->random_seed);
>  	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_get_page_shift(vm);
>  	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
> @@ -248,6 +247,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
>  		vcpu_last_completed_iteration[vcpu_id] = -1;
>  
> +	perf_test_set_write_percent(vm, 100);
>  	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
>  
>  	/* Allow the vCPUs to populate memory */
> @@ -269,6 +269,8 @@ 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);
>  
> +	perf_test_set_write_percent(vm, p->write_percent);
> +
>  	while (iteration < p->iterations) {
>  		/*
>  		 * Incrementing the iteration number will start the vCPUs
> @@ -341,7 +343,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);
> @@ -358,10 +360,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");
> @@ -369,6 +367,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);
>  }
> @@ -378,10 +381,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;
> @@ -393,7 +396,7 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:or:s:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
>  		case 'g':
>  			dirty_log_manual_caps = 0;
> @@ -413,10 +416,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':
> +			p.write_percent = atoi(optarg);
> +			TEST_ASSERT(p.write_percent >= 0
> +				    && p.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 f18530984b42..f93f2ea7c6a3 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -35,7 +35,7 @@ struct perf_test_args {
>  	uint64_t size;
>  	uint64_t guest_page_size;
>  	uint32_t random_seed;
> -	int wr_fract;
> +	uint32_t write_percent;
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
>  	bool nested;
> @@ -51,7 +51,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int 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 4d9c7d7693d9..12a3597be1f9 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -60,7 +60,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  			uint64_t addr = gva + (i * pta->guest_page_size);
>  			guest_random(&rand);
>  
> -			if (i % pta->wr_fract == 0)
> +			if (rand % 100 < pta->write_percent)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;
>  			else
>  				READ_ONCE(*(uint64_t *)addr);
> @@ -118,7 +118,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
>  	/* Set perf_test_args defaults. */
> -	pta->wr_fract = 1;
> +	pta->write_percent = 100;
>  	pta->random_seed = time(NULL);
>  
>  	/*
> @@ -221,10 +221,10 @@ 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;
> -	sync_global_to_guest(vm, perf_test_args);
> +	perf_test_args.write_percent = write_percent;
> +	sync_global_to_guest(vm, perf_test_args.write_percent);
>  }
>  
>  void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> -- 
> 2.37.2.789.g6183377224-goog
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code
  2022-09-09 12:42 ` [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code Colton Lewis
  2022-09-09 16:22   ` Ricardo Koller
@ 2022-09-09 17:08   ` David Matlack
  2022-09-12 19:13     ` Colton Lewis
  1 sibling, 1 reply; 15+ messages in thread
From: David Matlack @ 2022-09-09 17:08 UTC (permalink / raw)
  To: Colton Lewis; +Cc: kvm, pbonzini, maz, seanjc, oupton, ricarkol, andrew.jones

On Fri, Sep 09, 2022 at 12:42:58PM +0000, Colton Lewis wrote:
> Implement random number generation for guest code to randomize parts
> of the test, making it less predictable and a more accurate reflection
> of reality.
> 
> Create a -r argument to specify a random seed. If no argument is
> provided, the seed defaults to the current Unix timestamp. The random
> seed is set with perf_test_set_random_seed() and must be set before
> guest_code runs to apply.
> 
> The random number generator chosen is the Park-Miller Linear
> Congruential Generator, a fancy name for a basic and well-understood
> random number generator entirely sufficient for this purpose. Each
> vCPU calculates its own seed by adding its index to the seed provided.

Great commit message!

> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c    | 12 ++++++++++--
>  tools/testing/selftests/kvm/include/perf_test_util.h |  2 ++
>  tools/testing/selftests/kvm/include/test_util.h      |  2 ++
>  tools/testing/selftests/kvm/lib/perf_test_util.c     | 11 ++++++++++-
>  tools/testing/selftests/kvm/lib/test_util.c          |  9 +++++++++
>  5 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index d60a34cdfaee..2f91acd94130 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -126,6 +126,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)
> @@ -220,6 +221,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  				 p->slots, p->backing_src,
>  				 p->partition_vcpu_memory_access);
>  
> +	pr_info("Random seed: %u\n", p->random_seed);
> +	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_get_page_shift(vm);
> @@ -337,7 +340,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",
> @@ -362,6 +365,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");
> @@ -378,6 +382,7 @@ int main(int argc, char *argv[])
>  		.partition_vcpu_memory_access = true,
>  		.backing_src = DEFAULT_VM_MEM_SRC,
>  		.slots = 1,
> +		.random_seed = time(NULL),

It's a bad code smell that the random seed gets default initialized to
time(NULL) twice (here and in perf_test_create_vm()).

I also still think it would be better if the default random seed was
consistent across runs. Most use-cases of dirty_log_perf_test is for A/B
testing, so consistency is key. For example, running dirty_log_perf_test
at every commit to find regressions, or running dirty_log_perf_test to
study the performance effects of some change. In other words, I think
most use-cases will want a consistent seed across runs, so the default
behavior should match that. Otherwise I forsee myself (and automated
tools) having to pass in -r to every test runs to get consistent,
comparable, behavior.

What do you think about killing 2 birds with one stone here and make the
default random_seed 0. That requires no initialization and ensures
consistent random behavior across runs.

And then optionally... I would even recommend dropping the -r parameter
until someone wants to run dirty_log_perf_test with different seeds.
That would simplify the code even more. I have a feeling there won't be
much interest in different seeds since, at the end of the day, it will
always be the same rough distribution of accesses. More interesting than
different seeds will be adding support for different types of access
patterns.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/3] KVM: selftests: randomize page access order
  2022-09-09 12:43 ` [PATCH v5 3/3] KVM: selftests: randomize page access order Colton Lewis
@ 2022-09-09 17:17   ` Ricardo Koller
  2022-09-09 17:26   ` David Matlack
  1 sibling, 0 replies; 15+ messages in thread
From: Ricardo Koller @ 2022-09-09 17:17 UTC (permalink / raw)
  To: Colton Lewis; +Cc: kvm, pbonzini, maz, dmatlack, seanjc, oupton, andrew.jones

On Fri, Sep 09, 2022 at 12:43:00PM +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.
> 
> Population sets random access to false.
>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c | 11 +++++++++--
>  .../selftests/kvm/include/perf_test_util.h        |  2 ++
>  tools/testing/selftests/kvm/lib/perf_test_util.c  | 15 ++++++++++++++-
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index c2ad299b3760..3639d5f95033 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -127,6 +127,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)
> @@ -248,6 +249,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  		vcpu_last_completed_iteration[vcpu_id] = -1;
>  
>  	perf_test_set_write_percent(vm, 100);
> +	perf_test_set_random_access(vm, false);
>  	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
>  
>  	/* Allow the vCPUs to populate memory */
> @@ -270,6 +272,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  		ts_diff.tv_sec, ts_diff.tv_nsec);
>  
>  	perf_test_set_write_percent(vm, p->write_percent);
> +	perf_test_set_random_access(vm, p->random_access);
>  
>  	while (iteration < p->iterations) {
>  		/*
> @@ -341,10 +344,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"
> @@ -396,8 +400,11 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
> +	while ((opt = getopt(argc, argv, "aghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
> +		case 'a':
> +			p.random_access = true;
> +			break;
>  		case 'g':
>  			dirty_log_manual_caps = 0;
>  			break;
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index f93f2ea7c6a3..d9664a31e01c 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -39,6 +39,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];
>  };
> @@ -53,6 +54,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 12a3597be1f9..ce657fa92f05 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -46,6 +46,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_id];
>  	uint64_t gva;
>  	uint64_t pages;
> +	uint64_t addr;
>  	int i;
>  	uint32_t rand = pta->random_seed + vcpu_id;
>  
> @@ -57,7 +58,13 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  
>  	while (true) {
>  		for (i = 0; i < pages; i++) {
> -			uint64_t addr = gva + (i * pta->guest_page_size);
> +			guest_random(&rand);
> +
> +			if (pta->random_access)
> +				addr = gva + ((rand % pages) * pta->guest_page_size);
> +			else
> +				addr = gva + (i * pta->guest_page_size);
> +
>  			guest_random(&rand);
>  
>  			if (rand % 100 < pta->write_percent)
> @@ -233,6 +240,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
>  	sync_global_to_guest(vm, perf_test_args.random_seed);
>  }
>  
> +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.random_access);
> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> -- 
> 2.37.2.789.g6183377224-goog
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read
  2022-09-09 12:42 ` [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read Colton Lewis
  2022-09-09 17:07   ` Ricardo Koller
@ 2022-09-09 17:21   ` David Matlack
  2022-09-12 19:14     ` Colton Lewis
  1 sibling, 1 reply; 15+ messages in thread
From: David Matlack @ 2022-09-09 17:21 UTC (permalink / raw)
  To: Colton Lewis; +Cc: kvm, pbonzini, maz, seanjc, oupton, ricarkol, andrew.jones

On Fri, Sep 09, 2022 at 12:42:59PM +0000, Colton Lewis wrote:
> Randomize which pages are written vs read using the random number
> generator.
> 
> 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% writes.
> 
> Population always uses 100% writes.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/access_tracking_perf_test.c |  2 +-
>  .../selftests/kvm/dirty_log_perf_test.c       | 30 +++++++++++--------
>  .../selftests/kvm/include/perf_test_util.h    |  4 +--
>  .../selftests/kvm/lib/perf_test_util.c        | 10 +++----
>  4 files changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/access_tracking_perf_test.c b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> index d8909032317a..d86046ef3a0b 100644
> --- a/tools/testing/selftests/kvm/access_tracking_perf_test.c
> +++ b/tools/testing/selftests/kvm/access_tracking_perf_test.c
> @@ -274,7 +274,7 @@ static void run_iteration(struct kvm_vm *vm, int vcpus, const char *description)
>  static void access_memory(struct kvm_vm *vm, int 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, 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 2f91acd94130..c2ad299b3760 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -122,10 +122,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;
>  };
>  
> @@ -223,7 +223,6 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  
>  	pr_info("Random seed: %u\n", p->random_seed);
>  	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_get_page_shift(vm);
>  	guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
> @@ -248,6 +247,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
>  		vcpu_last_completed_iteration[vcpu_id] = -1;
>  
> +	perf_test_set_write_percent(vm, 100);

This is a very important line of code and it's not very clear why it's
here to a random reader. Please a comment here so someone doesn't have
to go through the same confusion/debugging we went through to figure out
why this is necessary. e.g.

        /*
         * Use 100% writes during the population phase to ensure all
         * memory is actually populated and not just mapped to the zero
         * page. The prevents expensive copy-on-write faults from
         * occurring during the dirty memory iterations below, which
         * would pollute the performance results.
         */
        perf_test_set_write_percent(vm, 100);

Aside from that,

Reviewed-by: David Matlack <dmatlack@google.com>

>  	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
>  
>  	/* Allow the vCPUs to populate memory */
> @@ -269,6 +269,8 @@ 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);
>  
> +	perf_test_set_write_percent(vm, p->write_percent);
> +
>  	while (iteration < p->iterations) {
>  		/*
>  		 * Incrementing the iteration number will start the vCPUs
> @@ -341,7 +343,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);
> @@ -358,10 +360,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");
> @@ -369,6 +367,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);
>  }
> @@ -378,10 +381,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;
> @@ -393,7 +396,7 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "ghi:p:m:nb:f:v:or:s:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
>  		case 'g':
>  			dirty_log_manual_caps = 0;
> @@ -413,10 +416,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':
> +			p.write_percent = atoi(optarg);
> +			TEST_ASSERT(p.write_percent >= 0
> +				    && p.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 f18530984b42..f93f2ea7c6a3 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -35,7 +35,7 @@ struct perf_test_args {
>  	uint64_t size;
>  	uint64_t guest_page_size;
>  	uint32_t random_seed;
> -	int wr_fract;
> +	uint32_t write_percent;
>  
>  	/* Run vCPUs in L2 instead of L1, if the architecture supports it. */
>  	bool nested;
> @@ -51,7 +51,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int 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 4d9c7d7693d9..12a3597be1f9 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -60,7 +60,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  			uint64_t addr = gva + (i * pta->guest_page_size);
>  			guest_random(&rand);
>  
> -			if (i % pta->wr_fract == 0)
> +			if (rand % 100 < pta->write_percent)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;
>  			else
>  				READ_ONCE(*(uint64_t *)addr);
> @@ -118,7 +118,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
>  	/* Set perf_test_args defaults. */
> -	pta->wr_fract = 1;
> +	pta->write_percent = 100;
>  	pta->random_seed = time(NULL);
>  
>  	/*
> @@ -221,10 +221,10 @@ 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;
> -	sync_global_to_guest(vm, perf_test_args);
> +	perf_test_args.write_percent = write_percent;
> +	sync_global_to_guest(vm, perf_test_args.write_percent);
>  }
>  
>  void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> -- 
> 2.37.2.789.g6183377224-goog
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/3] KVM: selftests: randomize page access order
  2022-09-09 12:43 ` [PATCH v5 3/3] KVM: selftests: randomize page access order Colton Lewis
  2022-09-09 17:17   ` Ricardo Koller
@ 2022-09-09 17:26   ` David Matlack
  2022-09-09 17:31     ` Ricardo Koller
  1 sibling, 1 reply; 15+ messages in thread
From: David Matlack @ 2022-09-09 17:26 UTC (permalink / raw)
  To: Colton Lewis; +Cc: kvm, pbonzini, maz, seanjc, oupton, ricarkol, andrew.jones

On Fri, Sep 09, 2022 at 12:43:00PM +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.
> 
> Population sets random access to false.

Please make sure to also explain the why in addition to the what.

> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c | 11 +++++++++--
>  .../selftests/kvm/include/perf_test_util.h        |  2 ++
>  tools/testing/selftests/kvm/lib/perf_test_util.c  | 15 ++++++++++++++-
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index c2ad299b3760..3639d5f95033 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -127,6 +127,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)
> @@ -248,6 +249,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  		vcpu_last_completed_iteration[vcpu_id] = -1;
>  
>  	perf_test_set_write_percent(vm, 100);
> +	perf_test_set_random_access(vm, false);
>  	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
>  
>  	/* Allow the vCPUs to populate memory */
> @@ -270,6 +272,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  		ts_diff.tv_sec, ts_diff.tv_nsec);
>  
>  	perf_test_set_write_percent(vm, p->write_percent);
> +	perf_test_set_random_access(vm, p->random_access);
>  
>  	while (iteration < p->iterations) {
>  		/*
> @@ -341,10 +344,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"
> @@ -396,8 +400,11 @@ int main(int argc, char *argv[])
>  
>  	guest_modes_append_default();
>  
> -	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
> +	while ((opt = getopt(argc, argv, "aghi:p:m:nb:v:or:s:x:w:")) != -1) {
>  		switch (opt) {
> +		case 'a':
> +			p.random_access = true;
> +			break;
>  		case 'g':
>  			dirty_log_manual_caps = 0;
>  			break;
> diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> index f93f2ea7c6a3..d9664a31e01c 100644
> --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> @@ -39,6 +39,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];
>  };
> @@ -53,6 +54,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 12a3597be1f9..ce657fa92f05 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -46,6 +46,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_id];
>  	uint64_t gva;
>  	uint64_t pages;
> +	uint64_t addr;
>  	int i;
>  	uint32_t rand = pta->random_seed + vcpu_id;
>  
> @@ -57,7 +58,13 @@ void perf_test_guest_code(uint32_t vcpu_id)
>  
>  	while (true) {
>  		for (i = 0; i < pages; i++) {
> -			uint64_t addr = gva + (i * pta->guest_page_size);
> +			guest_random(&rand);
> +
> +			if (pta->random_access)
> +				addr = gva + ((rand % pages) * pta->guest_page_size);
> +			else
> +				addr = gva + (i * pta->guest_page_size);
> +
>  			guest_random(&rand);

Is it on purpose use a separate random number for access offset and
read/write?

>  
>  			if (rand % 100 < pta->write_percent)
> @@ -233,6 +240,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
>  	sync_global_to_guest(vm, perf_test_args.random_seed);
>  }
>  
> +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.random_access);
> +}
> +
>  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
>  {
>  	return 0;
> -- 
> 2.37.2.789.g6183377224-goog
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/3] KVM: selftests: randomize page access order
  2022-09-09 17:26   ` David Matlack
@ 2022-09-09 17:31     ` Ricardo Koller
  2022-09-09 17:41       ` David Matlack
  0 siblings, 1 reply; 15+ messages in thread
From: Ricardo Koller @ 2022-09-09 17:31 UTC (permalink / raw)
  To: David Matlack
  Cc: Colton Lewis, kvm, pbonzini, maz, seanjc, oupton, andrew.jones

On Fri, Sep 09, 2022 at 10:26:10AM -0700, David Matlack wrote:
> On Fri, Sep 09, 2022 at 12:43:00PM +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.
> > 
> > Population sets random access to false.
> 
> Please make sure to also explain the why in addition to the what.
> 
> > 
> > Signed-off-by: Colton Lewis <coltonlewis@google.com>
> > ---
> >  tools/testing/selftests/kvm/dirty_log_perf_test.c | 11 +++++++++--
> >  .../selftests/kvm/include/perf_test_util.h        |  2 ++
> >  tools/testing/selftests/kvm/lib/perf_test_util.c  | 15 ++++++++++++++-
> >  3 files changed, 25 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > index c2ad299b3760..3639d5f95033 100644
> > --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > @@ -127,6 +127,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)
> > @@ -248,6 +249,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> >  		vcpu_last_completed_iteration[vcpu_id] = -1;
> >  
> >  	perf_test_set_write_percent(vm, 100);
> > +	perf_test_set_random_access(vm, false);
> >  	perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
> >  
> >  	/* Allow the vCPUs to populate memory */
> > @@ -270,6 +272,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> >  		ts_diff.tv_sec, ts_diff.tv_nsec);
> >  
> >  	perf_test_set_write_percent(vm, p->write_percent);
> > +	perf_test_set_random_access(vm, p->random_access);
> >  
> >  	while (iteration < p->iterations) {
> >  		/*
> > @@ -341,10 +344,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"
> > @@ -396,8 +400,11 @@ int main(int argc, char *argv[])
> >  
> >  	guest_modes_append_default();
> >  
> > -	while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
> > +	while ((opt = getopt(argc, argv, "aghi:p:m:nb:v:or:s:x:w:")) != -1) {
> >  		switch (opt) {
> > +		case 'a':
> > +			p.random_access = true;
> > +			break;
> >  		case 'g':
> >  			dirty_log_manual_caps = 0;
> >  			break;
> > diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> > index f93f2ea7c6a3..d9664a31e01c 100644
> > --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> > +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> > @@ -39,6 +39,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];
> >  };
> > @@ -53,6 +54,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 12a3597be1f9..ce657fa92f05 100644
> > --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> > +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> > @@ -46,6 +46,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
> >  	struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_id];
> >  	uint64_t gva;
> >  	uint64_t pages;
> > +	uint64_t addr;
> >  	int i;
> >  	uint32_t rand = pta->random_seed + vcpu_id;
> >  
> > @@ -57,7 +58,13 @@ void perf_test_guest_code(uint32_t vcpu_id)
> >  
> >  	while (true) {
> >  		for (i = 0; i < pages; i++) {
> > -			uint64_t addr = gva + (i * pta->guest_page_size);
> > +			guest_random(&rand);
> > +
> > +			if (pta->random_access)
> > +				addr = gva + ((rand % pages) * pta->guest_page_size);
> > +			else
> > +				addr = gva + (i * pta->guest_page_size);
> > +
> >  			guest_random(&rand);
> 
> Is it on purpose use a separate random number for access offset and
> read/write?
>

It's because of the following, from https://lore.kernel.org/kvm/YxDvVyFpMC9U3O25@google.com/

	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.

> >  
> >  			if (rand % 100 < pta->write_percent)
> > @@ -233,6 +240,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> >  	sync_global_to_guest(vm, perf_test_args.random_seed);
> >  }
> >  
> > +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.random_access);
> > +}
> > +
> >  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
> >  {
> >  	return 0;
> > -- 
> > 2.37.2.789.g6183377224-goog
> > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/3] KVM: selftests: randomize page access order
  2022-09-09 17:31     ` Ricardo Koller
@ 2022-09-09 17:41       ` David Matlack
  2022-09-12 19:14         ` Colton Lewis
  0 siblings, 1 reply; 15+ messages in thread
From: David Matlack @ 2022-09-09 17:41 UTC (permalink / raw)
  To: Ricardo Koller
  Cc: Colton Lewis, kvm list, Paolo Bonzini, Marc Zyngier,
	Sean Christopherson, Oliver Upton, Andrew Jones

On Fri, Sep 9, 2022 at 10:31 AM Ricardo Koller <ricarkol@google.com> wrote:
>
> On Fri, Sep 09, 2022 at 10:26:10AM -0700, David Matlack wrote:
> > On Fri, Sep 09, 2022 at 12:43:00PM +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.
> > >
> > > Population sets random access to false.
> >
> > Please make sure to also explain the why in addition to the what.
> >
> > >
> > > Signed-off-by: Colton Lewis <coltonlewis@google.com>
> > > ---
> > >  tools/testing/selftests/kvm/dirty_log_perf_test.c | 11 +++++++++--
> > >  .../selftests/kvm/include/perf_test_util.h        |  2 ++
> > >  tools/testing/selftests/kvm/lib/perf_test_util.c  | 15 ++++++++++++++-
> > >  3 files changed, 25 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > index c2ad299b3760..3639d5f95033 100644
> > > --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > @@ -127,6 +127,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)
> > > @@ -248,6 +249,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> > >             vcpu_last_completed_iteration[vcpu_id] = -1;
> > >
> > >     perf_test_set_write_percent(vm, 100);
> > > +   perf_test_set_random_access(vm, false);
> > >     perf_test_start_vcpu_threads(nr_vcpus, vcpu_worker);
> > >
> > >     /* Allow the vCPUs to populate memory */
> > > @@ -270,6 +272,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
> > >             ts_diff.tv_sec, ts_diff.tv_nsec);
> > >
> > >     perf_test_set_write_percent(vm, p->write_percent);
> > > +   perf_test_set_random_access(vm, p->random_access);
> > >
> > >     while (iteration < p->iterations) {
> > >             /*
> > > @@ -341,10 +344,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"
> > > @@ -396,8 +400,11 @@ int main(int argc, char *argv[])
> > >
> > >     guest_modes_append_default();
> > >
> > > -   while ((opt = getopt(argc, argv, "ghi:p:m:nb:v:or:s:x:w:")) != -1) {
> > > +   while ((opt = getopt(argc, argv, "aghi:p:m:nb:v:or:s:x:w:")) != -1) {
> > >             switch (opt) {
> > > +           case 'a':
> > > +                   p.random_access = true;
> > > +                   break;
> > >             case 'g':
> > >                     dirty_log_manual_caps = 0;
> > >                     break;
> > > diff --git a/tools/testing/selftests/kvm/include/perf_test_util.h b/tools/testing/selftests/kvm/include/perf_test_util.h
> > > index f93f2ea7c6a3..d9664a31e01c 100644
> > > --- a/tools/testing/selftests/kvm/include/perf_test_util.h
> > > +++ b/tools/testing/selftests/kvm/include/perf_test_util.h
> > > @@ -39,6 +39,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];
> > >  };
> > > @@ -53,6 +54,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 12a3597be1f9..ce657fa92f05 100644
> > > --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> > > +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> > > @@ -46,6 +46,7 @@ void perf_test_guest_code(uint32_t vcpu_id)
> > >     struct perf_test_vcpu_args *vcpu_args = &pta->vcpu_args[vcpu_id];
> > >     uint64_t gva;
> > >     uint64_t pages;
> > > +   uint64_t addr;
> > >     int i;
> > >     uint32_t rand = pta->random_seed + vcpu_id;
> > >
> > > @@ -57,7 +58,13 @@ void perf_test_guest_code(uint32_t vcpu_id)
> > >
> > >     while (true) {
> > >             for (i = 0; i < pages; i++) {
> > > -                   uint64_t addr = gva + (i * pta->guest_page_size);
> > > +                   guest_random(&rand);
> > > +
> > > +                   if (pta->random_access)
> > > +                           addr = gva + ((rand % pages) * pta->guest_page_size);
> > > +                   else
> > > +                           addr = gva + (i * pta->guest_page_size);
> > > +
> > >                     guest_random(&rand);
> >
> > Is it on purpose use a separate random number for access offset and
> > read/write?
> >
>
> It's because of the following, from https://lore.kernel.org/kvm/YxDvVyFpMC9U3O25@google.com/
>
>         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.

Makes sense. Depending on how many bits of randomness we need (e.g.
read/write only needs 7) we could still use one random number. But the
bit manipulation would probably more complex than just generating
another random number (which looks like a fairly cheap calculation).

Colton can you add a comment here to explain the subtlety?

>
> > >
> > >                     if (rand % 100 < pta->write_percent)
> > > @@ -233,6 +240,12 @@ void perf_test_set_random_seed(struct kvm_vm *vm, uint32_t random_seed)
> > >     sync_global_to_guest(vm, perf_test_args.random_seed);
> > >  }
> > >
> > > +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.random_access);
> > > +}
> > > +
> > >  uint64_t __weak perf_test_nested_pages(int nr_vcpus)
> > >  {
> > >     return 0;
> > > --
> > > 2.37.2.789.g6183377224-goog
> > >

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code
  2022-09-09 17:08   ` David Matlack
@ 2022-09-12 19:13     ` Colton Lewis
  0 siblings, 0 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-12 19:13 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, pbonzini, maz, seanjc, oupton, ricarkol, andrew.jones

David Matlack <dmatlack@google.com> writes:

> On Fri, Sep 09, 2022 at 12:42:58PM +0000, Colton Lewis wrote:
>> Implement random number generation for guest code to randomize parts
>> of the test, making it less predictable and a more accurate reflection
>> of reality.

>> Create a -r argument to specify a random seed. If no argument is
>> provided, the seed defaults to the current Unix timestamp. The random
>> seed is set with perf_test_set_random_seed() and must be set before
>> guest_code runs to apply.

>> The random number generator chosen is the Park-Miller Linear
>> Congruential Generator, a fancy name for a basic and well-understood
>> random number generator entirely sufficient for this purpose. Each
>> vCPU calculates its own seed by adding its index to the seed provided.

> Great commit message!


Thanks


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

>> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c  
>> b/tools/testing/selftests/kvm/dirty_log_perf_test.c
>> index d60a34cdfaee..2f91acd94130 100644
>> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
>> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
>> @@ -126,6 +126,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)
>> @@ -220,6 +221,8 @@ static void run_test(enum vm_guest_mode mode, void  
>> *arg)
>>   				 p->slots, p->backing_src,
>>   				 p->partition_vcpu_memory_access);

>> +	pr_info("Random seed: %u\n", p->random_seed);
>> +	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_get_page_shift(vm);
>> @@ -337,7 +340,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",
>> @@ -362,6 +365,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");
>> @@ -378,6 +382,7 @@ int main(int argc, char *argv[])
>>   		.partition_vcpu_memory_access = true,
>>   		.backing_src = DEFAULT_VM_MEM_SRC,
>>   		.slots = 1,
>> +		.random_seed = time(NULL),

> It's a bad code smell that the random seed gets default initialized to
> time(NULL) twice (here and in perf_test_create_vm()).

> I also still think it would be better if the default random seed was
> consistent across runs. Most use-cases of dirty_log_perf_test is for A/B
> testing, so consistency is key. For example, running dirty_log_perf_test
> at every commit to find regressions, or running dirty_log_perf_test to
> study the performance effects of some change. In other words, I think
> most use-cases will want a consistent seed across runs, so the default
> behavior should match that. Otherwise I forsee myself (and automated
> tools) having to pass in -r to every test runs to get consistent,
> comparable, behavior.

> What do you think about killing 2 birds with one stone here and make the
> default random_seed 0. That requires no initialization and ensures
> consistent random behavior across runs.


If you say so. Consider it done.

> And then optionally... I would even recommend dropping the -r parameter
> until someone wants to run dirty_log_perf_test with different seeds.
> That would simplify the code even more. I have a feeling there won't be
> much interest in different seeds since, at the end of the day, it will
> always be the same rough distribution of accesses. More interesting than
> different seeds will be adding support for different types of access
> patterns.

I'm leaving it in. I think people will want to compare different seeds
sometimes and it wasn't complicated to put in.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read
  2022-09-09 17:21   ` David Matlack
@ 2022-09-12 19:14     ` Colton Lewis
  0 siblings, 0 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-12 19:14 UTC (permalink / raw)
  To: David Matlack; +Cc: kvm, pbonzini, maz, seanjc, oupton, ricarkol, andrew.jones

David Matlack <dmatlack@google.com> writes:

> On Fri, Sep 09, 2022 at 12:42:59PM +0000, Colton Lewis wrote:
>> @@ -248,6 +247,7 @@ static void run_test(enum vm_guest_mode mode, void  
>> *arg)
>>   	for (vcpu_id = 0; vcpu_id < nr_vcpus; vcpu_id++)
>>   		vcpu_last_completed_iteration[vcpu_id] = -1;

>> +	perf_test_set_write_percent(vm, 100);

> This is a very important line of code and it's not very clear why it's
> here to a random reader. Please a comment here so someone doesn't have
> to go through the same confusion/debugging we went through to figure out
> why this is necessary. e.g.

>          /*
>           * Use 100% writes during the population phase to ensure all
>           * memory is actually populated and not just mapped to the zero
>           * page. The prevents expensive copy-on-write faults from
>           * occurring during the dirty memory iterations below, which
>           * would pollute the performance results.
>           */
>          perf_test_set_write_percent(vm, 100);

> Aside from that,

> Reviewed-by: David Matlack <dmatlack@google.com>


Will do.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 3/3] KVM: selftests: randomize page access order
  2022-09-09 17:41       ` David Matlack
@ 2022-09-12 19:14         ` Colton Lewis
  0 siblings, 0 replies; 15+ messages in thread
From: Colton Lewis @ 2022-09-12 19:14 UTC (permalink / raw)
  To: David Matlack; +Cc: ricarkol, kvm, pbonzini, maz, seanjc, oupton, andrew.jones

David Matlack <dmatlack@google.com> writes:

> On Fri, Sep 9, 2022 at 10:31 AM Ricardo Koller <ricarkol@google.com>  
> wrote:

>> On Fri, Sep 09, 2022 at 10:26:10AM -0700, David Matlack wrote:
>> > On Fri, Sep 09, 2022 at 12:43:00PM +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.
>> > >
>> > > Population sets random access to false.
>> >
>> > Please make sure to also explain the why in addition to the what.
>> >

Will do.

>> > > @@ -57,7 +58,13 @@ void perf_test_guest_code(uint32_t vcpu_id)
>> > >
>> > >     while (true) {
>> > >             for (i = 0; i < pages; i++) {
>> > > -                   uint64_t addr = gva + (i * pta->guest_page_size);
>> > > +                   guest_random(&rand);
>> > > +
>> > > +                   if (pta->random_access)
>> > > +                           addr = gva + ((rand % pages) *  
>> pta->guest_page_size);
>> > > +                   else
>> > > +                           addr = gva + (i * pta->guest_page_size);
>> > > +
>> > >                     guest_random(&rand);
>> >
>> > Is it on purpose use a separate random number for access offset and
>> > read/write?
>> >

>> It's because of the following, from  
>> https://lore.kernel.org/kvm/YxDvVyFpMC9U3O25@google.com/

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

> Makes sense. Depending on how many bits of randomness we need (e.g.
> read/write only needs 7) we could still use one random number. But the
> bit manipulation would probably more complex than just generating
> another random number (which looks like a fairly cheap calculation).

> Colton can you add a comment here to explain the subtlety?


Will do.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-09-12 19:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 12:42 [PATCH v5 0/3] KVM: selftests: randomize memory access of dirty_log_perf_test Colton Lewis
2022-09-09 12:42 ` [PATCH v5 1/3] KVM: selftests: implement random number generation for guest code Colton Lewis
2022-09-09 16:22   ` Ricardo Koller
2022-09-09 17:08   ` David Matlack
2022-09-12 19:13     ` Colton Lewis
2022-09-09 12:42 ` [PATCH v5 2/3] KVM: selftests: randomize which pages are written vs read Colton Lewis
2022-09-09 17:07   ` Ricardo Koller
2022-09-09 17:21   ` David Matlack
2022-09-12 19:14     ` Colton Lewis
2022-09-09 12:43 ` [PATCH v5 3/3] KVM: selftests: randomize page access order Colton Lewis
2022-09-09 17:17   ` Ricardo Koller
2022-09-09 17:26   ` David Matlack
2022-09-09 17:31     ` Ricardo Koller
2022-09-09 17:41       ` David Matlack
2022-09-12 19:14         ` Colton Lewis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).