more rt-tests cleanups and a cyclictest feature

Most functions in cyclictest were already 'static', with a few exceptions. Fixup those exceptions, in the interest of consistency, optimization, etc.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

src/cyclictest/cyclictest.c +5 -5

index 34053c5..9aebe32 100644
@@ -392,7 +392,7 @@ static inline int64_t calcdiff_ns(struct timespec t1, struct timespec t2)
 	return diff;
 }
 
-void traceopt(char *option)
+static void traceopt(char *option)
 {
 	char *ptr;
 	if (traceopt_count + 1 > traceopt_size) {

@@ -442,7 +442,7 @@ static void tracemark(char *fmt, ...)
 
 
 
-void tracing(int on)
+static void tracing(int on)
 {
 	if (on) {
 		switch (kernelversion) {

@@ -638,7 +638,7 @@ static void setup_tracer(void)
  *
  * the return value is a value in seconds
  */
-int parse_time_string(char *val)
+static int parse_time_string(char *val)
 {
 	char *end;
 	int t = strtol(val, &end, 10);

@@ -749,7 +749,7 @@ try_again:
  * - CLOCK_REALTIME
  *
  */
-void *timerthread(void *param)
+static void *timerthread(void *param)
 {
 	struct thread_param *par = param;
 	struct sched_param schedp;

@@ -1766,7 +1766,7 @@ static void print_stat(FILE *fp, struct thread_param *par, int index, int verbos
  * thread that creates a named fifo and hands out run stats when someone
  * reads from the fifo.
  */
-void *fifothread(void *param)
+static void *fifothread(void *param)
 {
 	int ret;
 	int fd;

Signature · 3 lines

2.5.0

--

Commit 15c6d81986b6 ("[cyclictest] added priority spreading option --priospread") introduced an additional option with the prefix 'prio'; this means that it is no longer possible for a user to specify '--prio' instead of the full '--priority'. Doing so results in the following error:

  $ cyclictest --prio=4
  cyclictest: option '--prio=4' is ambiguous; possibilities: '--priority' '--priospread'

Fix this up by updating the 'help' output.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

src/cyclictest/cyclictest.c +1 -1

index 9aebe32..4ced67f 100644
@@ -1062,7 +1062,7 @@ static void display_help(int error)
 	       "-N       --nsecs           print results in ns instead of us (default us)\n"
 	       "-o RED   --oscope=RED      oscilloscope mode, reduce verbose output by RED\n"
 	       "-O TOPT  --traceopt=TOPT   trace option\n"
-	       "-p PRIO  --prio=PRIO       priority of highest prio thread\n"
+	       "-p PRIO  --priority=PRIO   priority of highest prio thread\n"
 	       "-P       --preemptoff      Preempt off tracing (used with -b)\n"
 	       "-q       --quiet           print only a summary on exit\n"
 	       "	 --priospread       spread priority levels starting at specified value\n"

Signature · 1 line

2.5.0

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

Quoted 26 lines · Josh Cartwright

Commit 15c6d81986b6 ("[cyclictest] added priority spreading option --priospread") introduced an additional option with the prefix 'prio'; this means that it is no longer possible for a user to specify '--prio' instead of the full '--priority'. Doing so results in the following error:

  $ cyclictest --prio=4
  cyclictest: option '--prio=4' is ambiguous; possibilities: '--priority' '--priospread'

Fix this up by updating the 'help' output.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

src/cyclictest/cyclictest.c +1 -1

index 9aebe32..4ced67f 100644
@@ -1062,7 +1062,7 @@ static void display_help(int error)
 	       "-N       --nsecs           print results in ns instead of us (default us)\n"
 	       "-o RED   --oscope=RED      oscilloscope mode, reduce verbose output by RED\n"
 	       "-O TOPT  --traceopt=TOPT   trace option\n"
-	       "-p PRIO  --prio=PRIO       priority of highest prio thread\n"
+	       "-p PRIO  --priority=PRIO   priority of highest prio thread\n"

Quoted 5 lines

src/cyclictest/cyclictest.c +0 -0

 	       "-P       --preemptoff      Preempt off tracing (used with -b)\n"
 	       "-q       --quiet           print only a summary on exit\n"
 	       "	 --priospread       spread priority levels starting at specified value\n"

Signature · 1 line

2.5.0

This was already fixed in v0.93
417ddb5db0ab cyclictest: Fix long priority help text option

The condition that numa_on_and_available() checks is impossible to hit, as it's already being checked during process_options().

Removal of numa_on_and_available() also has the side effect of removing the following warning during build:

src/cyclictest/rt_numa.h:259:15: warning: implicit declaration of function ‘numa_available’ [-Wimplicit-function-declaration]

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 3 ---
 src/cyclictest/rt_numa.h    | 9 ---------
 2 files changed, 12 deletions(-)

src/cyclictest/cyclictest.c +0 -3

index 4ced67f..dc754fd 100644
@@ -1816,9 +1816,6 @@ int main(int argc, char **argv)
 	if (verbose)
 		printf("Max CPUs = %d\n", max_cpus);
 
-	/* Checks if numa is on, program exits if numa on but not available */
-	numa_on_and_available();
-
 	/* lock all memory (prevent swapping) */
 	if (lockall)
 		if (mlockall(MCL_CURRENT|MCL_FUTURE) == -1) {

src/cyclictest/rt_numa.h +0 -9

index 06c9420..caa80e6 100644
@@ -251,15 +251,6 @@ static inline void rt_bitmask_free(struct bitmask *mask)
 
 #endif	/* NUMA */
 
-/*
- * Any behavioral differences above are transparent to these functions
- */
-static void numa_on_and_available()
-{
-	if (numa && (numa_available() == -1))
-		fatal("--numa specified and numa functions not available.\n");
-}
-
 /** Returns number of bits set in mask. */
 static inline unsigned int rt_numa_bitmask_count(const struct bitmask *mask)
 {

Signature · 3 lines

2.5.0

--

From c74b5d3d0e8a4a298dd0480f832e43ac886fdca0 Mon Sep 17 00:00:00 2001

Quoted 9 lines · Josh Cartwright

The condition that numa_on_and_available() checks is impossible to hit, as it's already being checked during process_options().

Removal of numa_on_and_available() also has the side effect of removing the following warning during build:

src/cyclictest/rt_numa.h:259:15: warning: implicit declaration of function ???numa_available??? [-Wimplicit-function-declaration]

Signed-off-by Josh Cartwright <[email protected]>

We don't support building without numa libs anymore, although we of course support running on machines without numa. Never-the-less I created two versions of numa_on_and_available, one for if you build with the unsupported NUMA=0 and one for if you build with NUMA=1, which is the default.

I would prefer not to drop this function, since I think it cleanly documents the fact that numa_available must be called before any other numa library functions are defined.

As Josh Cartwright reported though, there was no need to call it from main() since it was already tested in process_options(), so I tested it there.

Tested by building with NUMA=0, NUMA=1 and with the non-standard -Wimplicit-function-declaration

Reported-by Signed-off-by: Josh Cartwright <[email protected]>

Signed-off-by John Kacur <[email protected]>

---
 src/cyclictest/cyclictest.c |  7 ++-----
 src/cyclictest/rt_numa.h    | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 10 deletions(-)

src/cyclictest/cyclictest.c +0 -0

index 213c527fd714..b3abfcc67407 100644

Quoted 11 lines · John Kacur

Quoted 9 lines · Josh Cartwright

The condition that numa_on_and_available() checks is impossible to hit, as it's already being checked during process_options().

Removal of numa_on_and_available() also has the side effect of removing the following warning during build:

src/cyclictest/rt_numa.h:259:15: warning: implicit declaration of function ???numa_available??? [-Wimplicit-function-declaration]

Signed-off-by Josh Cartwright <[email protected]>

[..]

Quoted 5 lines

We don't support building without numa libs anymore, although we of course support running on machines without numa. Never-the-less I created two versions of numa_on_and_available, one for if you build with the unsupported NUMA=0 and one for if you build with NUMA=1, which is the default.

:(. You risk plenty of embedded folks who don't much care about NUMA.

Quoted 9 lines

I would prefer not to drop this function, since I think it cleanly documents the fact that numa_available must be called before any other numa library functions are defined.

As Josh Cartwright reported though, there was no need to call it from main() since it was already tested in process_options(), so I tested it there.

Quoted 4 lines

Tested by building with NUMA=0, NUMA=1 and with the non-standard -Wimplicit-function-declaration

Reported-by Signed-off-by: Josh Cartwright <[email protected]>

s/Signed-off-by: /

:)

Quoted 11 lines

Signed-off-by John Kacur <[email protected]>

---
 src/cyclictest/cyclictest.c |  7 ++-----
 src/cyclictest/rt_numa.h    | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 10 deletions(-)

src/cyclictest/cyclictest.c +0 -0

index 213c527fd714..b3abfcc67407 100644

src/cyclictest/cyclictest.c static void process_options (int argc, char *argv[], int max_cpus) · line 1457

Quoted 7 lines

src/cyclictest/cyclictest.c +2 -0

@@ -1451,12 +1451,11 @@ static void process_options (int argc, char *argv[], int max_cpus)
 			setvbuf(stdout, NULL, _IONBF, 0); break;
 		case 'U':
 		case OPT_NUMA: /* NUMA testing */
+			numa = 1;	/* Turn numa on */
 			if (smp)
 				fatal("numa and smp options are mutually exclusive\n");
+			numa_on_and_available();

...except numa is always on here. So why check if NUMA is enabled again? Seems like a waste.

src/cyclictest/cyclictest.c static void process_options (int argc, char *argv[], int max_cpus) · line 1458

Quoted 3 lines

src/cyclictest/cyclictest.c +0 -2

 #ifdef NUMA
-			if (numa_available() == -1)
-				fatal("NUMA functionality not available!");

Perhaps a middle ground should be to keep this check around and define a static inline numa_available() in the NUMA:=0 case which unconditionally returns -1.

Thanks for taking a look,

tsnorm() is not used at all in signaltest. Remove it.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/signaltest/signaltest.c | 8 --------
 1 file changed, 8 deletions(-)

src/signaltest/signaltest.c +0 -8

index 9454a26..c6d1cfd 100644
@@ -69,14 +69,6 @@ static int shutdown;
 static int tracelimit = 0;
 static int oldtrace = 0;
 
-static inline void tsnorm(struct timespec *ts)
-{
-	while (ts->tv_nsec >= NSEC_PER_SEC) {
-		ts->tv_nsec -= NSEC_PER_SEC;
-		ts->tv_sec++;
-	}
-}
-
 static inline long calcdiff(struct timespec t1, struct timespec t2)
 {
 	long diff;

Signature · 3 lines

2.5.0

--

Quoted 29 lines · Josh Cartwright

tsnorm() is not used at all in signaltest. Remove it.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/signaltest/signaltest.c | 8 --------
 1 file changed, 8 deletions(-)

src/signaltest/signaltest.c +0 -8

index 9454a26..c6d1cfd 100644
@@ -69,14 +69,6 @@ static int shutdown;
 static int tracelimit = 0;
 static int oldtrace = 0;
 
-static inline void tsnorm(struct timespec *ts)
-{
-	while (ts->tv_nsec >= NSEC_PER_SEC) {
-		ts->tv_nsec -= NSEC_PER_SEC;
-		ts->tv_sec++;
-	}
-}
-
 static inline long calcdiff(struct timespec t1, struct timespec t2)
 {
 	long diff;

Signature · 1 line

2.5.0

Quoted 1 line

--

As you can see, many of the programs in this suite are modeled after cyclictest, but many of them don't unfortunately receive nearly the amount of testing that cyclictest does. Now the fact that this function was copied from cyclictest, but not used, sends off alarm bells in my head. We have various struct timespec in signaltest, could the fact that we are not calling tsnorm mean that there are some hidden potential defects? Rather than removing this function, I'd like to spend some time auditing the use of timespec here until I'm convinced.

On any sane platform sizeof(long) == sizeof(unsigned long), so this does not actually fix a real bug, but the code should at least be consistent.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/rt_numa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

src/cyclictest/rt_numa.h +1 -1

index caa80e6..9da9fd8 100644
@@ -229,7 +229,7 @@ static inline struct bitmask* rt_numa_parse_cpustring(const char* s,
 			 * max_cpus bits */
 			int nlongs = (max_cpus+BITS_PER_LONG-1)/BITS_PER_LONG;
 
-			mask->maskp = calloc(nlongs, sizeof(long));
+			mask->maskp = calloc(nlongs, sizeof(unsigned long));
 			if (mask->maskp) {
 				mask->maskp[cpu/BITS_PER_LONG] |=
 					(1UL << (cpu % BITS_PER_LONG));

Signature · 3 lines

2.5.0

--

The fifothread is only created when use_fifo is set; having the thread itself perform a check is redundant and unnecessary. Drop it.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 3 ---
 1 file changed, 3 deletions(-)

src/cyclictest/cyclictest.c +0 -3

index dc754fd..896cdd7 100644
@@ -1773,9 +1773,6 @@ static void *fifothread(void *param)
 	FILE *fp;
 	int i;
 
-	if (use_fifo == 0)
-		return NULL;
-
 	unlink(fifopath);
 	ret = mkfifo(fifopath, 0666);
 	if (ret) {

Signature · 3 lines

2.5.0

--

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

src/cyclictest/cyclictest.c +4 -1

index 896cdd7..2c44f8e 100644
@@ -2058,8 +2058,11 @@ int main(int argc, char **argv)
 			fatal("failed to create thread %d: %s\n", i, strerror(status));
 
 	}
-	if (use_fifo)
+	if (use_fifo) {
 		status = pthread_create(&fifo_threadid, NULL, fifothread, NULL);
+		if (status)
+			fatal("failed to create fifo thread: %s\n", strerror(status));
+	}
 
 	while (!shutdown) {
 		char lavg[256];

Signature · 3 lines

2.5.0

--

These functions never return to their caller. Mark them as such to aide in code generation and help out static analysis.

Signed-off-by Josh Cartwright <[email protected]>

---
 src/include/error.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

src/include/error.h +3 -3

index ae05a2e..4acff49 100644
@@ -6,14 +6,14 @@
 #include <stdarg.h>
 #include <string.h>
 
-void err_exit(int err, char *fmt, ...);
+void err_exit(int err, char *fmt, ...) __attribute__((noreturn));
 void err_msg(char *fmt, ...);
 void err_msg_n(int err, char *fmt, ...);
-void err_quit(char *fmt, ...);
+void err_quit(char *fmt, ...) __attribute__((noreturn));
 void debug(char *fmt, ...);
 void info(char *fmt, ...);
 void warn(char *fmt, ...);
-void fatal(char *fmt, ...);
+void fatal(char *fmt, ...) __attribute__((noreturn));
 void err_doit(int err, const char *fmt, va_list ap);
 
 #endif	/* __ERROR_H */

Signature · 3 lines

2.5.0

--

From: Gratian Crisan <[email protected]>

Add an option '-J' or '--histfile' to dump the latency histogram to <path> instead of stdout. This allows for live update of the current min, avg, and max numbers while retaining the option to save histogram data for later analysis.

Signed-off-by Gratian Crisan <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 97 ++++++++++++++++++++++++++++-----------------
 1 file changed, 60 insertions(+), 37 deletions(-)

src/cyclictest/cyclictest.c +60 -37

index 2c44f8e..ec1ed17 100644
@@ -188,6 +188,7 @@ static int aligned = 0;
 static int secaligned = 0;
 static int offset = 0;
 static int laptop = 0;
+static int use_histfile = 0;
 
 static pthread_cond_t refresh_on_max_cond = PTHREAD_COND_INITIALIZER;
 static pthread_mutex_t refresh_on_max_lock = PTHREAD_MUTEX_INITIALIZER;

@@ -210,6 +211,7 @@ static char *procfileprefix = "/proc/sys/kernel/";
 static char *fileprefix;
 static char tracer[MAX_PATH];
 static char fifopath[MAX_PATH];
+static char histfile[MAX_PATH];
 static char **traceptr;
 static int traceopt_count;
 static int traceopt_size;

@@ -1049,6 +1051,7 @@ static void display_help(int error)
 	       "                           (with same priority about many threads)\n"
 	       "                           US is the max time to be be tracked in microseconds\n"
 	       "-H       --histofall=US    same as -h except with an additional summary column\n"
+	       "-J       --histfile=<path> dump the latency histogram to <path> instead of stdout\n"
 	       "-i INTV  --interval=INTV   base interval of thread in us default=1000\n"
 	       "-I       --irqsoff         Irqsoff tracing (used with -b)\n"
 	       "-l LOOPS --loops=LOOPS     number of loops: default=0(endless)\n"

@@ -1214,13 +1217,13 @@ static char *policyname(int policy)
 enum option_values {
 	OPT_AFFINITY=1, OPT_NOTRACE, OPT_BREAKTRACE, OPT_PREEMPTIRQ, OPT_CLOCK,
 	OPT_CONTEXT, OPT_DISTANCE, OPT_DURATION, OPT_LATENCY, OPT_EVENT,
-	OPT_FTRACE, OPT_FIFO, OPT_HISTOGRAM, OPT_HISTOFALL, OPT_INTERVAL,
-	OPT_IRQSOFF, OPT_LOOPS, OPT_MLOCKALL, OPT_REFRESH, OPT_NANOSLEEP,
-	OPT_NSECS, OPT_OSCOPE, OPT_TRACEOPT, OPT_PRIORITY, OPT_PREEMPTOFF,
-	OPT_QUIET, OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION, OPT_SYSTEM,
-	OPT_SMP, OPT_THREADS, OPT_TRACER, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE,
-	OPT_WAKEUP, OPT_WAKEUPRT, OPT_DBGCYCLIC, OPT_POLICY, OPT_HELP, OPT_NUMOPTS,
-	OPT_ALIGNED, OPT_LAPTOP, OPT_SECALIGNED,
+	OPT_FTRACE, OPT_FIFO, OPT_HISTOGRAM, OPT_HISTOFALL, OPT_HISTFILE,
+	OPT_INTERVAL, OPT_IRQSOFF, OPT_LOOPS, OPT_MLOCKALL, OPT_REFRESH, OPT_NANOSLEEP,
+	OPT_NSECS, OPT_OSCOPE, OPT_TRACEOPT, OPT_PRIORITY, OPT_PREEMPTOFF, OPT_QUIET,
+	OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION, OPT_SYSTEM, OPT_SMP, OPT_THREADS,
+	OPT_TRACER, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE, OPT_WAKEUP, OPT_WAKEUPRT,
+	OPT_DBGCYCLIC, OPT_POLICY, OPT_HELP, OPT_NUMOPTS, OPT_ALIGNED, OPT_LAPTOP,
+	OPT_SECALIGNED,
 };
 
 /* Process commandline options */

@@ -1251,6 +1254,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
 			{"fifo",             required_argument, NULL, OPT_FIFO },
 			{"histogram",        required_argument, NULL, OPT_HISTOGRAM },
 			{"histofall",        required_argument, NULL, OPT_HISTOFALL },
+			{"histfile",	     required_argument, NULL, OPT_HISTFILE },
 			{"interval",         required_argument, NULL, OPT_INTERVAL },
 			{"irqsoff",          no_argument,       NULL, OPT_IRQSOFF },
 			{"laptop",	     no_argument,	NULL, OPT_LAPTOP },

@@ -1348,6 +1352,11 @@ static void process_options (int argc, char *argv[], int max_cpus)
 		case 'h':
 		case OPT_HISTOGRAM:
 			histogram = atoi(optarg); break;
+		case 'J':
+		case OPT_HISTFILE:
+			use_histfile = 1;
+			strncpy(histfile, optarg, strlen(optarg));
+			break;
 		case 'i':
 		case OPT_INTERVAL:
 			interval = atoi(optarg); break;

@@ -1653,74 +1662,88 @@ static void print_hist(struct thread_param *par[], int nthreads)
 	int i, j;
 	unsigned long long int log_entries[nthreads+1];
 	unsigned long maxmax, alloverflows;
+	FILE *fd;
 
 	bzero(log_entries, sizeof(log_entries));
 
-	printf("# Histogram\n");
+	if (use_histfile) {
+		fd = fopen(histfile, "w");
+		if (!fd) {
+			perror("opening histogram file:");
+			return;
+		}
+	} else {
+		fd = stdout;
+	}
+
+	fprintf(fd, "# Histogram\n");
 	for (i = 0; i < histogram; i++) {
 		unsigned long long int allthreads = 0;
 
-		printf("%06d ", i);
+		fprintf(fd, "%06d ", i);
 
 		for (j = 0; j < nthreads; j++) {
 			unsigned long curr_latency=par[j]->stats->hist_array[i];
-			printf("%06lu", curr_latency);
+			fprintf(fd, "%06lu", curr_latency);
 			if (j < nthreads - 1)
-				printf("\t");
+				fprintf(fd, "\t");
 			log_entries[j] += curr_latency;
 			allthreads += curr_latency;
 		}
 		if (histofall && nthreads > 1) {
-			printf("\t%06llu", allthreads);
+			fprintf(fd, "\t%06llu", allthreads);
 			log_entries[nthreads] += allthreads;
 		}
-		printf("\n");
+		fprintf(fd, "\n");
 	}
-	printf("# Total:");
+	fprintf(fd, "# Total:");
 	for (j = 0; j < nthreads; j++)
-		printf(" %09llu", log_entries[j]);
+		fprintf(fd, " %09llu", log_entries[j]);
 	if (histofall && nthreads > 1)
-		printf(" %09llu", log_entries[nthreads]);
-	printf("\n");
-	printf("# Min Latencies:");
+		fprintf(fd, " %09llu", log_entries[nthreads]);
+	fprintf(fd, "\n");
+	fprintf(fd, "# Min Latencies:");
 	for (j = 0; j < nthreads; j++)
-		printf(" %05lu", par[j]->stats->min);
-	printf("\n");
-	printf("# Avg Latencies:");
+		fprintf(fd, " %05lu", par[j]->stats->min);
+	fprintf(fd, "\n");
+	fprintf(fd, "# Avg Latencies:");
 	for (j = 0; j < nthreads; j++)
-		printf(" %05lu", par[j]->stats->cycles ?
+		fprintf(fd, " %05lu", par[j]->stats->cycles ?
 		       (long)(par[j]->stats->avg/par[j]->stats->cycles) : 0);
-	printf("\n");
-	printf("# Max Latencies:");
+	fprintf(fd, "\n");
+	fprintf(fd, "# Max Latencies:");
 	maxmax = 0;
 	for (j = 0; j < nthreads; j++) {
-		printf(" %05lu", par[j]->stats->max);
+		fprintf(fd, " %05lu", par[j]->stats->max);
 		if (par[j]->stats->max > maxmax)
 			maxmax = par[j]->stats->max;
 	}
 	if (histofall && nthreads > 1)
-		printf(" %05lu", maxmax);
-	printf("\n");
-	printf("# Histogram Overflows:");
+		fprintf(fd, " %05lu", maxmax);
+	fprintf(fd, "\n");
+	fprintf(fd, "# Histogram Overflows:");
 	alloverflows = 0;
 	for (j = 0; j < nthreads; j++) {
-		printf(" %05lu", par[j]->stats->hist_overflow);
+		fprintf(fd, " %05lu", par[j]->stats->hist_overflow);
 		alloverflows += par[j]->stats->hist_overflow;
 	}
 	if (histofall && nthreads > 1)
-		printf(" %05lu", alloverflows);
-	printf("\n");
+		fprintf(fd, " %05lu", alloverflows);
+	fprintf(fd, "\n");
 
-	printf("# Histogram Overflow at cycle number:\n");
+	fprintf(fd, "# Histogram Overflow at cycle number:\n");
 	for (i = 0; i < nthreads; i++) {
-		printf("# Thread %d:", i);
+		fprintf(fd, "# Thread %d:", i);
 		for (j = 0; j < par[i]->stats->num_outliers; j++)
-			printf(" %05lu", par[i]->stats->outliers[j]);
+			fprintf(fd, " %05lu", par[i]->stats->outliers[j]);
 		if (par[i]->stats->num_outliers < par[i]->stats->hist_overflow)
-			printf(" # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
-		printf("\n");
+			fprintf(fd, " # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
+		fprintf(fd, "\n");
 	}
-	printf("\n");
+	fprintf(fd, "\n");
+
+	if (use_histfile)
+		fclose(fd);
 }
 
 static void print_stat(FILE *fp, struct thread_param *par, int index, int verbose, int quiet)

Signature · 3 lines

2.5.0

--

Quoted 8 lines · Josh Cartwright

From: Gratian Crisan <[email protected]>

Add an option '-J' or '--histfile' to dump the latency histogram to <path> instead of stdout. This allows for live update of the current min, avg, and max numbers while retaining the option to save histogram data for later analysis.

Signed-off-by Gratian Crisan <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

We worked really hard to remove the large amount of options, and we may have been over zealous in some cases (Carsten?).

If I were to accept this patch, I would at least like you to remove the short form option, and just have it in the long form.

Quoted 9 lines · John Kacur

Quoted 8 lines · Josh Cartwright

From: Gratian Crisan <[email protected]>

Add an option '-J' or '--histfile' to dump the latency histogram to <path> instead of stdout. This allows for live update of the current min, avg, and max numbers while retaining the option to save histogram data for later analysis.

Signed-off-by Gratian Crisan <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

[..]

Quoted 3 lines

We worked really hard to remove the large amount of options, and we may have been over zealous in some cases (Carsten?).

Fair enough, cyclictest has way too many knobs. Regardless, we've at least found this particular option useful.

Quoted 2 lines

If I were to accept this patch, I would at least like you to remove the short form option, and just have it in the long form.

Here is a v2 with the short form -J dropped.

Thanks,
Josh

-- 8< --
From: Gratian Crisan <[email protected]>
Subject: [PATCH v2] cyclictest: add option for dumping the histogram in a file

Add an option '--histfile' to dump the latency histogram to <path> instead of stdout. This allows for live update of the current min, avg, and max numbers while retaining the option to save histogram data for later analysis.

Signed-off-by Gratian Crisan <[email protected]>

Signed-off-by Josh Cartwright <[email protected]>

---
 src/cyclictest/cyclictest.c | 96 ++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 37 deletions(-)

src/cyclictest/cyclictest.c +59 -37

index 62f7cd9..5c9600c 100644
@@ -188,6 +188,7 @@ static int aligned = 0;
 static int secaligned = 0;
 static int offset = 0;
 static int laptop = 0;
+static int use_histfile = 0;
 
 static pthread_cond_t refresh_on_max_cond = PTHREAD_COND_INITIALIZER;
 static pthread_mutex_t refresh_on_max_lock = PTHREAD_MUTEX_INITIALIZER;

@@ -210,6 +211,7 @@ static char *procfileprefix = "/proc/sys/kernel/";
 static char *fileprefix;
 static char tracer[MAX_PATH];
 static char fifopath[MAX_PATH];
+static char histfile[MAX_PATH];
 static char **traceptr;
 static int traceopt_count;
 static int traceopt_size;

@@ -1049,6 +1051,7 @@ static void display_help(int error)
 	       "                           (with same priority about many threads)\n"
 	       "                           US is the max time to be be tracked in microseconds\n"
 	       "-H       --histofall=US    same as -h except with an additional summary column\n"
+	       "	 --histfile=<path> dump the latency histogram to <path> instead of stdout\n"
 	       "-i INTV  --interval=INTV   base interval of thread in us default=1000\n"
 	       "-I       --irqsoff         Irqsoff tracing (used with -b)\n"
 	       "-l LOOPS --loops=LOOPS     number of loops: default=0(endless)\n"

@@ -1214,13 +1217,13 @@ static char *policyname(int policy)
 enum option_values {
 	OPT_AFFINITY=1, OPT_NOTRACE, OPT_BREAKTRACE, OPT_PREEMPTIRQ, OPT_CLOCK,
 	OPT_CONTEXT, OPT_DISTANCE, OPT_DURATION, OPT_LATENCY, OPT_EVENT,
-	OPT_FTRACE, OPT_FIFO, OPT_HISTOGRAM, OPT_HISTOFALL, OPT_INTERVAL,
-	OPT_IRQSOFF, OPT_LOOPS, OPT_MLOCKALL, OPT_REFRESH, OPT_NANOSLEEP,
-	OPT_NSECS, OPT_OSCOPE, OPT_TRACEOPT, OPT_PRIORITY, OPT_PREEMPTOFF,
-	OPT_QUIET, OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION, OPT_SYSTEM,
-	OPT_SMP, OPT_THREADS, OPT_TRACER, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE,
-	OPT_WAKEUP, OPT_WAKEUPRT, OPT_DBGCYCLIC, OPT_POLICY, OPT_HELP, OPT_NUMOPTS,
-	OPT_ALIGNED, OPT_LAPTOP, OPT_SECALIGNED,
+	OPT_FTRACE, OPT_FIFO, OPT_HISTOGRAM, OPT_HISTOFALL, OPT_HISTFILE,
+	OPT_INTERVAL, OPT_IRQSOFF, OPT_LOOPS, OPT_MLOCKALL, OPT_REFRESH, OPT_NANOSLEEP,
+	OPT_NSECS, OPT_OSCOPE, OPT_TRACEOPT, OPT_PRIORITY, OPT_PREEMPTOFF, OPT_QUIET,
+	OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION, OPT_SYSTEM, OPT_SMP, OPT_THREADS,
+	OPT_TRACER, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE, OPT_WAKEUP, OPT_WAKEUPRT,
+	OPT_DBGCYCLIC, OPT_POLICY, OPT_HELP, OPT_NUMOPTS, OPT_ALIGNED, OPT_LAPTOP,
+	OPT_SECALIGNED,
 };
 
 /* Process commandline options */

@@ -1251,6 +1254,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
 			{"fifo",             required_argument, NULL, OPT_FIFO },
 			{"histogram",        required_argument, NULL, OPT_HISTOGRAM },
 			{"histofall",        required_argument, NULL, OPT_HISTOFALL },
+			{"histfile",	     required_argument, NULL, OPT_HISTFILE },
 			{"interval",         required_argument, NULL, OPT_INTERVAL },
 			{"irqsoff",          no_argument,       NULL, OPT_IRQSOFF },
 			{"laptop",	     no_argument,	NULL, OPT_LAPTOP },

@@ -1348,6 +1352,10 @@ static void process_options (int argc, char *argv[], int max_cpus)
 		case 'h':
 		case OPT_HISTOGRAM:
 			histogram = atoi(optarg); break;
+		case OPT_HISTFILE:
+			use_histfile = 1;
+			strncpy(histfile, optarg, strlen(optarg));
+			break;
 		case 'i':
 		case OPT_INTERVAL:
 			interval = atoi(optarg); break;

@@ -1653,74 +1661,88 @@ static void print_hist(struct thread_param *par[], int nthreads)
 	int i, j;
 	unsigned long long int log_entries[nthreads+1];
 	unsigned long maxmax, alloverflows;
+	FILE *fd;
 
 	bzero(log_entries, sizeof(log_entries));
 
-	printf("# Histogram\n");
+	if (use_histfile) {
+		fd = fopen(histfile, "w");
+		if (!fd) {
+			perror("opening histogram file:");
+			return;
+		}
+	} else {
+		fd = stdout;
+	}
+
+	fprintf(fd, "# Histogram\n");
 	for (i = 0; i < histogram; i++) {
 		unsigned long long int allthreads = 0;
 
-		printf("%06d ", i);
+		fprintf(fd, "%06d ", i);
 
 		for (j = 0; j < nthreads; j++) {
 			unsigned long curr_latency=par[j]->stats->hist_array[i];
-			printf("%06lu", curr_latency);
+			fprintf(fd, "%06lu", curr_latency);
 			if (j < nthreads - 1)
-				printf("\t");
+				fprintf(fd, "\t");
 			log_entries[j] += curr_latency;
 			allthreads += curr_latency;
 		}
 		if (histofall && nthreads > 1) {
-			printf("\t%06llu", allthreads);
+			fprintf(fd, "\t%06llu", allthreads);
 			log_entries[nthreads] += allthreads;
 		}
-		printf("\n");
+		fprintf(fd, "\n");
 	}
-	printf("# Total:");
+	fprintf(fd, "# Total:");
 	for (j = 0; j < nthreads; j++)
-		printf(" %09llu", log_entries[j]);
+		fprintf(fd, " %09llu", log_entries[j]);
 	if (histofall && nthreads > 1)
-		printf(" %09llu", log_entries[nthreads]);
-	printf("\n");
-	printf("# Min Latencies:");
+		fprintf(fd, " %09llu", log_entries[nthreads]);
+	fprintf(fd, "\n");
+	fprintf(fd, "# Min Latencies:");
 	for (j = 0; j < nthreads; j++)
-		printf(" %05lu", par[j]->stats->min);
-	printf("\n");
-	printf("# Avg Latencies:");
+		fprintf(fd, " %05lu", par[j]->stats->min);
+	fprintf(fd, "\n");
+	fprintf(fd, "# Avg Latencies:");
 	for (j = 0; j < nthreads; j++)
-		printf(" %05lu", par[j]->stats->cycles ?
+		fprintf(fd, " %05lu", par[j]->stats->cycles ?
 		       (long)(par[j]->stats->avg/par[j]->stats->cycles) : 0);
-	printf("\n");
-	printf("# Max Latencies:");
+	fprintf(fd, "\n");
+	fprintf(fd, "# Max Latencies:");
 	maxmax = 0;
 	for (j = 0; j < nthreads; j++) {
-		printf(" %05lu", par[j]->stats->max);
+		fprintf(fd, " %05lu", par[j]->stats->max);
 		if (par[j]->stats->max > maxmax)
 			maxmax = par[j]->stats->max;
 	}
 	if (histofall && nthreads > 1)
-		printf(" %05lu", maxmax);
-	printf("\n");
-	printf("# Histogram Overflows:");
+		fprintf(fd, " %05lu", maxmax);
+	fprintf(fd, "\n");
+	fprintf(fd, "# Histogram Overflows:");
 	alloverflows = 0;
 	for (j = 0; j < nthreads; j++) {
-		printf(" %05lu", par[j]->stats->hist_overflow);
+		fprintf(fd, " %05lu", par[j]->stats->hist_overflow);
 		alloverflows += par[j]->stats->hist_overflow;
 	}
 	if (histofall && nthreads > 1)
-		printf(" %05lu", alloverflows);
-	printf("\n");
+		fprintf(fd, " %05lu", alloverflows);
+	fprintf(fd, "\n");
 
-	printf("# Histogram Overflow at cycle number:\n");
+	fprintf(fd, "# Histogram Overflow at cycle number:\n");
 	for (i = 0; i < nthreads; i++) {
-		printf("# Thread %d:", i);
+		fprintf(fd, "# Thread %d:", i);
 		for (j = 0; j < par[i]->stats->num_outliers; j++)
-			printf(" %05lu", par[i]->stats->outliers[j]);
+			fprintf(fd, " %05lu", par[i]->stats->outliers[j]);
 		if (par[i]->stats->num_outliers < par[i]->stats->hist_overflow)
-			printf(" # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
-		printf("\n");
+			fprintf(fd, " # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
+		fprintf(fd, "\n");
 	}
-	printf("\n");
+	fprintf(fd, "\n");
+
+	if (use_histfile)
+		fclose(fd);
 }
 
 static void print_stat(FILE *fp, struct thread_param *par, int index, int verbose, int quiet)

Signature · 1 line

2.5.1