diff mbox series

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

Message ID 20220810175830.2175089-3-coltonlewis@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: selftests: Randomize memory access of dirty_log_perf_test | expand

Commit Message

Colton Lewis Aug. 10, 2022, 5:58 p.m. UTC
Randomize which pages are written vs read by using the random number
table for each page modulo 100. This changes how the -w argument
works. It is now a percentage from 0 to 100 inclusive that represents
what percentage of accesses are writes. It keeps the same default of
100 percent writes.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/dirty_log_perf_test.c | 12 +++++++-----
 tools/testing/selftests/kvm/lib/perf_test_util.c  |  4 ++--
 2 files changed, 9 insertions(+), 7 deletions(-)

Comments

David Matlack Aug. 10, 2022, 11:33 p.m. UTC | #1
On Wed, Aug 10, 2022 at 05:58:29PM +0000, Colton Lewis wrote:
> Randomize which pages are written vs read by using the random number

Same thing here about stating what the patch does first.

> table for each page modulo 100. This changes how the -w argument

s/-f/-w/

Although I would love it if you renamed -f to -w in the code instead.

> works. It is now a percentage from 0 to 100 inclusive that represents
> what percentage of accesses are writes. It keeps the same default of
> 100 percent writes.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_perf_test.c | 12 +++++++-----

access_tracking_perf_test.c also uses wr_fract and will need to be
updated.

>  tools/testing/selftests/kvm/lib/perf_test_util.c  |  4 ++--
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 80a1cbe7fbb0..dcc5d44fc757 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -381,8 +381,8 @@ static void help(char *name)
>  	       "     (default: 1G)\n");
>  	printf(" -f: specify the fraction of pages which should be written to\n"

s/fraction/percentage/

>  	       "     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");
> +	       "     [0-100]%% of pages to write.\n"
> +	       "     (default: 100 i.e. all pages are written to.)\n");

Mention that the implementation is probabilistic.

>  	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");
> @@ -399,7 +399,7 @@ 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,
> +		.wr_fract = 100,

Please rename wr_fract to e.g. write_percent to reflect the new
semantics. Same goes for perf_test_set_wr_fract(),
perf_test_args.wr_fract, etc.

>  		.partition_vcpu_memory_access = true,
>  		.backing_src = DEFAULT_VM_MEM_SRC,
>  		.slots = 1,
> @@ -439,8 +439,10 @@ int main(int argc, char *argv[])
>  			break;
>  		case 'f':
>  			p.wr_fract = atoi(optarg);
> -			TEST_ASSERT(p.wr_fract >= 1,
> -				    "Write fraction cannot be less than one");
> +			TEST_ASSERT(p.wr_fract >= 0,
> +				    "Write fraction cannot be less than 0");
> +			TEST_ASSERT(p.wr_fract <= 100,
> +				    "Write fraction cannot be greater than 100");

nit: This could be combined into a single assert pretty easily.

>  			break;
>  		case 'v':
>  			nr_vcpus = atoi(optarg);
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index b04e8d2c0f37..3c7b93349fef 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -64,7 +64,7 @@ void perf_test_guest_code(uint32_t vcpu_idx)
>  		for (i = 0; i < pages; i++) {
>  			uint64_t addr = gva + (i * pta->guest_page_size);
>  
> -			if (i % pta->wr_fract == 0)
> +			if (random_table[vcpu_idx][i] % 100 < pta->wr_fract)
>  				*(uint64_t *)addr = 0x0123456789ABCDEF;
>  			else
>  				READ_ONCE(*(uint64_t *)addr);
> @@ -168,7 +168,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
>  	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>  
>  	/* By default vCPUs will write to memory. */
> -	pta->wr_fract = 1;
> +	pta->wr_fract = 100;
>  
>  	/*
>  	 * Snapshot the non-huge page size.  This is used by the guest code to
> -- 
> 2.37.1.559.g78731f0fdb-goog
>
David Matlack Aug. 10, 2022, 11:37 p.m. UTC | #2
On Wed, Aug 10, 2022 at 04:33:44PM -0700, David Matlack wrote:
> On Wed, Aug 10, 2022 at 05:58:29PM +0000, Colton Lewis wrote:
> > Randomize which pages are written vs read by using the random number
> 
> Same thing here about stating what the patch does first.

Sorry -- you do state what the patch does first here. But I think it
could just be a little more direct and specific. e.g.

  Replace the -f<fraction> option in dirty_log_perf_test.c with
  -w<percent>, to allow the user to specify the percentage of which
  pages are written.

> 
> > table for each page modulo 100. This changes how the -w argument
> > works. It is now a percentage from 0 to 100 inclusive that represents
> > what percentage of accesses are writes. It keeps the same default of
> > 100 percent writes.
Colton Lewis Aug. 12, 2022, 4:11 p.m. UTC | #3
On Wed, Aug 10, 2022 at 04:37:47PM -0700, David Matlack wrote:
> On Wed, Aug 10, 2022 at 04:33:44PM -0700, David Matlack wrote:
> > On Wed, Aug 10, 2022 at 05:58:29PM +0000, Colton Lewis wrote:
> > > Randomize which pages are written vs read by using the random number
> > 
> > Same thing here about stating what the patch does first.
> 
> Sorry -- you do state what the patch does first here. But I think it
> could just be a little more direct and specific. e.g.
> 
>   Replace the -f<fraction> option in dirty_log_perf_test.c with
>   -w<percent>, to allow the user to specify the percentage of which
>   pages are written.
> 
> > 
> > > table for each page modulo 100. This changes how the -w argument
> > > works. It is now a percentage from 0 to 100 inclusive that represents
> > > what percentage of accesses are writes. It keeps the same default of
> > > 100 percent writes.

Will do.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 80a1cbe7fbb0..dcc5d44fc757 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -381,8 +381,8 @@  static void help(char *name)
 	       "     (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");
+	       "     [0-100]%% of pages to write.\n"
+	       "     (default: 100 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");
@@ -399,7 +399,7 @@  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,
+		.wr_fract = 100,
 		.partition_vcpu_memory_access = true,
 		.backing_src = DEFAULT_VM_MEM_SRC,
 		.slots = 1,
@@ -439,8 +439,10 @@  int main(int argc, char *argv[])
 			break;
 		case 'f':
 			p.wr_fract = atoi(optarg);
-			TEST_ASSERT(p.wr_fract >= 1,
-				    "Write fraction cannot be less than one");
+			TEST_ASSERT(p.wr_fract >= 0,
+				    "Write fraction cannot be less than 0");
+			TEST_ASSERT(p.wr_fract <= 100,
+				    "Write fraction cannot be greater than 100");
 			break;
 		case 'v':
 			nr_vcpus = atoi(optarg);
diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
index b04e8d2c0f37..3c7b93349fef 100644
--- a/tools/testing/selftests/kvm/lib/perf_test_util.c
+++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
@@ -64,7 +64,7 @@  void perf_test_guest_code(uint32_t vcpu_idx)
 		for (i = 0; i < pages; i++) {
 			uint64_t addr = gva + (i * pta->guest_page_size);
 
-			if (i % pta->wr_fract == 0)
+			if (random_table[vcpu_idx][i] % 100 < pta->wr_fract)
 				*(uint64_t *)addr = 0x0123456789ABCDEF;
 			else
 				READ_ONCE(*(uint64_t *)addr);
@@ -168,7 +168,7 @@  struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int nr_vcpus,
 	pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
 
 	/* By default vCPUs will write to memory. */
-	pta->wr_fract = 1;
+	pta->wr_fract = 100;
 
 	/*
 	 * Snapshot the non-huge page size.  This is used by the guest code to