[PATCH] Fix the processing of optional options for cyclictest

[PATCH] Fix the processing of optional options for cyclictest

[John Kacur]

Problem: when running cyclic test, short options are not parsing
optional arguments correctly when followed by a space.
"-t3" works, but "-t 3" does not.

The following patch fixes the parsing for -a (affinity) and -t
(threads) whether there is a space or no space after the optional
argument, and of course it does the right thing if there is no
argument.

Please apply, because there is the possibility that people are not
testing what they think, especially if tests are automated.

Thanks

Author: John Kacur <jkacur@gmail.com>  2008-06-27 09:47:56
Committer: John Kacur <jkacur@gmail.com>  2008-06-27 09:47:56
Parent: 7a052daef09a5af77815e21fbff84ef580110dee (rt-tests/Makefile:
Import an updated version number into RPM spec file)
Branch: master
Follows: v0.22
Precedes:

    -Fix the processing of options with optional parameters, so that the user
    can write -tNUM or -t NUM and it will work as expected. This was done
    for the -t and -a options
    - Modify the usage messages to correspond with the way the program makes use
    of the options.

------------------------- src/cyclictest/cyclictest.c -------------------------
index d3f545f..bf60f09 100644
@@ -571,7 +571,8 @@ static void display_help(void)
 	printf("cyclictest V %1.2f\n", VERSION_STRING);
 	printf("Usage:\n"
 	       "cyclictest <options>\n\n"
-	       "-a       --affinity        run thread #N on processor #N, if
possible\n"
+	       "-a [NUM] --affinity        run thread #N on processor #N, if
possible\n"
+	       "                           with NUM restricts it to the
first NUM processors\n"
 	       "-a PROC  --affinity=PROC   run all threads on processor #PROC\n"
 	       "-b USEC  --breaktrace=USEC send break trace command when
latency > USEC\n"
 	       "-B       --preemptirqs     both preempt and irqsoff tracing
(used with -b)\n"
@@ -591,7 +592,9 @@ static void display_help(void)
 	       "-r       --relative        use relative timer instead of absolute\n"
 	       "-s       --system          use sys_nanosleep and sys_setitimer\n"
 	       "-t       --threads         one thread per available processor\n"
-	       "-t NUM   --threads=NUM     number of threads: without -t default=1\n"
+	       "-t [NUM] --threads=NUM     number of threads:\n"
+	       "                           without NUM, threads = max_cpus\n"
+	       "                           without -t default = 1\n"
 	       "-v       --verbose         output values on stdout for statistics\n"
 	       "                           format: n:c:v n=tasknum c=count
v=value in us\n");
 	exit(0);
@@ -661,8 +664,12 @@ static void process_options (int argc, char *argv[])
 			if (optarg != NULL) {
 				affinity = atoi(optarg);
 				setaffinity = AFFINITY_SPECIFIED;
-			} else
+			} else if (optind<argc && atoi(argv[optind])) {
+				affinity = atoi(argv[optind]);
+				setaffinity = AFFINITY_SPECIFIED;
+			} else {
 				setaffinity = AFFINITY_USEALL;
+			}
 			break;
 		case 'b': tracelimit = atoi(optarg); break;
 		case 'B': tracetype = IRQPREEMPTOFF; break;
@@ -682,6 +689,8 @@ static void process_options (int argc, char *argv[])
 		case 't':
 			if (optarg != NULL)
 				num_threads = atoi(optarg);
+			else if (optind<argc && atoi(argv[optind]))
+				num_threads = atoi(argv[optind]);
 			else
 				num_threads = max_cpus;
 			break;

Re: [PATCH] Fix the processing of optional options for cyclictest

[Leon Woestenberg]

Hello,

I still found it strange that the RTWiki mentions "-t NUM" as if it
works, so the manual page of cyclictest and the wiki needs a review
too.

I think I have access and will change this:

http://rt.wiki.kernel.org/index.php/RT_PREEMPT_HOWTO

Figure 11. Benchmarking the Realtime Environment using Cyclictest

# ./cyclictest -p 80 -t 5 -n
1.58 1.61 1.62 3/68 4079

T: 0 ( 3131) P:80 I:    1000 C:16469865 Min:       8 Act:      13 Max: 62
T: 1 ( 3132) P:79 I:    1500 C: 9979903 Min:       8 Act:      18 Max: 75
T: 2 ( 3133) P:78 I:    2000 C: 7934887 Min:       9 Act:      22 Max: 83
T: 3 ( 3134) P:77 I:    2500 C: 6587910 Min:       9 Act:      25 Max: 81
T: 4 ( 3135) P:76 I:    3000 C: 5489925 Min:       9 Act:      27 Max: 86

Regards,
-- 
Leon

Re: [PATCH] Fix the processing of optional options for cyclictest

[Steven Rostedt]

5B

On Fri, 27 Jun 2008, Leon Woestenberg wrote:

>
> Hello,
>
> I still found it strange that the RTWiki mentions "-t NUM" as if it
> works, so the manual page of cyclictest and the wiki needs a review
> too.

It doesn't?

Then again I must have an old version of cyclictest, because both -t5 and
-t 5 work for me.

-- Steve

>
> I think I have access and will change this:
>
> http://rt.wiki.kernel.org/index.php/RT_PREEMPT_HOWTO
>
> Figure 11. Benchmarking the Realtime Environment using Cyclictest
>
> # ./cyclictest -p 80 -t 5 -n
> 1.58 1.61 1.62 3/68 4079
>
> T: 0 ( 3131) P:80 I:    1000 C:16469865 Min:       8 Act:      13 Max: 62
> T: 1 ( 3132) P:79 I:    1500 C: 9979903 Min:       8 Act:      18 Max: 75
> T: 2 ( 3133) P:78 I:    2000 C: 7934887 Min:       9 Act:      22 Max: 83
> T: 3 ( 3134) P:77 I:    2500 C: 6587910 Min:       9 Act:      25 Max: 81
> T: 4 ( 3135) P:76 I:    3000 C: 5489925 Min:       9 Act:      27 Max: 86

Re: [PATCH] Fix the processing of optional options for cyclictest

[Clark Williams]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Steven Rostedt wrote:
> 5B
>
> On Fri, 27 Jun 2008, Leon Woestenberg wrote:
>
>> Hello,
>>
>> I still found it strange that the RTWiki mentions "-t NUM" as if it
>> works, so the manual page of cyclictest and the wiki needs a review
>> too.
>
> It doesn't?
>
> Then again I must have an old version of cyclictest, because both -t5 and
> -t 5 work for me.
>
> -- Steve
>

You do. The current source in Thomas's git tree exhibits this
behavior, because of the way getopt() handles optional arguments.

Clark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iEYEARECAAYFAkhlEBMACgkQqA4JVb61b9eXdQCdG2PSWB40Os5VrT/wzttiUgFY
6fwAoIe+jW6q4AjIvmW1YQ8UMZHqFXoY
=Hxzp
-----END PGP SIGNATURE-----

Re: [PATCH] Fix the processing of optional options for cyclictest

[John Kacur]

On Fri, Jun 27, 2008 at 6:06 PM, Clark Williams
<clark.williams@gmail.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Steven Rostedt wrote:
>> 5B
>>
>> On Fri, 27 Jun 2008, Leon Woestenberg wrote:
>>
>>> Hello,
>>>
>>> I still found it strange that the RTWiki mentions "-t NUM" as if it
>>> works, so the manual page of cyclictest and the wiki needs a review
>>> too.
>>
>> It doesn't?
>>
>> Then again I must have an old version of cyclictest, because both -t5 and
>> -t 5 work for me.
>>
>> -- Steve
>>
>
> You do. The current source in Thomas's git tree exhibits this
> behavior, because of the way getopt() handles optional arguments.
>
> Clark

Right the extra-code is only necessary for the processing of optional
arguments, so without checking the history of the program, I would
assume that the NUM argument to -t was not optional in the past. Now
it is optional because -t without arguments implies threads =
max_cpus. I made some very minor edits to the wiki a little while ago
to reflect this but the Wiki page does need to updated more, I think
the usage file corresponds a lot more closely to the actual behaviour.

crap, I see I left of the signed-off-by: John Kacur <jkacur@gmail.com>
line on the patch, do I need to resubmit it?

John

Re: [PATCH] Fix the processing of optional options for cyclictest

[Thomas Gleixner]

On Fri, 27 Jun 2008, John Kacur wrote:
> On Fri, Jun 27, 2008 at 6:06 PM, Clark Williams
> <clark.williams@gmail.com> wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > Steven Rostedt wrote:
> >> 5B
> >>
> >> On Fri, 27 Jun 2008, Leon Woestenberg wrote:
> >>
> >>> Hello,
> >>>
> >>> I still found it strange that the RTWiki mentions "-t NUM" as if it
> >>> works, so the manual page of cyclictest and the wiki needs a review
> >>> too.
> >>
> >> It doesn't?
> >>
> >> Then again I must have an old version of cyclictest, because both -t5 and
> >> -t 5 work for me.
> >>
> >> -- Steve
> >>
> >
> > You do. The current source in Thomas's git tree exhibits this
> > behavior, because of the way getopt() handles optional arguments.
> >
> > Clark
> 
> Right the extra-code is only necessary for the processing of optional
> arguments, so without checking the history of the program, I would
> assume that the NUM argument to -t was not optional in the past. Now
> it is optional because -t without arguments implies threads =
> max_cpus. I made some very minor edits to the wiki a little while ago
> to reflect this but the Wiki page does need to updated more, I think
> the usage file corresponds a lot more closely to the actual behaviour.
> 
> crap, I see I left of the signed-off-by: John Kacur <jkacur@gmail.com>
> line on the patch, do I need to resubmit it?

Grmbl, was going to ask that, but did not find you on IRC and I
committed it _before_ reading this mail. 

I applied it with two tweaks:

1) Your patch was line wrapped :(

2) I fixed the info for -a NUM. We pin the threads to exactly the CPU
specified with NUM

Pushed out to git and released 0.23.

Thanks,

	tglx