[PATCH 0/4] rt-tests: minor fixes

[PATCH 0/4] rt-tests: minor fixes

[Luiz Capitulino]

Hi,

I've found these little things while reading the code and
building rt-tests. They are all minor, but I'm sending
them anyways.

Luiz Capitulino (4):
  pip_stress: drop unused variable
  rt-migrate-test: fix return code
  don't use exit(-1) for failures
  cyclictest: initialize 'stop' early

 src/cyclictest/cyclictest.c           |  4 +++-
 src/pi_tests/pi_stress.c              |  2 +-
 src/pi_tests/pip_stress.c             |  7 +++----
 src/rt-migrate-test/rt-migrate-test.c | 12 ++++++------
 src/signaltest/signaltest.c           |  2 +-
 5 files changed, 14 insertions(+), 13 deletions(-)

-- 
2.1.0

[PATCH 1/4] pip_stress: drop unused variable

[Luiz Capitulino]

src/pi_tests/pip_stress.c: In function ‘main’:
src/pi_tests/pip_stress.c:94:6: warning: unused variable ‘c’ [-Wunused-variable]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 src/pi_tests/pip_stress.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/pi_tests/pip_stress.c b/src/pi_tests/pip_stress.c
index 66b3dc1..812a703 100644
--- a/src/pi_tests/pip_stress.c
+++ b/src/pi_tests/pip_stress.c
@@ -91,7 +91,6 @@ int main(void)
 	cpu_set_t set, *setp = &set;
 	int res;
 	int *minimum_priority = (int*)&prio_min;
-	int c;
 
 	*minimum_priority = sched_get_priority_min(policy);
 
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] pip_stress: drop unused variable

[John Kacur]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]



On Wed, 24 Feb 2016, Luiz Capitulino wrote:

> src/pi_tests/pip_stress.c: In function ‘main’:
> src/pi_tests/pip_stress.c:94:6: warning: unused variable ‘c’ [-Wunused-variable]
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  src/pi_tests/pip_stress.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/pi_tests/pip_stress.c b/src/pi_tests/pip_stress.c
> index 66b3dc1..812a703 100644
> --- a/src/pi_tests/pip_stress.c
> +++ b/src/pi_tests/pip_stress.c
> @@ -91,7 +91,6 @@ int main(void)
>  	cpu_set_t set, *setp = &set;
>  	int res;
>  	int *minimum_priority = (int*)&prio_min;
> -	int c;
>  
>  	*minimum_priority = sched_get_priority_min(policy);
>  
> -- 
> 2.1.0

Where are you getting your repo from?
According to my logs this was already fixed on June 30, 2015

The current repo is at
git://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git

Thanks

Re: [PATCH 1/4] pip_stress: drop unused variable

[Luiz Capitulino]

On Wed, 16 Mar 2016 23:00:37 +0100 (CET)
John Kacur <jkacur@redhat.com> wrote:

> 
> 
> On Wed, 24 Feb 2016, Luiz Capitulino wrote:
> 
> > src/pi_tests/pip_stress.c: In function ‘main’:
> > src/pi_tests/pip_stress.c:94:6: warning: unused variable ‘c’ [-Wunused-variable]
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  src/pi_tests/pip_stress.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/src/pi_tests/pip_stress.c b/src/pi_tests/pip_stress.c
> > index 66b3dc1..812a703 100644
> > --- a/src/pi_tests/pip_stress.c
> > +++ b/src/pi_tests/pip_stress.c
> > @@ -91,7 +91,6 @@ int main(void)
> >  	cpu_set_t set, *setp = &set;
> >  	int res;
> >  	int *minimum_priority = (int*)&prio_min;
> > -	int c;
> >  
> >  	*minimum_priority = sched_get_priority_min(policy);
> >  
> > -- 
> > 2.1.0
> 
> Where are you getting your repo from?
> According to my logs this was already fixed on June 30, 2015
> 
> The current repo is at
> git://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git

Oh, I'm using:

git://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rt-tests.git

I'll the series.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] rt-migrate-test: fix return code

[Luiz Capitulino]

Change both return codes for the stop == true case:

 * For failures, use exit(1) as exit(-1) is wrong
   (it actually becomes 255 in the shell)

 * For success, use exit(2) instead of exit(1) as
   exit(1) is usually used for errors

This should preserve the requirement of allowing
shell script while loops to break when Ctrl-C is hit.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 src/rt-migrate-test/rt-migrate-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
index fc6fd81..6e8acc2 100644
--- a/src/rt-migrate-test/rt-migrate-test.c
+++ b/src/rt-migrate-test/rt-migrate-test.c
@@ -594,9 +594,9 @@ int main (int argc, char **argv)
 		 * loop know to break.
 		 */
 		if (check < 0)
-			exit(-1);
-		else
 			exit(1);
+		else
+			exit(2);
 	}
 	if (check < 0)
 		exit(-1);
-- 
2.1.0

Re: [PATCH 2/4] rt-migrate-test: fix return code

[John Kacur]

On Wed, 24 Feb 2016, Luiz Capitulino wrote:

> Change both return codes for the stop == true case:
> 
>  * For failures, use exit(1) as exit(-1) is wrong
>    (it actually becomes 255 in the shell)
> 
>  * For success, use exit(2) instead of exit(1) as
>    exit(1) is usually used for errors
> 
> This should preserve the requirement of allowing
> shell script while loops to break when Ctrl-C is hit.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  src/rt-migrate-test/rt-migrate-test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
> index fc6fd81..6e8acc2 100644
> --- a/src/rt-migrate-test/rt-migrate-test.c
> +++ b/src/rt-migrate-test/rt-migrate-test.c
> @@ -594,9 +594,9 @@ int main (int argc, char **argv)
>  		 * loop know to break.
>  		 */
>  		if (check < 0)
> -			exit(-1);
> -		else
>  			exit(1);
> +		else
> +			exit(2);
>  	}
>  	if (check < 0)
>  		exit(-1);
> -- 
> 2.1.0
> 
> --

You can change the failure status from -1 to 1 if you wish, this is the 
normal value for EXIT_FAILURE

However the normal value for EXIT_SUCCESS is 0, so please do not change 
that.

Thanks

John

Re: [PATCH 2/4] rt-migrate-test: fix return code

[Luiz Capitulino]

On Wed, 16 Mar 2016 23:14:10 +0100 (CET)
John Kacur <jkacur@redhat.com> wrote:

> 
> 
> On Wed, 24 Feb 2016, Luiz Capitulino wrote:
> 
> > Change both return codes for the stop == true case:
> > 
> >  * For failures, use exit(1) as exit(-1) is wrong
> >    (it actually becomes 255 in the shell)
> > 
> >  * For success, use exit(2) instead of exit(1) as
> >    exit(1) is usually used for errors
> > 
> > This should preserve the requirement of allowing
> > shell script while loops to break when Ctrl-C is hit.
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  src/rt-migrate-test/rt-migrate-test.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
> > index fc6fd81..6e8acc2 100644
> > --- a/src/rt-migrate-test/rt-migrate-test.c
> > +++ b/src/rt-migrate-test/rt-migrate-test.c
> > @@ -594,9 +594,9 @@ int main (int argc, char **argv)
> >  		 * loop know to break.
> >  		 */
> >  		if (check < 0)
> > -			exit(-1);
> > -		else
> >  			exit(1);
> > +		else
> > +			exit(2);
> >  	}
> >  	if (check < 0)
> >  		exit(-1);
> > -- 
> > 2.1.0
> > 
> > --
> 
> You can change the failure status from -1 to 1 if you wish, this is the 
> normal value for EXIT_FAILURE
> 
> However the normal value for EXIT_SUCCESS is 0, so please do not change 
> that.

I'm not changing it. I'm maintaining the protocol used by rt-migrate-test:

        /*
         * We use this test in bash while loops
         * So if we hit Ctrl-C then let the while
         * loop know to break.
         */

[PATCH 3/4] don't use exit(-1) for failures

[Luiz Capitulino]

The kernel uses only 8 bits of the status as a return
code, so this actually becomes 255 in the shell.

In any case, the most widely convension is exit(1)
for failures, so let's be consistent.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 src/pi_tests/pi_stress.c              | 2 +-
 src/pi_tests/pip_stress.c             | 6 +++---
 src/rt-migrate-test/rt-migrate-test.c | 8 ++++----
 src/signaltest/signaltest.c           | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/pi_tests/pi_stress.c b/src/pi_tests/pi_stress.c
index aaa36c3..574fb65 100644
--- a/src/pi_tests/pi_stress.c
+++ b/src/pi_tests/pi_stress.c
@@ -634,7 +634,7 @@ int verify_cpu(int cpu)
 	if (status == -1) {
 		err = errno;
 		fprintf(stderr, "sched_getaffinity %s\n", strerror(err));
-		exit(-1);
+		exit(1);
 	}
 
 	if (CPU_ISSET(cpu, &mask))
diff --git a/src/pi_tests/pip_stress.c b/src/pi_tests/pip_stress.c
index 812a703..bef1f92 100644
--- a/src/pi_tests/pip_stress.c
+++ b/src/pi_tests/pip_stress.c
@@ -95,7 +95,7 @@ int main(void)
 	*minimum_priority = sched_get_priority_min(policy);
 
 	if (check_privs())
-		exit(-1);
+		exit(1);
 
 	mptr = mmap_page();	/* Get a page of shared memory */
 	resource = (pthread_mutex_t*)mptr;	/* point our lock to it */
@@ -138,7 +138,7 @@ int main(void)
 		pid2 = fork();		/* parent code */
 		if (pid2 == -1) {
 			perror("fork: ");
-			exit(-1);
+			exit(1);
 		} else if (pid2 != 0) {		/* parent code */
 			high(pid2);
 		} else {			/* child code */
@@ -231,7 +231,7 @@ void *mmap_page(void)
 	mptr = mmap(NULL, pgsize, PROTRW, MMAP_FLAGS, 0, 0);
 	if (mptr == MAP_FAILED) {
 		perror("In function mmap_page - mmap");
-		exit(-1);
+		exit(1);
 	}
 
 	return mptr;
diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
index 6e8acc2..4a7ecbb 100644
--- a/src/rt-migrate-test/rt-migrate-test.c
+++ b/src/rt-migrate-test/rt-migrate-test.c
@@ -141,7 +141,7 @@ static void perr(char *fmt, ...)
 
 	perror(buffer);
 	fflush(stderr);
-	exit(-1);
+	exit(1);
 }
 
 static void print_progress_bar(int percent)
@@ -228,11 +228,11 @@ static void parse_options (int argc, char *argv[])
 	}
 	if (nr_runs <= 0) {
 		fprintf(stderr, "Error: --loops argument is non-positive.  Exiting.\n");
-		exit(-1);
+		exit(1);
 	}
 	if (prio_start < 1 || prio_start > 99) {
 		fprintf(stderr, "Error: invalid value for --prio: %d (valid: 1-99)\n", prio_start);
-		exit(-1);
+		exit(1);
 	}
 }
 
@@ -599,7 +599,7 @@ int main (int argc, char **argv)
 			exit(2);
 	}
 	if (check < 0)
-		exit(-1);
+		exit(1);
 	else
 		exit(0);
 
diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
index 9454a26..7eaceec 100644
--- a/src/signaltest/signaltest.c
+++ b/src/signaltest/signaltest.c
@@ -318,7 +318,7 @@ int main(int argc, char **argv)
 	int i, ret = -1;
 
 	if (check_privs())
-		exit(-1);
+		exit(1);
 
 	process_options(argc, argv);
 
-- 
2.1.0

Re: [PATCH 3/4] don't use exit(-1) for failures

[John Kacur]

On Wed, 24 Feb 2016, Luiz Capitulino wrote:

> The kernel uses only 8 bits of the status as a return
> code, so this actually becomes 255 in the shell.
> 
> In any case, the most widely convension is exit(1)
> for failures, so let's be consistent.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  src/pi_tests/pi_stress.c              | 2 +-
>  src/pi_tests/pip_stress.c             | 6 +++---
>  src/rt-migrate-test/rt-migrate-test.c | 8 ++++----
>  src/signaltest/signaltest.c           | 2 +-
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/pi_tests/pi_stress.c b/src/pi_tests/pi_stress.c
> index aaa36c3..574fb65 100644
> --- a/src/pi_tests/pi_stress.c
> +++ b/src/pi_tests/pi_stress.c
> @@ -634,7 +634,7 @@ int verify_cpu(int cpu)
>  	if (status == -1) {
>  		err = errno;
>  		fprintf(stderr, "sched_getaffinity %s\n", strerror(err));
> -		exit(-1);
> +		exit(1);
>  	}
>  
>  	if (CPU_ISSET(cpu, &mask))
> diff --git a/src/pi_tests/pip_stress.c b/src/pi_tests/pip_stress.c
> index 812a703..bef1f92 100644
> --- a/src/pi_tests/pip_stress.c
> +++ b/src/pi_tests/pip_stress.c
> @@ -95,7 +95,7 @@ int main(void)
>  	*minimum_priority = sched_get_priority_min(policy);
>  
>  	if (check_privs())
> -		exit(-1);
> +		exit(1);
>  
>  	mptr = mmap_page();	/* Get a page of shared memory */
>  	resource = (pthread_mutex_t*)mptr;	/* point our lock to it */
> @@ -138,7 +138,7 @@ int main(void)
>  		pid2 = fork();		/* parent code */
>  		if (pid2 == -1) {
>  			perror("fork: ");
> -			exit(-1);
> +			exit(1);
>  		} else if (pid2 != 0) {		/* parent code */
>  			high(pid2);
>  		} else {			/* child code */
> @@ -231,7 +231,7 @@ void *mmap_page(void)
>  	mptr = mmap(NULL, pgsize, PROTRW, MMAP_FLAGS, 0, 0);
>  	if (mptr == MAP_FAILED) {
>  		perror("In function mmap_page - mmap");
> -		exit(-1);
> +		exit(1);
>  	}
>  
>  	return mptr;
> diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c
> index 6e8acc2..4a7ecbb 100644
> --- a/src/rt-migrate-test/rt-migrate-test.c
> +++ b/src/rt-migrate-test/rt-migrate-test.c
> @@ -141,7 +141,7 @@ static void perr(char *fmt, ...)
>  
>  	perror(buffer);
>  	fflush(stderr);
> -	exit(-1);
> +	exit(1);
>  }
>  
>  static void print_progress_bar(int percent)
> @@ -228,11 +228,11 @@ static void parse_options (int argc, char *argv[])
>  	}
>  	if (nr_runs <= 0) {
>  		fprintf(stderr, "Error: --loops argument is non-positive.  Exiting.\n");
> -		exit(-1);
> +		exit(1);
>  	}
>  	if (prio_start < 1 || prio_start > 99) {
>  		fprintf(stderr, "Error: invalid value for --prio: %d (valid: 1-99)\n", prio_start);
> -		exit(-1);
> +		exit(1);
>  	}
>  }
>  
> @@ -599,7 +599,7 @@ int main (int argc, char **argv)
>  			exit(2);
>  	}
>  	if (check < 0)
> -		exit(-1);
> +		exit(1);
>  	else
>  		exit(0);
>  
> diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
> index 9454a26..7eaceec 100644
> --- a/src/signaltest/signaltest.c
> +++ b/src/signaltest/signaltest.c
> @@ -318,7 +318,7 @@ int main(int argc, char **argv)
>  	int i, ret = -1;
>  
>  	if (check_privs())
> -		exit(-1);
> +		exit(1);
>  
>  	process_options(argc, argv);
>  
> -- 
> 2.1.0
> 
> --

This should be okay, but the patch doesn't apply, probably because
you're not using the most recent source. If you respin this patch and it 
applies cleanly I'll probably take it. (unless someone tells me that 
something like rteval or Carsten's tests require an exit of -1

Thanks

John

[PATCH 4/4] cyclictest: initialize 'stop' early

[Luiz Capitulino]

For some modes like MODE_CLOCK_NANOSLEEP, the clock
is already ticking when 'stop' is initialized. This
shouldn't matter, as this memset() should very fast
(I'd guess a few dozens or hundrends nanoseconds at
most?), but I also think there's no reason to do it
this late. So, initialize it along with everything
else.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 src/cyclictest/cyclictest.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 34053c5..22c5403 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -793,6 +793,8 @@ void *timerthread(void *param)
 		tspec.it_interval = interval;
 	}
 
+	memset(&stop, 0, sizeof(stop));
+
 	memset(&schedp, 0, sizeof(schedp));
 	schedp.sched_priority = par->prio;
 	if (setscheduler(0, par->policy, &schedp))
@@ -832,10 +834,10 @@ void *timerthread(void *param)
 	tsnorm(&next);
 
 	if (duration) {
-		memset(&stop, 0, sizeof(stop)); /* grrr */
 		stop = now;
 		stop.tv_sec += duration;
 	}
+
 	if (par->mode == MODE_CYCLIC) {
 		if (par->timermode == TIMER_ABSTIME)
 			tspec.it_value = next;
-- 
2.1.0