1. Patchew
  2. linux
  3. [PATCH v6 0/5] dirty_log_perf_test vCPU pinning
  4. View patch

[PATCH v6 2/5] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test

Vipin Sharma posted 5 patches 3 years, 5 months ago
There is a newer version of this series
[PATCH v6 2/5] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test
Posted by Vipin Sharma 3 years, 5 months ago 
There are 13 command line options and they are not in any order. Put
them in alphabetical order to make it easy to add new options.

No functional change intended.

Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
 .../selftests/kvm/dirty_log_perf_test.c       | 36 ++++++++++---------
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 56e08da3a87f..5bb6954b2358 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -406,50 +406,52 @@ int main(int argc, char *argv[])
 
 	guest_modes_append_default();
 
-	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
+	while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
 		switch (opt) {
+		case 'b':
+			guest_percpu_mem_size = parse_size(optarg);
+			break;
 		case 'e':
 			/* 'e' is for evil. */
 			run_vcpus_while_disabling_dirty_logging = true;
 			break;
+		case 'f':
+			p.wr_fract = atoi(optarg);
+			TEST_ASSERT(p.wr_fract >= 1,
+				    "Write fraction cannot be less than one");
+			break;
 		case 'g':
 			dirty_log_manual_caps = 0;
 			break;
+		case 'h':
+			help(argv[0]);
+			break;
 		case 'i':
 			p.iterations = atoi(optarg);
 			break;
-		case 'p':
-			p.phys_offset = strtoull(optarg, NULL, 0);
-			break;
 		case 'm':
 			guest_modes_cmdline(optarg);
 			break;
 		case 'n':
 			perf_test_args.nested = true;
 			break;
-		case 'b':
-			guest_percpu_mem_size = parse_size(optarg);
+		case 'o':
+			p.partition_vcpu_memory_access = false;
 			break;
-		case 'f':
-			p.wr_fract = atoi(optarg);
-			TEST_ASSERT(p.wr_fract >= 1,
-				    "Write fraction cannot be less than one");
+		case 'p':
+			p.phys_offset = strtoull(optarg, NULL, 0);
+			break;
+		case 's':
+			p.backing_src = parse_backing_src_type(optarg);
 			break;
 		case 'v':
 			nr_vcpus = atoi(optarg);
 			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
 				    "Invalid number of vcpus, must be between 1 and %d", max_vcpus);
 			break;
-		case 'o':
-			p.partition_vcpu_memory_access = false;
-			break;
-		case 's':
-			p.backing_src = parse_backing_src_type(optarg);
-			break;
 		case 'x':
 			p.slots = atoi(optarg);
 			break;
-		case 'h':
 		default:
 			help(argv[0]);
 			break;
-- 
2.38.0.135.g90850a2211-goog
RE: [PATCH v6 2/5] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test
Posted by Wang, Wei W 3 years, 5 months ago 
On Saturday, October 22, 2022 5:18 AM, Vipin Sharma wrote:
> There are 13 command line options and they are not in any order. Put them in
> alphabetical order to make it easy to add new options.
> 
> No functional change intended.
> 
> Signed-off-by: Vipin Sharma <vipinsh@google.com>
> ---
>  .../selftests/kvm/dirty_log_perf_test.c       | 36 ++++++++++---------
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> index 56e08da3a87f..5bb6954b2358 100644
> --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> @@ -406,50 +406,52 @@ int main(int argc, char *argv[])
> 
>  	guest_modes_append_default();
> 
> -	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
> +	while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
>  		switch (opt) {
> +		case 'b':
> +			guest_percpu_mem_size = parse_size(optarg);
> +			break;

"break" wasn't there.
This is kind of a functional change (i.e. bug fixing), and would be better to be
moved to patch 1.

Other parts look good to me.

Reviewed-by: Wei Wang <wei.w.wang@intel.com>


>  		case 'e':
>  			/* 'e' is for evil. */
>  			run_vcpus_while_disabling_dirty_logging = true;
>  			break;
> +		case 'f':
> +			p.wr_fract = atoi(optarg);
> +			TEST_ASSERT(p.wr_fract >= 1,
> +				    "Write fraction cannot be less than one");
> +			break;
>  		case 'g':
>  			dirty_log_manual_caps = 0;
>  			break;
> +		case 'h':
> +			help(argv[0]);
> +			break;
>  		case 'i':
>  			p.iterations = atoi(optarg);
>  			break;
> -		case 'p':
> -			p.phys_offset = strtoull(optarg, NULL, 0);
> -			break;
>  		case 'm':
>  			guest_modes_cmdline(optarg);
>  			break;
>  		case 'n':
>  			perf_test_args.nested = true;
>  			break;
> -		case 'b':
> -			guest_percpu_mem_size = parse_size(optarg);
> +		case 'o':
> +			p.partition_vcpu_memory_access = false;
>  			break;
> -		case 'f':
> -			p.wr_fract = atoi(optarg);
> -			TEST_ASSERT(p.wr_fract >= 1,
> -				    "Write fraction cannot be less than one");
> +		case 'p':
> +			p.phys_offset = strtoull(optarg, NULL, 0);
> +			break;
> +		case 's':
> +			p.backing_src = parse_backing_src_type(optarg);
>  			break;
>  		case 'v':
>  			nr_vcpus = atoi(optarg);
>  			TEST_ASSERT(nr_vcpus > 0 && nr_vcpus <= max_vcpus,
>  				    "Invalid number of vcpus, must be between 1 and %d",
> max_vcpus);
>  			break;
> -		case 'o':
> -			p.partition_vcpu_memory_access = false;
> -			break;
> -		case 's':
> -			p.backing_src = parse_backing_src_type(optarg);
> -			break;
>  		case 'x':
>  			p.slots = atoi(optarg);
>  			break;
> -		case 'h':
>  		default:
>  			help(argv[0]);
>  			break;
> --
> 2.38.0.135.g90850a2211-goog
Re: [PATCH v6 2/5] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test
Posted by Sean Christopherson 3 years, 5 months ago 
On Wed, Oct 26, 2022, Wang, Wei W wrote:
> On Saturday, October 22, 2022 5:18 AM, Vipin Sharma wrote:
> > There are 13 command line options and they are not in any order. Put them in
> > alphabetical order to make it easy to add new options.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Vipin Sharma <vipinsh@google.com>
> > ---
> >  .../selftests/kvm/dirty_log_perf_test.c       | 36 ++++++++++---------
> >  1 file changed, 19 insertions(+), 17 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > index 56e08da3a87f..5bb6954b2358 100644
> > --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > @@ -406,50 +406,52 @@ int main(int argc, char *argv[])
> > 
> >  	guest_modes_append_default();
> > 
> > -	while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
> > +	while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
> >  		switch (opt) {
> > +		case 'b':
> > +			guest_percpu_mem_size = parse_size(optarg);
> > +			break;
> 
> "break" wasn't there.
> This is kind of a functional change (i.e. bug fixing), and would be better to be
> moved to patch 1.

Ya, good catch.
Re: [PATCH v6 2/5] KVM: selftests: Put command line options in alphabetical order in dirty_log_perf_test
Posted by Vipin Sharma 3 years, 5 months ago 
On Wed, Oct 26, 2022 at 8:04 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Oct 26, 2022, Wang, Wei W wrote:
> > On Saturday, October 22, 2022 5:18 AM, Vipin Sharma wrote:
> > > There are 13 command line options and they are not in any order. Put them in
> > > alphabetical order to make it easy to add new options.
> > >
> > > No functional change intended.
> > >
> > > Signed-off-by: Vipin Sharma <vipinsh@google.com>
> > > ---
> > >  .../selftests/kvm/dirty_log_perf_test.c       | 36 ++++++++++---------
> > >  1 file changed, 19 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > index 56e08da3a87f..5bb6954b2358 100644
> > > --- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > +++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
> > > @@ -406,50 +406,52 @@ int main(int argc, char *argv[])
> > >
> > >     guest_modes_append_default();
> > >
> > > -   while ((opt = getopt(argc, argv, "eghi:p:m:nb:f:v:os:x:")) != -1) {
> > > +   while ((opt = getopt(argc, argv, "b:ef:ghi:m:nop:s:v:x:")) != -1) {
> > >             switch (opt) {
> > > +           case 'b':
> > > +                   guest_percpu_mem_size = parse_size(optarg);
> > > +                   break;
> >
> > "break" wasn't there.
> > This is kind of a functional change (i.e. bug fixing), and would be better to be
> > moved to patch 1.
>
> Ya, good catch.

diff is not very good in showing intuitive diff. "break" was there, it
just added "case 'o'" before it and didn't move break.

-               case 'b':
-                       guest_percpu_mem_size = parse_size(optarg);
+               case 'o':
+                       p.partition_vcpu_memory_access = false;
                        break;

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.