[RFC 0/7] rt-tests: cyclictest: Add support to report standard deviation

[RFC 0/7] rt-tests: cyclictest: Add support to report standard deviation

[Punit Agrawal]

Hi,

Standard deviation is an important measure to evaluate the latency
response of a real time system. It is calculated by downstream tools
such as rteval but the value cannot be correctly calculated when the
latency value exceeds the range of histogram.

This series adds support to report the streaming standard deviation of
latencies to cyclictest. This approach avoids having to track all
latency values and adds minimal overhead (two additional doubles and
compute steps) to each iteration.

In terms of patch organization, the first 5 patches are clean-ups that
were noticed in the course of developing the support for standard
deviation. It should be possible to apply them indepndently.

Patch 6 converts the existing average calculation to a streaming
version. This running average value is needed for the standard
deviation.

Patch 7 finally adds support for calculating standard deviation.

The changes were verified by capturing the latency samples and
verifying the average and standard deviation with manual calculation.

All feedback welcome.

Thanks,
Punit

Punit Agrawal (7):
  rt-tests: cyclictest: Drop unused defines
  rt-tests: cyclictest: Simplify duplicate initialization of "stop"
  rt-tests: cyclictest: Drop unnecessary variable "stopped"
  rt-tests: cyclictest: Drop unnecessary variable "bufsize"
  rt-tests: cyclictest: Move signal handler to avoid function
    declaration
  rt-tests: cyclictest: Use streaming algorithm to calculate averages
  rt-tests: cyclictest: Add support to report standard deviation

 Makefile                    |   2 +-
 src/cyclictest/cyclictest.c | 149 +++++++++++++++++++++---------------
 2 files changed, 87 insertions(+), 64 deletions(-)

-- 
2.32.0

[RFC 1/7] rt-tests: cyclictest: Drop unused defines

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

KVARS, KVARNAMELEN and KVALUELEN defines are not used in
cyclictest. Drop them.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/cyclictest/cyclictest.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 2187d98de725..0c1e6617e0e1 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -99,10 +99,6 @@ extern int clock_nanosleep(clockid_t __clock_id, int __flags,
 /* Must be power of 2 ! */
 #define VALBUF_SIZE		16384
 
-#define KVARS			32
-#define KVARNAMELEN		32
-#define KVALUELEN		32
-
 #if (defined(__i386__) || defined(__x86_64__))
 #define ARCH_HAS_SMI_COUNTER
 #endif
-- 
2.32.0

[RFC 2/7] rt-tests: cyclictest: Simplify duplicate initialization of "stop"

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

The timespec structure "stop" is initialised whether it is used or not
as the compiler is not smart enough to figure out that it's use is
always guarded by the "duration" variable. As a result, "stop" needs
to be initialised whether it's used or not to avoid a compiler
warning.

Replace the duplicate memset statements by initializing "stop" using
structure initialiser.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/cyclictest/cyclictest.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 0c1e6617e0e1..d06ed01c58f4 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -507,7 +507,7 @@ static void *timerthread(void *param)
 	struct sigevent sigev;
 	sigset_t sigset;
 	timer_t timer;
-	struct timespec now, next, interval, stop;
+	struct timespec now, next, interval, stop = { 0 };
 	struct itimerval itimer;
 	struct itimerspec tspec;
 	struct thread_stat *stat = par->stats;
@@ -516,8 +516,6 @@ static void *timerthread(void *param)
 	pthread_t thread;
 	unsigned long smi_now, smi_old = 0;
 
-	memset(&stop, 0, sizeof(stop));
-
 	/* if we're running in numa mode, set our memory node */
 	if (par->node != -1)
 		rt_numa_set_numa_run_on_node(par->node, par->cpu);
@@ -598,7 +596,6 @@ static void *timerthread(void *param)
 	tsnorm(&next);
 
 	if (duration) {
-		memset(&stop, 0, sizeof(stop)); /* grrr */
 		stop = now;
 		stop.tv_sec += duration;
 	}
-- 
2.32.0

[RFC 3/7] rt-tests: cyclictest: Drop unnecessary variable "stopped"

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

In timerthread(), "stopped" is used to track when the tracelimit has
been breached. It prevents additional updates of break_thread_id.

But the timerthread functionality is already guarded by "shutdown"
which is also updated then latency crosses tracelimit. Remove the
redundant variable "stopped" to simplify the logic.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/cyclictest/cyclictest.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index d06ed01c58f4..721d242a1da0 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -511,7 +511,6 @@ static void *timerthread(void *param)
 	struct itimerval itimer;
 	struct itimerspec tspec;
 	struct thread_stat *stat = par->stats;
-	int stopped = 0;
 	cpu_set_t mask;
 	pthread_t thread;
 	unsigned long smi_now, smi_old = 0;
@@ -715,8 +714,7 @@ static void *timerthread(void *param)
 		if (duration && (calcdiff(now, stop) >= 0))
 			shutdown++;
 
-		if (!stopped && tracelimit && (diff > tracelimit)) {
-			stopped++;
+		if (tracelimit && (diff > tracelimit)) {
 			shutdown++;
 			pthread_mutex_lock(&break_thread_id_lock);
 			if (break_thread_id == 0) {
-- 
2.32.0

[RFC 4/7] rt-tests: cyclictest: Drop unnecessary variable "bufsize"

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

Two copies of "bufsize", initialised with the same value are declared
in enclosed blocks. Remove the redundant declaration.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/cyclictest/cyclictest.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 721d242a1da0..9c67a3ce3034 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -2054,7 +2054,6 @@ int main(int argc, char **argv)
 			memset(stat->values, 0, bufsize);
 			par->bufmsk = VALBUF_SIZE - 1;
 			if (smi) {
-				int bufsize = VALBUF_SIZE * sizeof(long);
 				stat->smis = threadalloc(bufsize, node);
 				if (!stat->smis)
 					goto outall;
-- 
2.32.0

[RFC 5/7] rt-tests: cyclictest: Move signal handler to avoid function declaration

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/cyclictest/cyclictest.c | 78 ++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 41 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 9c67a3ce3034..48a782fe167d 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -220,10 +220,6 @@ static char jsonfile[MAX_PATH];
 static struct thread_param **parameters;
 static struct thread_stat **statistics;
 
-static void print_stat(FILE *fp, struct thread_param *par, int index, int verbose, int quiet);
-static void rstat_print_stat(struct thread_param *par, int index, int verbose, int quiet);
-static void rstat_setup(void);
-
 static int latency_target_fd = -1;
 static int32_t latency_target_value = 0;
 
@@ -1311,43 +1307,6 @@ static int check_timer(void)
 	return (ts.tv_sec != 0 || ts.tv_nsec != 1);
 }
 
-static void sighand(int sig)
-{
-	if (sig == SIGUSR1) {
-		int i;
-		int oldquiet = quiet;
-
-		quiet = 0;
-		fprintf(stderr, "#---------------------------\n");
-		fprintf(stderr, "# cyclictest current status:\n");
-		for (i = 0; i < num_threads; i++)
-			print_stat(stderr, parameters[i], i, 0, 0);
-		fprintf(stderr, "#---------------------------\n");
-		quiet = oldquiet;
-		return;
-	} else if (sig == SIGUSR2) {
-		int i;
-		int oldquiet = quiet;
-
-		if (rstat_fd == -1) {
-			fprintf(stderr, "ERROR: rstat_fd not valid\n");
-			return;
-		}
-		rstat_ftruncate(rstat_fd, 0);
-		quiet = 0;
-		dprintf(rstat_fd, "#---------------------------\n");
-		dprintf(rstat_fd, "# cyclictest current status:\n");
-		for (i = 0; i < num_threads; i++)
-			rstat_print_stat(parameters[i], i, 0, 0);
-		dprintf(rstat_fd, "#---------------------------\n");
-		quiet = oldquiet;
-		return;
-	}
-	shutdown = 1;
-	if (refresh_on_max)
-		pthread_cond_signal(&refresh_on_max_cond);
-}
-
 static void print_tids(struct thread_param *par[], int nthreads)
 {
 	int i;
@@ -1566,6 +1525,43 @@ static void rstat_print_stat(struct thread_param *par, int index, int verbose, i
 }
 
 
+static void sighand(int sig)
+{
+	if (sig == SIGUSR1) {
+		int i;
+		int oldquiet = quiet;
+
+		quiet = 0;
+		fprintf(stderr, "#---------------------------\n");
+		fprintf(stderr, "# cyclictest current status:\n");
+		for (i = 0; i < num_threads; i++)
+			print_stat(stderr, parameters[i], i, 0, 0);
+		fprintf(stderr, "#---------------------------\n");
+		quiet = oldquiet;
+		return;
+	} else if (sig == SIGUSR2) {
+		int i;
+		int oldquiet = quiet;
+
+		if (rstat_fd == -1) {
+			fprintf(stderr, "ERROR: rstat_fd not valid\n");
+			return;
+		}
+		rstat_ftruncate(rstat_fd, 0);
+		quiet = 0;
+		dprintf(rstat_fd, "#---------------------------\n");
+		dprintf(rstat_fd, "# cyclictest current status:\n");
+		for (i = 0; i < num_threads; i++)
+			rstat_print_stat(parameters[i], i, 0, 0);
+		dprintf(rstat_fd, "#---------------------------\n");
+		quiet = oldquiet;
+		return;
+	}
+	shutdown = 1;
+	if (refresh_on_max)
+		pthread_cond_signal(&refresh_on_max_cond);
+}
+
 /*
  * thread that creates a named fifo and hands out run stats when someone
  * reads from the fifo.
-- 
2.32.0

[RFC 6/7] rt-tests: cyclictest: Use streaming algorithm to calculate averages

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

As part of latency evaluation, cyclictest reports the average
latencies observed for the threads. To calculate the average, the
aggregate latency of a thread across all samples is tracked and the
value calcualted at the end when it is reported.

In preparation for adding support to report standard deviation, update
the average calculation to track the streaming average, i.e., averages
of the samples observed so far.

Streaming average can be calculated using -

	  avg = prev_avg + (sample - prev_avg) / #samples

The patch implements this formula and updates the usage of average in
the rest of the code.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 src/cyclictest/cyclictest.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 48a782fe167d..dde8b1625c62 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -616,6 +616,7 @@ static void *timerthread(void *param)
 		uint64_t diff;
 		unsigned long diff_smi = 0;
 		int sigs, ret;
+		double prev_avg;
 
 		/* Wait for next period */
 		switch (par->mode) {
@@ -702,7 +703,6 @@ static void *timerthread(void *param)
 			if (refresh_on_max)
 				pthread_cond_signal(&refresh_on_max_cond);
 		}
-		stat->avg += (double) diff;
 
 		if (trigger && (diff > trigger))
 			trigger_update(par, diff, calctime(now));
@@ -742,6 +742,14 @@ static void *timerthread(void *param)
 
 		stat->cycles++;
 
+		/*
+		 * Calculate the streaming average
+		 *
+		 * avg = avg' + (x - avg') / n
+		 */
+		prev_avg = stat->avg;
+		stat->avg = prev_avg + ((diff - prev_avg) / stat->cycles);
+
 		next.tv_sec += interval.tv_sec;
 		next.tv_nsec += interval.tv_nsec;
 		if (par->mode == MODE_CYCLIC) {
@@ -1429,8 +1437,7 @@ static void print_stat(FILE *fp, struct thread_param *par, int index, int verbos
 
 			fprintf(fp, fmt, index, stat->tid, par->prio,
 				par->interval, stat->cycles, stat->min,
-				stat->act, stat->cycles ?
-				(long)(stat->avg/stat->cycles) : 0, stat->max);
+				stat->act, (long)stat->avg, stat->max);
 
 			if (smi)
 				fprintf(fp, " SMI:%8ld", stat->smi_count);
@@ -1485,8 +1492,7 @@ static void rstat_print_stat(struct thread_param *par, int index, int verbose, i
 
 			dprintf(fd, fmt, index, stat->tid, par->prio,
 				par->interval, stat->cycles, stat->min,
-				stat->act, stat->cycles ?
-				(long)(stat->avg/stat->cycles) : 0, stat->max);
+				stat->act, (long)stat->avg, stat->max);
 
 			if (smi)
 				dprintf(fd, " SMI:%8ld", stat->smi_count);
@@ -1774,7 +1780,7 @@ static void write_stats(FILE *f, void *data)
 		fprintf(f, "      \"cycles\": %ld,\n", s->cycles);
 		fprintf(f, "      \"min\": %ld,\n", s->min);
 		fprintf(f, "      \"max\": %ld,\n", s->max);
-		fprintf(f, "      \"avg\": %.2f,\n", s->avg/s->cycles);
+		fprintf(f, "      \"avg\": %.2f,\n", s->avg);
 		fprintf(f, "      \"cpu\": %d,\n", par[i]->cpu);
 		fprintf(f, "      \"node\": %d\n", par[i]->node);
 		fprintf(f, "    }%s\n", i == num_threads - 1 ? "" : ",");
-- 
2.32.0

[RFC 7/7] rt-tests: cyclictest: Add support to report standard deviation

[Punit Agrawal]

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

For real time systems, the standard deviation is an important metric
to help qualify the suitability for low-latency workloads. Downstream
tools such as rteval calculate the value of the standard deviation
based on the latency histogram but this approach doesn't work in
situations when the max latency exceeds the range of the histogram.

The common formula to calculating standard deviation requires tracking
all samples which is not feasible. But an alternative approach is to
track the streaming standard deviation of the latency samples. With
this it is possible to update the standard deviation using -

         d = d' + (x - avg)(x - avg')

    stddev = sqrt (d / (n - 1))

where 'd' is similar to the variance, 'x' is the current latency
sample, 'n' is the number of samples and prime terms (') refer to the
previous values.

Implement the above algorithm to calculate the standard deviation and
update the reporting to also include standard deviation.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 Makefile                    |  2 +-
 src/cyclictest/cyclictest.c | 43 +++++++++++++++++++++++++++++++------
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index ec5d693436d0..3f08788c0208 100644
--- a/Makefile
+++ b/Makefile
@@ -21,7 +21,7 @@ sources = cyclictest.c \
 	  oslat.c
 
 TARGETS = $(sources:.c=)
-LIBS	= -lrt -lpthread
+LIBS	= -lrt -lpthread -lm
 RTTESTLIB = -lrttest -L$(OBJDIR)
 EXTRA_LIBS ?= -ldl	# for get_cpu
 RTTESTNUMA = -lrttestnuma -lnuma
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index dde8b1625c62..2c18013a7643 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -23,6 +23,7 @@
 #include <errno.h>
 #include <limits.h>
 #include <linux/unistd.h>
+#include <math.h>
 
 #include <sys/prctl.h>
 #include <sys/stat.h>
@@ -134,6 +135,7 @@ struct thread_stat {
 	long max;
 	long act;
 	double avg;
+	double var;
 	long *values;
 	long *smis;
 	long *hist_array;
@@ -358,6 +360,16 @@ try_again:
 	return err;
 }
 
+static double calc_stddev(struct thread_stat *stat)
+{
+	/* standard deviation of single samples is 0 */
+	if (stat->cycles <= 1)
+		return 0;
+
+	return sqrt(stat->var / (stat->cycles - 1));
+}
+
+
 #ifdef ARCH_HAS_SMI_COUNTER
 static int open_msr_file(int cpu)
 {
@@ -750,6 +762,17 @@ static void *timerthread(void *param)
 		prev_avg = stat->avg;
 		stat->avg = prev_avg + ((diff - prev_avg) / stat->cycles);
 
+		/*
+		 * Calculate a slight variation on variance - the
+		 * variance multiplied by (n - 1). This allows
+		 * updating the quantity as data becomes available
+		 * without keeping track of all past samples or averages.
+		 *
+		 * (n - 1) * var = var' + (x - avg) * (x - avg')
+		 *
+		 */
+		stat->var += (diff - stat->avg) * (diff - prev_avg);
+
 		next.tv_sec += interval.tv_sec;
 		next.tv_nsec += interval.tv_nsec;
 		if (par->mode == MODE_CYCLIC) {
@@ -1379,6 +1402,10 @@ static void print_hist(struct thread_param *par[], int nthreads)
 		fprintf(fd, " %05lu", par[j]->stats->cycles ?
 		       (long)(par[j]->stats->avg/par[j]->stats->cycles) : 0);
 	fprintf(fd, "\n");
+	fprintf(fd, "# StdDev Latencies:");
+	for (j = 0; j < nthreads; j++)
+		fprintf(fd, " %05lu", (long)calc_stddev(par[j]->stats));
+	fprintf(fd, "\n");
 	fprintf(fd, "# Max Latencies:");
 	maxmax = 0;
 	for (j = 0; j < nthreads; j++) {
@@ -1430,14 +1457,15 @@ static void print_stat(FILE *fp, struct thread_param *par, int index, int verbos
 			char *fmt;
 			if (use_nsecs)
 				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "
-				        "Min:%7ld Act:%8ld Avg:%8ld Max:%8ld";
+				        "Min:%7ld Act:%8ld Avg:%8ld StdDev:%8ld Max:%8ld";
 			else
 				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "
-				        "Min:%7ld Act:%5ld Avg:%5ld Max:%8ld";
+				        "Min:%7ld Act:%5ld Avg:%5ld StdDev:%5ld Max:%8ld";
 
 			fprintf(fp, fmt, index, stat->tid, par->prio,
 				par->interval, stat->cycles, stat->min,
-				stat->act, (long)stat->avg, stat->max);
+				stat->act, (long)stat->avg, (long)calc_stddev(stat),
+				stat->max);
 
 			if (smi)
 				fprintf(fp, " SMI:%8ld", stat->smi_count);
@@ -1485,14 +1513,15 @@ static void rstat_print_stat(struct thread_param *par, int index, int verbose, i
 			char *fmt;
 			if (use_nsecs)
 				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "
-				        "Min:%7ld Act:%8ld Avg:%8ld Max:%8ld";
+				        "Min:%7ld Act:%8ld Avg:%8ld StdDev:%8ld Max:%8ld";
 			else
 				fmt = "T:%2d (%5d) P:%2d I:%ld C:%7lu "
-				        "Min:%7ld Act:%5ld Avg:%5ld Max:%8ld";
+				        "Min:%7ld Act:%5ld Avg:%5ld StdDev:%5ld Max:%8ld";
 
 			dprintf(fd, fmt, index, stat->tid, par->prio,
 				par->interval, stat->cycles, stat->min,
-				stat->act, (long)stat->avg, stat->max);
+				stat->act, (long)stat->avg, (long)calc_stddev(stat),
+				stat->max);
 
 			if (smi)
 				dprintf(fd, " SMI:%8ld", stat->smi_count);
@@ -1781,6 +1810,7 @@ static void write_stats(FILE *f, void *data)
 		fprintf(f, "      \"min\": %ld,\n", s->min);
 		fprintf(f, "      \"max\": %ld,\n", s->max);
 		fprintf(f, "      \"avg\": %.2f,\n", s->avg);
+		fprintf(f, "      \"stddev\": %.2f,\n", calc_stddev(s));
 		fprintf(f, "      \"cpu\": %d,\n", par[i]->cpu);
 		fprintf(f, "      \"node\": %d\n", par[i]->node);
 		fprintf(f, "    }%s\n", i == num_threads - 1 ? "" : ",");
@@ -2091,6 +2121,7 @@ int main(int argc, char **argv)
 		stat->min = 1000000;
 		stat->max = 0;
 		stat->avg = 0.0;
+		stat->var = 0;
 		stat->threadstarted = 1;
 		stat->smi_count = 0;
 		status = pthread_create(&stat->thread, &attr, timerthread, par);
-- 
2.32.0

Re: [RFC 7/7] rt-tests: cyclictest: Add support to report standard deviation

[Daniel Wagner]

Hi Punit,

On Thu, Oct 14, 2021 at 04:12:47PM +0900, Punit Agrawal wrote:
> @@ -1781,6 +1810,7 @@ static void write_stats(FILE *f, void *data)
>  		fprintf(f, "      \"min\": %ld,\n", s->min);
>  		fprintf(f, "      \"max\": %ld,\n", s->max);
>  		fprintf(f, "      \"avg\": %.2f,\n", s->avg);
> +		fprintf(f, "      \"stddev\": %.2f,\n", calc_stddev(s));
>  		fprintf(f, "      \"cpu\": %d,\n", par[i]->cpu);
>  		fprintf(f, "      \"node\": %d\n", par[i]->node);
>  		fprintf(f, "    }%s\n", i == num_threads - 1 ? "" : ",");

I think in this case you should also increase the version number of the
JSON file. And while at it, I'd love to see the same stats
fixes/extension for the other tools, not just cyclictest.

Daniel

Re: [RFC 1/7] rt-tests: cyclictest: Drop unused defines

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> KVARS, KVARNAMELEN and KVALUELEN defines are not used in
> cyclictest. Drop them.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 2187d98de725..0c1e6617e0e1 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -99,10 +99,6 @@ extern int clock_nanosleep(clockid_t __clock_id, int __flags,
>  /* Must be power of 2 ! */
>  #define VALBUF_SIZE		16384
>  
> -#define KVARS			32
> -#define KVARNAMELEN		32
> -#define KVALUELEN		32
> -
>  #if (defined(__i386__) || defined(__x86_64__))
>  #define ARCH_HAS_SMI_COUNTER
>  #endif
> -- 
> 2.32.0
> 
> 

This looks like more left over code from before we handed tracing 
functionality to tracecmd. Just remove the 'RFC' and I'll sign-off on 
that.

Re: [RFC 2/7] rt-tests: cyclictest: Simplify duplicate initialization of "stop"

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> The timespec structure "stop" is initialised whether it is used or not
> as the compiler is not smart enough to figure out that it's use is
> always guarded by the "duration" variable. As a result, "stop" needs
> to be initialised whether it's used or not to avoid a compiler
> warning.
> 
> Replace the duplicate memset statements by initializing "stop" using
> structure initialiser.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 0c1e6617e0e1..d06ed01c58f4 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -507,7 +507,7 @@ static void *timerthread(void *param)
>  	struct sigevent sigev;
>  	sigset_t sigset;
>  	timer_t timer;
> -	struct timespec now, next, interval, stop;
> +	struct timespec now, next, interval, stop = { 0 };
>  	struct itimerval itimer;
>  	struct itimerspec tspec;
>  	struct thread_stat *stat = par->stats;
> @@ -516,8 +516,6 @@ static void *timerthread(void *param)
>  	pthread_t thread;
>  	unsigned long smi_now, smi_old = 0;
>  
> -	memset(&stop, 0, sizeof(stop));
> -
>  	/* if we're running in numa mode, set our memory node */
>  	if (par->node != -1)
>  		rt_numa_set_numa_run_on_node(par->node, par->cpu);
> @@ -598,7 +596,6 @@ static void *timerthread(void *param)
>  	tsnorm(&next);
>  
>  	if (duration) {
> -		memset(&stop, 0, sizeof(stop)); /* grrr */
>  		stop = now;
>  		stop.tv_sec += duration;
>  	}
> -- 
> 2.32.0
> 
> 

Looks good, I'll sign off on that, just remove the RFC

Re: [RFC 4/7] rt-tests: cyclictest: Drop unnecessary variable "bufsize"

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> Two copies of "bufsize", initialised with the same value are declared
> in enclosed blocks. Remove the redundant declaration.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 721d242a1da0..9c67a3ce3034 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -2054,7 +2054,6 @@ int main(int argc, char **argv)
>  			memset(stat->values, 0, bufsize);
>  			par->bufmsk = VALBUF_SIZE - 1;
>  			if (smi) {
> -				int bufsize = VALBUF_SIZE * sizeof(long);
>  				stat->smis = threadalloc(bufsize, node);
>  				if (!stat->smis)
>  					goto outall;
> -- 
> 2.32.0
> 
> 
NACK: two different scopes, two different variables.

Re: [RFC 5/7] rt-tests: cyclictest: Move signal handler to avoid function declaration

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 78 ++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 41 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 9c67a3ce3034..48a782fe167d 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -220,10 +220,6 @@ static char jsonfile[MAX_PATH];
>  static struct thread_param **parameters;
>  static struct thread_stat **statistics;
>  
> -static void print_stat(FILE *fp, struct thread_param *par, int index, int verbose, int quiet);
> -static void rstat_print_stat(struct thread_param *par, int index, int verbose, int quiet);
> -static void rstat_setup(void);
> -
>  static int latency_target_fd = -1;
>  static int32_t latency_target_value = 0;
>  
> @@ -1311,43 +1307,6 @@ static int check_timer(void)
>  	return (ts.tv_sec != 0 || ts.tv_nsec != 1);
>  }
>  
> -static void sighand(int sig)
> -{
> -	if (sig == SIGUSR1) {
> -		int i;
> -		int oldquiet = quiet;
> -
> -		quiet = 0;
> -		fprintf(stderr, "#---------------------------\n");
> -		fprintf(stderr, "# cyclictest current status:\n");
> -		for (i = 0; i < num_threads; i++)
> -			print_stat(stderr, parameters[i], i, 0, 0);
> -		fprintf(stderr, "#---------------------------\n");
> -		quiet = oldquiet;
> -		return;
> -	} else if (sig == SIGUSR2) {
> -		int i;
> -		int oldquiet = quiet;
> -
> -		if (rstat_fd == -1) {
> -			fprintf(stderr, "ERROR: rstat_fd not valid\n");
> -			return;
> -		}
> -		rstat_ftruncate(rstat_fd, 0);
> -		quiet = 0;
> -		dprintf(rstat_fd, "#---------------------------\n");
> -		dprintf(rstat_fd, "# cyclictest current status:\n");
> -		for (i = 0; i < num_threads; i++)
> -			rstat_print_stat(parameters[i], i, 0, 0);
> -		dprintf(rstat_fd, "#---------------------------\n");
> -		quiet = oldquiet;
> -		return;
> -	}
> -	shutdown = 1;
> -	if (refresh_on_max)
> -		pthread_cond_signal(&refresh_on_max_cond);
> -}
> -
>  static void print_tids(struct thread_param *par[], int nthreads)
>  {
>  	int i;
> @@ -1566,6 +1525,43 @@ static void rstat_print_stat(struct thread_param *par, int index, int verbose, i
>  }
>  
>  
> +static void sighand(int sig)
> +{
> +	if (sig == SIGUSR1) {
> +		int i;
> +		int oldquiet = quiet;
> +
> +		quiet = 0;
> +		fprintf(stderr, "#---------------------------\n");
> +		fprintf(stderr, "# cyclictest current status:\n");
> +		for (i = 0; i < num_threads; i++)
> +			print_stat(stderr, parameters[i], i, 0, 0);
> +		fprintf(stderr, "#---------------------------\n");
> +		quiet = oldquiet;
> +		return;
> +	} else if (sig == SIGUSR2) {
> +		int i;
> +		int oldquiet = quiet;
> +
> +		if (rstat_fd == -1) {
> +			fprintf(stderr, "ERROR: rstat_fd not valid\n");
> +			return;
> +		}
> +		rstat_ftruncate(rstat_fd, 0);
> +		quiet = 0;
> +		dprintf(rstat_fd, "#---------------------------\n");
> +		dprintf(rstat_fd, "# cyclictest current status:\n");
> +		for (i = 0; i < num_threads; i++)
> +			rstat_print_stat(parameters[i], i, 0, 0);
> +		dprintf(rstat_fd, "#---------------------------\n");
> +		quiet = oldquiet;
> +		return;
> +	}
> +	shutdown = 1;
> +	if (refresh_on_max)
> +		pthread_cond_signal(&refresh_on_max_cond);
> +}
> +
>  /*
>   * thread that creates a named fifo and hands out run stats when someone
>   * reads from the fifo.
> -- 
> 2.32.0
> 
> 

NACK: What's wrong with function declarations? They allow the freedeom to 
put your function where you wish. This patch is unnecessary churn, sorry.

Re: [RFC 7/7] rt-tests: cyclictest: Add support to report standard deviation

[Punit Agrawal]

Daniel Wagner <wagi@monom.org> writes:

> Hi Punit,
>
> On Thu, Oct 14, 2021 at 04:12:47PM +0900, Punit Agrawal wrote:
>> @@ -1781,6 +1810,7 @@ static void write_stats(FILE *f, void *data)
>>  		fprintf(f, "      \"min\": %ld,\n", s->min);
>>  		fprintf(f, "      \"max\": %ld,\n", s->max);
>>  		fprintf(f, "      \"avg\": %.2f,\n", s->avg);
>> +		fprintf(f, "      \"stddev\": %.2f,\n", calc_stddev(s));
>>  		fprintf(f, "      \"cpu\": %d,\n", par[i]->cpu);
>>  		fprintf(f, "      \"node\": %d\n", par[i]->node);
>>  		fprintf(f, "    }%s\n", i == num_threads - 1 ? "" : ",");
>
> I think in this case you should also increase the version number of the
> JSON file.

Sure, I can bump up the version to 1.1 - minor update as it's adding a
new field rather than changing existing ones. Or "2" if that is
preferred.

Taking a closer look, the rt_write_json() helper, that outputs the
version, is shared with other tools. Bumping the version will bump it up
for all of them - would that be OK?

> And while at it, I'd love to see the same stats
> fixes/extension for the other tools, not just cyclictest.

I can look at adding support for standard deviation to the other tools
if there's interest once the cyclictest changes have been merged.

Thanks for taking a look.

Punit

Re: [RFC 4/7] rt-tests: cyclictest: Drop unnecessary variable "bufsize"

[Punit Agrawal]

John Kacur <jkacur@redhat.com> writes:

> On Thu, 14 Oct 2021, Punit Agrawal wrote:
>
>> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> 
>> Two copies of "bufsize", initialised with the same value are declared
>> in enclosed blocks. Remove the redundant declaration.
>> 
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> ---
>>  src/cyclictest/cyclictest.c | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
>> index 721d242a1da0..9c67a3ce3034 100644
>> --- a/src/cyclictest/cyclictest.c
>> +++ b/src/cyclictest/cyclictest.c
>> @@ -2054,7 +2054,6 @@ int main(int argc, char **argv)
>>  			memset(stat->values, 0, bufsize);
>>  			par->bufmsk = VALBUF_SIZE - 1;
>>  			if (smi) {
>> -				int bufsize = VALBUF_SIZE * sizeof(long);
>>  				stat->smis = threadalloc(bufsize, node);
>>  				if (!stat->smis)
>>  					goto outall;
>> -- 
>> 2.32.0
>> 
>> 
> NACK: two different scopes, two different variables.

If they had different values or if the inner scope was not dependent
(nested) I would agree.

But no strong opinion - I'll drop this one.

Re: [RFC 5/7] rt-tests: cyclictest: Move signal handler to avoid function declaration

[Punit Agrawal]

Hi John,

John Kacur <jkacur@redhat.com> writes:

> On Thu, 14 Oct 2021, Punit Agrawal wrote:
>
>> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> 
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> ---
>>  src/cyclictest/cyclictest.c | 78 ++++++++++++++++++-------------------
>>  1 file changed, 37 insertions(+), 41 deletions(-)
>> 
>> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
>> index 9c67a3ce3034..48a782fe167d 100644
>> --- a/src/cyclictest/cyclictest.c
>> +++ b/src/cyclictest/cyclictest.c
>> @@ -220,10 +220,6 @@ static char jsonfile[MAX_PATH];
>>  static struct thread_param **parameters;
>>  static struct thread_stat **statistics;
>>  
>> -static void print_stat(FILE *fp, struct thread_param *par, int index, int verbose, int quiet);
>> -static void rstat_print_stat(struct thread_param *par, int index, int verbose, int quiet);
>> -static void rstat_setup(void);
>> -
>>  static int latency_target_fd = -1;
>>  static int32_t latency_target_value = 0;
>>  
>> @@ -1311,43 +1307,6 @@ static int check_timer(void)
>>  	return (ts.tv_sec != 0 || ts.tv_nsec != 1);
>>  }
>>  
>> -static void sighand(int sig)
>> -{
>> -	if (sig == SIGUSR1) {
>> -		int i;
>> -		int oldquiet = quiet;
>> -
>> -		quiet = 0;
>> -		fprintf(stderr, "#---------------------------\n");
>> -		fprintf(stderr, "# cyclictest current status:\n");
>> -		for (i = 0; i < num_threads; i++)
>> -			print_stat(stderr, parameters[i], i, 0, 0);
>> -		fprintf(stderr, "#---------------------------\n");
>> -		quiet = oldquiet;
>> -		return;
>> -	} else if (sig == SIGUSR2) {
>> -		int i;
>> -		int oldquiet = quiet;
>> -
>> -		if (rstat_fd == -1) {
>> -			fprintf(stderr, "ERROR: rstat_fd not valid\n");
>> -			return;
>> -		}
>> -		rstat_ftruncate(rstat_fd, 0);
>> -		quiet = 0;
>> -		dprintf(rstat_fd, "#---------------------------\n");
>> -		dprintf(rstat_fd, "# cyclictest current status:\n");
>> -		for (i = 0; i < num_threads; i++)
>> -			rstat_print_stat(parameters[i], i, 0, 0);
>> -		dprintf(rstat_fd, "#---------------------------\n");
>> -		quiet = oldquiet;
>> -		return;
>> -	}
>> -	shutdown = 1;
>> -	if (refresh_on_max)
>> -		pthread_cond_signal(&refresh_on_max_cond);
>> -}
>> -
>>  static void print_tids(struct thread_param *par[], int nthreads)
>>  {
>>  	int i;
>> @@ -1566,6 +1525,43 @@ static void rstat_print_stat(struct thread_param *par, int index, int verbose, i
>>  }
>>  
>>  
>> +static void sighand(int sig)
>> +{
>> +	if (sig == SIGUSR1) {
>> +		int i;
>> +		int oldquiet = quiet;
>> +
>> +		quiet = 0;
>> +		fprintf(stderr, "#---------------------------\n");
>> +		fprintf(stderr, "# cyclictest current status:\n");
>> +		for (i = 0; i < num_threads; i++)
>> +			print_stat(stderr, parameters[i], i, 0, 0);
>> +		fprintf(stderr, "#---------------------------\n");
>> +		quiet = oldquiet;
>> +		return;
>> +	} else if (sig == SIGUSR2) {
>> +		int i;
>> +		int oldquiet = quiet;
>> +
>> +		if (rstat_fd == -1) {
>> +			fprintf(stderr, "ERROR: rstat_fd not valid\n");
>> +			return;
>> +		}
>> +		rstat_ftruncate(rstat_fd, 0);
>> +		quiet = 0;
>> +		dprintf(rstat_fd, "#---------------------------\n");
>> +		dprintf(rstat_fd, "# cyclictest current status:\n");
>> +		for (i = 0; i < num_threads; i++)
>> +			rstat_print_stat(parameters[i], i, 0, 0);
>> +		dprintf(rstat_fd, "#---------------------------\n");
>> +		quiet = oldquiet;
>> +		return;
>> +	}
>> +	shutdown = 1;
>> +	if (refresh_on_max)
>> +		pthread_cond_signal(&refresh_on_max_cond);
>> +}
>> +
>>  /*
>>   * thread that creates a named fifo and hands out run stats when someone
>>   * reads from the fifo.
>> -- 
>> 2.32.0
>> 
>> 
>
> NACK: What's wrong with function declarations? They allow the freedeom to 
> put your function where you wish. This patch is unnecessary churn, sorry.

I wasn't implying there's anything wrong with a function
declaration. They seemed unnecessary and can be easily avoided. But if
there's a reason to prefer the current placement of the signal handler
(I couldn't see any obvious pattern), I'll drop the patch.

Thanks for comments on the other patches as well. I'll update the set
once you've had a chance to go through the rest.

Punit

Re: [RFC 7/7] rt-tests: cyclictest: Add support to report standard deviation

[Daniel Wagner]

On Fri, Oct 15, 2021 at 04:58:19PM +0900, Punit Agrawal wrote:
> Sure, I can bump up the version to 1.1 - minor update as it's adding a
> new field rather than changing existing ones. Or "2" if that is
> preferred.

I'd say we just increment the number to 2 and don't any semantic meaning
to the version number. Instead push the compatible checks to the user
side. I suspect many don't care anyway.

BTW, I've noticed we print the version number wrong. Currently,
'cyclictest --version' will print 'V 2.20' instead 'V 2.2'. The printf
formatting string is wrong. Same in the JSON output.  If you don't mind
you could update this well when you are updating file version
string. Thanks!

> I can look at adding support for standard deviation to the other tools
> if there's interest once the cyclictest changes have been merged.

Yeah, there a couple of tests which quite similar. So you might just
have to move the newly added code to util.h(?) and add the calls to the
tests.. It shouldn't be too difficult just a bit of repetitive
work. Probably a good starting point for an intern :)

> Thanks for taking a look.

Thanks for contributing!

Daniel

Re: [RFC 4/7] rt-tests: cyclictest: Drop unnecessary variable "bufsize"

[John Kacur]

On Fri, 15 Oct 2021, Punit Agrawal wrote:

> John Kacur <jkacur@redhat.com> writes:
> 
> > On Thu, 14 Oct 2021, Punit Agrawal wrote:
> >
> >> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> >> 
> >> Two copies of "bufsize", initialised with the same value are declared
> >> in enclosed blocks. Remove the redundant declaration.
> >> 
> >> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> >> ---
> >>  src/cyclictest/cyclictest.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >> 
> >> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> >> index 721d242a1da0..9c67a3ce3034 100644
> >> --- a/src/cyclictest/cyclictest.c
> >> +++ b/src/cyclictest/cyclictest.c
> >> @@ -2054,7 +2054,6 @@ int main(int argc, char **argv)
> >>  			memset(stat->values, 0, bufsize);
> >>  			par->bufmsk = VALBUF_SIZE - 1;
> >>  			if (smi) {
> >> -				int bufsize = VALBUF_SIZE * sizeof(long);
> >>  				stat->smis = threadalloc(bufsize, node);
> >>  				if (!stat->smis)
> >>  					goto outall;
> >> -- 
> >> 2.32.0
> >> 
> >> 
> > NACK: two different scopes, two different variables.
> 
> If they had different values or if the inner scope was not dependent
> (nested) I would agree.
> 
> But no strong opinion - I'll drop this one.
> 

I had another look at this and realize that I am wrong.
bufsize is just a buffer size passed to threadalloc
which does the malloc which gives the two different addresses we
are concerned with. Go ahead and submit this one and I'll sign-off
on it.

Thanks

John

Re: [RFC 0/7] rt-tests: cyclictest: Add support to report standard deviation

[Joseph Salisbury]

On Thu, Oct 14, 2021 at 3:13 AM Punit Agrawal <punitagrawal@gmail.com> wrote:
>
> Hi,
>
> Standard deviation is an important measure to evaluate the latency
> response of a real time system. It is calculated by downstream tools
> such as rteval but the value cannot be correctly calculated when the
> latency value exceeds the range of histogram.
>
> This series adds support to report the streaming standard deviation of
> latencies to cyclictest. This approach avoids having to track all
> latency values and adds minimal overhead (two additional doubles and
> compute steps) to each iteration.
>
> In terms of patch organization, the first 5 patches are clean-ups that
> were noticed in the course of developing the support for standard
> deviation. It should be possible to apply them indepndently.
>
> Patch 6 converts the existing average calculation to a streaming
> version. This running average value is needed for the standard
> deviation.
>
> Patch 7 finally adds support for calculating standard deviation.
>
> The changes were verified by capturing the latency samples and
> verifying the average and standard deviation with manual calculation.
>
> All feedback welcome.
>
> Thanks,
> Punit
>
> Punit Agrawal (7):
>   rt-tests: cyclictest: Drop unused defines
>   rt-tests: cyclictest: Simplify duplicate initialization of "stop"
>   rt-tests: cyclictest: Drop unnecessary variable "stopped"
>   rt-tests: cyclictest: Drop unnecessary variable "bufsize"
>   rt-tests: cyclictest: Move signal handler to avoid function
>     declaration
>   rt-tests: cyclictest: Use streaming algorithm to calculate averages
>   rt-tests: cyclictest: Add support to report standard deviation
>
>  Makefile                    |   2 +-
>  src/cyclictest/cyclictest.c | 149 +++++++++++++++++++++---------------
>  2 files changed, 87 insertions(+), 64 deletions(-)
>
> --
> 2.32.0
>

Hi Punit,

Thanks for your contributions!

I noticed there were no commit messages for patches 1 and 5.

Would it be possible to add commit messages to all patches in the
future?  Commit messages help those trying to learn (Me :-) ) and
understand the purpose of the patch.

Thanks,

Joe

Re: [RFC 0/7] rt-tests: cyclictest: Add support to report standard deviation

[Punit Agrawal]

Hi Joseph,

Joseph Salisbury <josephtsalisbury@gmail.com> writes:

> On Thu, Oct 14, 2021 at 3:13 AM Punit Agrawal <punitagrawal@gmail.com> wrote:
>>
>> Hi,
>>
>> Standard deviation is an important measure to evaluate the latency
>> response of a real time system. It is calculated by downstream tools
>> such as rteval but the value cannot be correctly calculated when the
>> latency value exceeds the range of histogram.
>>
>> This series adds support to report the streaming standard deviation of
>> latencies to cyclictest. This approach avoids having to track all
>> latency values and adds minimal overhead (two additional doubles and
>> compute steps) to each iteration.
>>
>> In terms of patch organization, the first 5 patches are clean-ups that
>> were noticed in the course of developing the support for standard
>> deviation. It should be possible to apply them indepndently.
>>
>> Patch 6 converts the existing average calculation to a streaming
>> version. This running average value is needed for the standard
>> deviation.
>>
>> Patch 7 finally adds support for calculating standard deviation.
>>
>> The changes were verified by capturing the latency samples and
>> verifying the average and standard deviation with manual calculation.
>>
>> All feedback welcome.
>>
>> Thanks,
>> Punit
>>
>> Punit Agrawal (7):
>>   rt-tests: cyclictest: Drop unused defines
>>   rt-tests: cyclictest: Simplify duplicate initialization of "stop"
>>   rt-tests: cyclictest: Drop unnecessary variable "stopped"
>>   rt-tests: cyclictest: Drop unnecessary variable "bufsize"
>>   rt-tests: cyclictest: Move signal handler to avoid function
>>     declaration
>>   rt-tests: cyclictest: Use streaming algorithm to calculate averages
>>   rt-tests: cyclictest: Add support to report standard deviation
>>
>>  Makefile                    |   2 +-
>>  src/cyclictest/cyclictest.c | 149 +++++++++++++++++++++---------------
>>  2 files changed, 87 insertions(+), 64 deletions(-)
>>
>> --
>> 2.32.0
>>
>
> Hi Punit,
>
> Thanks for your contributions!
>
> I noticed there were no commit messages for patches 1 and 5.

If you look closely, patch 1 does have a commit log.

I missed adding a commit log for patch 5 - but the gist of it is to drop
function declarations when they are not required. In this case, this is
achieved by having the caller defined after all the callees have been
defined.

I'll add a log if the patch gets reposted.

Thanks for taking a look.

Punit

[...]

Re: [RFC 1/7] rt-tests: cyclictest: Drop unused defines

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> KVARS, KVARNAMELEN and KVALUELEN defines are not used in
> cyclictest. Drop them.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 2187d98de725..0c1e6617e0e1 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -99,10 +99,6 @@ extern int clock_nanosleep(clockid_t __clock_id, int __flags,
>  /* Must be power of 2 ! */
>  #define VALBUF_SIZE		16384
>  
> -#define KVARS			32
> -#define KVARNAMELEN		32
> -#define KVALUELEN		32
> -
>  #if (defined(__i386__) || defined(__x86_64__))
>  #define ARCH_HAS_SMI_COUNTER
>  #endif
> -- 
> 2.32.0
> 
> 
Signed-off-by: John Kacur <jkacur@redhat.com>

Re: [RFC 2/7] rt-tests: cyclictest: Simplify duplicate initialization of "stop"

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> The timespec structure "stop" is initialised whether it is used or not
> as the compiler is not smart enough to figure out that it's use is
> always guarded by the "duration" variable. As a result, "stop" needs
> to be initialised whether it's used or not to avoid a compiler
> warning.
> 
> Replace the duplicate memset statements by initializing "stop" using
> structure initialiser.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 0c1e6617e0e1..d06ed01c58f4 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -507,7 +507,7 @@ static void *timerthread(void *param)
>  	struct sigevent sigev;
>  	sigset_t sigset;
>  	timer_t timer;
> -	struct timespec now, next, interval, stop;
> +	struct timespec now, next, interval, stop = { 0 };
>  	struct itimerval itimer;
>  	struct itimerspec tspec;
>  	struct thread_stat *stat = par->stats;
> @@ -516,8 +516,6 @@ static void *timerthread(void *param)
>  	pthread_t thread;
>  	unsigned long smi_now, smi_old = 0;
>  
> -	memset(&stop, 0, sizeof(stop));
> -
>  	/* if we're running in numa mode, set our memory node */
>  	if (par->node != -1)
>  		rt_numa_set_numa_run_on_node(par->node, par->cpu);
> @@ -598,7 +596,6 @@ static void *timerthread(void *param)
>  	tsnorm(&next);
>  
>  	if (duration) {
> -		memset(&stop, 0, sizeof(stop)); /* grrr */
>  		stop = now;
>  		stop.tv_sec += duration;
>  	}
> -- 
> 2.32.0
> 
> 
Signed-off-by: John Kacur <jkacur@redhat.com>

Re: [RFC 4/7] rt-tests: cyclictest: Drop unnecessary variable "bufsize"

[John Kacur]

On Thu, 14 Oct 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> Two copies of "bufsize", initialised with the same value are declared
> in enclosed blocks. Remove the redundant declaration.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  src/cyclictest/cyclictest.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 721d242a1da0..9c67a3ce3034 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -2054,7 +2054,6 @@ int main(int argc, char **argv)
>  			memset(stat->values, 0, bufsize);
>  			par->bufmsk = VALBUF_SIZE - 1;
>  			if (smi) {
> -				int bufsize = VALBUF_SIZE * sizeof(long);
>  				stat->smis = threadalloc(bufsize, node);
>  				if (!stat->smis)
>  					goto outall;
> -- 
> 2.32.0
> 
> 
Signed-off-by: John Kacur <jkacur@redhat.com>