All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature
@ 2015-08-31 16:35 Josh Cartwright
  2015-08-31 16:35 ` [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static' Josh Cartwright
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

Hey Clark, John-

Here's another set, this one contains eight patches are very minor cleanups
(found mostly via code inspection and by building with warnings cranked up).

The last patch introduces an option to cyclictest to control the dumping of
histogram data into a separate file, it's useful for when you want to see the
'normal' cyclictest live-updating, but also want to store away the generated
histogram for later analysis.

(My goal was to present a net-negative diffstat...I came so close!)

Thanks!
  Josh

Gratian Crisan (1):
  cyclictest: Add option for dumping the histogram in a file

Josh Cartwright (8):
  cyclictest: consistently make all functions 'static'
  cyclictest: fixup documentation for --priority option
  cyclictest: drop unnecessary numa_on_and_available() check
  signaltest: drop unused tsnorm()
  cyclictest: use correct type when allocating cpu bitmask size
  cyclictest: drop impossible use_fifo conditional
  cyclictest: fail if use_fifo && thread creation failed
  error: mark fatal, err_exit, err_quit as being noreturn

 src/cyclictest/cyclictest.c | 120 ++++++++++++++++++++++++++------------------
 src/cyclictest/rt_numa.h    |  11 +---
 src/include/error.h         |   6 +--
 src/signaltest/signaltest.c |   8 ---
 4 files changed, 74 insertions(+), 71 deletions(-)

-- 
2.5.0


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static'
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-02 13:03   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option Josh Cartwright
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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 <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 34053c5..9aebe32 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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;
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
  2015-08-31 16:35 ` [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static' Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-01 21:03   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check Josh Cartwright
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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 <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 9aebe32..4ced67f 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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"
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
  2015-08-31 16:35 ` [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static' Josh Cartwright
  2015-08-31 16:35 ` [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 21:50   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 4/9] signaltest: drop unused tsnorm() Josh Cartwright
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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 <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 3 ---
 src/cyclictest/rt_numa.h    | 9 ---------
 2 files changed, 12 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 4ced67f..dc754fd 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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) {
diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
index 06c9420..caa80e6 100644
--- a/src/cyclictest/rt_numa.h
+++ b/src/cyclictest/rt_numa.h
@@ -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");
-}

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 4/9] signaltest: drop unused tsnorm()
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
                   ` (2 preceding siblings ...)
  2015-08-31 16:35 ` [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 22:00   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 5/9] cyclictest: use correct type when allocating cpu bitmask size Josh Cartwright
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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

Signed-off-by: Josh Cartwright <joshc@ni.com>
---
 src/signaltest/signaltest.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
index 9454a26..c6d1cfd 100644
--- a/src/signaltest/signaltest.c
+++ b/src/signaltest/signaltest.c
@@ -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++;
-	}
-}

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 5/9] cyclictest: use correct type when allocating cpu bitmask size
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
                   ` (3 preceding siblings ...)
  2015-08-31 16:35 ` [PATCH rt-tests 4/9] signaltest: drop unused tsnorm() Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 22:19   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 6/9] cyclictest: drop impossible use_fifo conditional Josh Cartwright
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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 <joshc@ni.com>
---
 src/cyclictest/rt_numa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
index caa80e6..9da9fd8 100644
--- a/src/cyclictest/rt_numa.h
+++ b/src/cyclictest/rt_numa.h
@@ -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));
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 6/9] cyclictest: drop impossible use_fifo conditional
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
                   ` (4 preceding siblings ...)
  2015-08-31 16:35 ` [PATCH rt-tests 5/9] cyclictest: use correct type when allocating cpu bitmask size Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 22:24   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 7/9] cyclictest: fail if use_fifo && thread creation failed Josh Cartwright
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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 <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index dc754fd..896cdd7 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1773,9 +1773,6 @@ static void *fifothread(void *param)
 	FILE *fp;
 	int i;
 
-	if (use_fifo == 0)
-		return NULL;

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 7/9] cyclictest: fail if use_fifo && thread creation failed
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
                   ` (5 preceding siblings ...)
  2015-08-31 16:35 ` [PATCH rt-tests 6/9] cyclictest: drop impossible use_fifo conditional Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 22:25   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 8/9] error: mark fatal, err_exit, err_quit as being noreturn Josh Cartwright
  2015-08-31 16:35 ` [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file Josh Cartwright
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

Signed-off-by: Josh Cartwright <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 896cdd7..2c44f8e 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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];
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 8/9] error: mark fatal, err_exit, err_quit as being noreturn
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
                   ` (6 preceding siblings ...)
  2015-08-31 16:35 ` [PATCH rt-tests 7/9] cyclictest: fail if use_fifo && thread creation failed Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 22:31   ` John Kacur
  2015-08-31 16:35 ` [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file Josh Cartwright
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users

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 <joshc@ni.com>
---
 src/include/error.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/include/error.h b/src/include/error.h
index ae05a2e..4acff49 100644
--- a/src/include/error.h
+++ b/src/include/error.h
@@ -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 */
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file
  2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
                   ` (7 preceding siblings ...)
  2015-08-31 16:35 ` [PATCH rt-tests 8/9] error: mark fatal, err_exit, err_quit as being noreturn Josh Cartwright
@ 2015-08-31 16:35 ` Josh Cartwright
  2015-09-15 23:05   ` John Kacur
  8 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-08-31 16:35 UTC (permalink / raw
  To: Clark Williams, John Kacur; +Cc: linux-rt-users, Gratian Crisan

From: Gratian Crisan <gratian.crisan@ni.com>

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 <gratian.crisan@ni.com>
Signed-off-by: Josh Cartwright <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 97 ++++++++++++++++++++++++++++-----------------
 1 file changed, 60 insertions(+), 37 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 2c44f8e..ec1ed17 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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)
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option
  2015-08-31 16:35 ` [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option Josh Cartwright
@ 2015-09-01 21:03   ` John Kacur
  2015-09-01 21:11     ` Josh Cartwright
  0 siblings, 1 reply; 25+ messages in thread
From: John Kacur @ 2015-09-01 21:03 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> 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 <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 9aebe32..4ced67f 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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"
> -- 
> 2.5.0

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

Thanks> 
> --
> 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
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option
  2015-09-01 21:03   ` John Kacur
@ 2015-09-01 21:11     ` Josh Cartwright
  0 siblings, 0 replies; 25+ messages in thread
From: Josh Cartwright @ 2015-09-01 21:11 UTC (permalink / raw
  To: John Kacur; +Cc: Clark Williams, linux-rt-users

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

On Tue, Sep 01, 2015 at 11:03:08PM +0200, John Kacur wrote:
> On Mon, 31 Aug 2015, Josh Cartwright wrote:
> 
> > 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 <joshc@ni.com>
> > ---
> >  src/cyclictest/cyclictest.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> > index 9aebe32..4ced67f 100644
> > --- a/src/cyclictest/cyclictest.c
> > +++ b/src/cyclictest/cyclictest.c
> > @@ -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"
> > -- 
> > 2.5.0
> 
> This was already fixed in v0.93
> 417ddb5db0ab cyclictest: Fix long priority help text option

Ahh, so it was.  Sorry about that, I had my git remote pointed at
clrkwllms/rt-tests.git instead of rt-tests/rt-tests.git which hadn't
been updated.

The rest should still be relevant.

Thanks,

  Josh

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static'
  2015-08-31 16:35 ` [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static' Josh Cartwright
@ 2015-09-02 13:03   ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-02 13:03 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> 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 <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 34053c5..9aebe32 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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;
> -- 
> 2.5.0
> 
> --

Signed-off-by: John Kacur <jkacur@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check
  2015-08-31 16:35 ` [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check Josh Cartwright
@ 2015-09-15 21:50   ` John Kacur
  2015-09-16 19:47     ` Josh Cartwright
  0 siblings, 1 reply; 25+ messages in thread
From: John Kacur @ 2015-09-15 21:50 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users

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



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> 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 <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 3 ---
>  src/cyclictest/rt_numa.h    | 9 ---------
>  2 files changed, 12 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 4ced67f..dc754fd 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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) {
> diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
> index 06c9420..caa80e6 100644
> --- a/src/cyclictest/rt_numa.h
> +++ b/src/cyclictest/rt_numa.h
> @@ -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)
>  {
> -- 
> 2.5.0
> 
> --

I would prefer not to drop this function, since I think it cleanly 
documents a necessary initialization for using numa, althought here is 
something to be said for how cleanly your patch changes it. Proposing the 
following patch for now instead. Further clean-ups are possible.

>From c74b5d3d0e8a4a298dd0480f832e43ac886fdca0 Mon Sep 17 00:00:00 2001
From: John Kacur <jkacur@redhat.com>
Date: Tue, 15 Sep 2015 23:37:12 +0200
Subject: [PATCH] numa_on_and_available: Remove from main in cyclictest

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 <joshc@ni.com>
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c |  7 ++-----
 src/cyclictest/rt_numa.h    | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 213c527fd714..b3abfcc67407 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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();
 #ifdef NUMA
-			if (numa_available() == -1)
-				fatal("NUMA functionality not available!");
-			numa = 1;
 			num_threads = max_cpus;
 			setaffinity = AFFINITY_USEALL;
 			use_nanosleep = MODE_CLOCK_NANOSLEEP;
@@ -1816,8 +1815,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)
diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
index 06c9420e53cc..c1b3f942762d 100644
--- a/src/cyclictest/rt_numa.h
+++ b/src/cyclictest/rt_numa.h
@@ -192,6 +192,12 @@ static inline void rt_bitmask_free(struct bitmask *mask)
 
 #endif	/* LIBNUMA_API_VERSION */
 
+static void numa_on_and_available()
+{
+	if (numa && (numa_available() == -1))
+		fatal("--numa specified and numa functions not available.\n");
+}
+
 #else /* ! NUMA */
 
 struct bitmask {
@@ -249,17 +255,19 @@ static inline void rt_bitmask_free(struct bitmask *mask)
 	free(mask);
 }
 
-#endif	/* NUMA */
 
-/*
- * Any behavioral differences above are transparent to these functions
- */
 static void numa_on_and_available()
 {
-	if (numa && (numa_available() == -1))
+	if (numa) /* NUMA is not defined here */
 		fatal("--numa specified and numa functions not available.\n");
 }
 
+#endif	/* NUMA */
+
+/*
+ * Any behavioral differences above are transparent to these functions
+ */
+
 /** Returns number of bits set in mask. */
 static inline unsigned int rt_numa_bitmask_count(const struct bitmask *mask)
 {
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 4/9] signaltest: drop unused tsnorm()
  2015-08-31 16:35 ` [PATCH rt-tests 4/9] signaltest: drop unused tsnorm() Josh Cartwright
@ 2015-09-15 22:00   ` John Kacur
  2015-09-16 19:48     ` Josh Cartwright
  0 siblings, 1 reply; 25+ messages in thread
From: John Kacur @ 2015-09-15 22:00 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> tsnorm() is not used at all in signaltest.  Remove it.
> 
> Signed-off-by: Josh Cartwright <joshc@ni.com>
> ---
>  src/signaltest/signaltest.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
> index 9454a26..c6d1cfd 100644
> --- a/src/signaltest/signaltest.c
> +++ b/src/signaltest/signaltest.c
> @@ -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;
> -- 
> 2.5.0
> 
> --

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.

Thanks

John

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 5/9] cyclictest: use correct type when allocating cpu bitmask size
  2015-08-31 16:35 ` [PATCH rt-tests 5/9] cyclictest: use correct type when allocating cpu bitmask size Josh Cartwright
@ 2015-09-15 22:19   ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-15 22:19 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> 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 <joshc@ni.com>
> ---
>  src/cyclictest/rt_numa.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
> index caa80e6..9da9fd8 100644
> --- a/src/cyclictest/rt_numa.h
> +++ b/src/cyclictest/rt_numa.h
> @@ -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));
> -- 
> 2.5.0
> 
> --

Suspect this isn't the only inconsistency here between longs and unsigned 
longs, but
Signed-off-by: John Kacur <jkacur@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 6/9] cyclictest: drop impossible use_fifo conditional
  2015-08-31 16:35 ` [PATCH rt-tests 6/9] cyclictest: drop impossible use_fifo conditional Josh Cartwright
@ 2015-09-15 22:24   ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-15 22:24 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> 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 <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index dc754fd..896cdd7 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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) {
> -- 
> 2.5.0
> 
> --
Signed-off-by: John Kacur <jkacur@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 7/9] cyclictest: fail if use_fifo && thread creation failed
  2015-08-31 16:35 ` [PATCH rt-tests 7/9] cyclictest: fail if use_fifo && thread creation failed Josh Cartwright
@ 2015-09-15 22:25   ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-15 22:25 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> Signed-off-by: Josh Cartwright <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 896cdd7..2c44f8e 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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];
> -- 
> 2.5.0
> 
> --

Signed-off-by: John Kacur <jkacur@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 8/9] error: mark fatal, err_exit, err_quit as being noreturn
  2015-08-31 16:35 ` [PATCH rt-tests 8/9] error: mark fatal, err_exit, err_quit as being noreturn Josh Cartwright
@ 2015-09-15 22:31   ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-15 22:31 UTC (permalink / raw
  To: Josh Cartwright; +Cc: Clark Williams, John Kacur, linux-rt-users



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> 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 <joshc@ni.com>
> ---
>  src/include/error.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/include/error.h b/src/include/error.h
> index ae05a2e..4acff49 100644
> --- a/src/include/error.h
> +++ b/src/include/error.h
> @@ -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 */
> -- 
> 2.5.0
> 
> --
Signed-off-by: John Kacur <jkacur@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file
  2015-08-31 16:35 ` [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file Josh Cartwright
@ 2015-09-15 23:05   ` John Kacur
  2015-09-16 19:56     ` Josh Cartwright
  0 siblings, 1 reply; 25+ messages in thread
From: John Kacur @ 2015-09-15 23:05 UTC (permalink / raw
  To: Josh Cartwright
  Cc: Clark Williams, John Kacur, linux-rt-users, Gratian Crisan



On Mon, 31 Aug 2015, Josh Cartwright wrote:

> From: Gratian Crisan <gratian.crisan@ni.com>
> 
> 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 <gratian.crisan@ni.com>
> Signed-off-by: Josh Cartwright <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 97 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 60 insertions(+), 37 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 2c44f8e..ec1ed17 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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)
> -- 
> 2.5.0
> 
> --

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.

Thanks

John Kacur

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check
  2015-09-15 21:50   ` John Kacur
@ 2015-09-16 19:47     ` Josh Cartwright
  2015-09-17 19:15       ` John Kacur
  0 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-09-16 19:47 UTC (permalink / raw
  To: John Kacur; +Cc: Clark Williams, linux-rt-users

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

On Tue, Sep 15, 2015 at 11:50:16PM +0200, John Kacur wrote:
> On Mon, 31 Aug 2015, Josh Cartwright wrote:
> 
> > 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 <joshc@ni.com>

[..]

> 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.

> 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 <joshc@ni.com>

s/Signed-off-by: /

:)

> Signed-off-by: John Kacur <jkacur@redhat.com>
> ---
>  src/cyclictest/cyclictest.c |  7 ++-----
>  src/cyclictest/rt_numa.h    | 18 +++++++++++++-----
>  2 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 213c527fd714..b3abfcc67407 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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.

>  #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,
  Josh

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 4/9] signaltest: drop unused tsnorm()
  2015-09-15 22:00   ` John Kacur
@ 2015-09-16 19:48     ` Josh Cartwright
  0 siblings, 0 replies; 25+ messages in thread
From: Josh Cartwright @ 2015-09-16 19:48 UTC (permalink / raw
  To: John Kacur; +Cc: Clark Williams, linux-rt-users

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

On Wed, Sep 16, 2015 at 12:00:10AM +0200, John Kacur wrote:
> On Mon, 31 Aug 2015, Josh Cartwright wrote:
> 
> > tsnorm() is not used at all in signaltest.  Remove it.
> > 
> > Signed-off-by: Josh Cartwright <joshc@ni.com>
> > ---
> >  src/signaltest/signaltest.c | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
> > index 9454a26..c6d1cfd 100644
> > --- a/src/signaltest/signaltest.c
> > +++ b/src/signaltest/signaltest.c
> > @@ -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;
> > -- 
> > 2.5.0
> > 
> > --
> 
> 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.

Yeah, I did the cheap-and-easy thing here; but you raise a good point,
we should audit signaltest to see where any timespec math is done to
ensure proper normalization.

  Josh

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file
  2015-09-15 23:05   ` John Kacur
@ 2015-09-16 19:56     ` Josh Cartwright
  2015-09-17 19:40       ` John Kacur
  0 siblings, 1 reply; 25+ messages in thread
From: Josh Cartwright @ 2015-09-16 19:56 UTC (permalink / raw
  To: John Kacur; +Cc: Clark Williams, linux-rt-users, Gratian Crisan

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

On Wed, Sep 16, 2015 at 01:05:51AM +0200, John Kacur wrote:
> On Mon, 31 Aug 2015, Josh Cartwright wrote:
> > From: Gratian Crisan <gratian.crisan@ni.com>
> > 
> > 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 <gratian.crisan@ni.com>
> > Signed-off-by: Josh Cartwright <joshc@ni.com>
[..]
> 
> 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.

> 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 <gratian.crisan@ni.com>
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 <gratian.crisan@ni.com>
Signed-off-by: Josh Cartwright <joshc@ni.com>
---
 src/cyclictest/cyclictest.c | 96 ++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 37 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 62f7cd9..5c9600c 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -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)
-- 
2.5.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check
  2015-09-16 19:47     ` Josh Cartwright
@ 2015-09-17 19:15       ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-17 19:15 UTC (permalink / raw
  To: Josh Cartwright; +Cc: John Kacur, Clark Williams, linux-rt-users



On Wed, 16 Sep 2015, Josh Cartwright wrote:

> On Tue, Sep 15, 2015 at 11:50:16PM +0200, John Kacur wrote:
> > On Mon, 31 Aug 2015, Josh Cartwright wrote:
> > 
> > > 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 <joshc@ni.com>
> 
> [..]
> 
> > 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.

Sorry, there is some confusion here, and it's probably partly on my side.
We do of course still support building without NUMA. All that I was really 
referring to was, that if you are building on platforms that support NUMA,
we are making the development libs a requirement for compiling, in our 
distro spec files, even you are running without NUMA
But please continue to test with NUMA=0 builds as well as the default, and 
report / or fix any builds that don't work with NUMA=0

> 
> > 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 <joshc@ni.com>
> 
> s/Signed-off-by: /
> 

Oops, okay, fixing that msg.

> :)
> 
> > Signed-off-by: John Kacur <jkacur@redhat.com>
> > ---
> >  src/cyclictest/cyclictest.c |  7 ++-----
> >  src/cyclictest/rt_numa.h    | 18 +++++++++++++-----
> >  2 files changed, 15 insertions(+), 10 deletions(-)
> > 
> > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> > index 213c527fd714..b3abfcc67407 100644
> > --- a/src/cyclictest/cyclictest.c
> > +++ b/src/cyclictest/cyclictest.c
> > @@ -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.
> 
> >  #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,

You are of course correct here, but it's just one extra check at init 
time, so the impact is pretty close to zero.

Will consider more clean-ups in the future, but it's probably not worth 
obsessing about. Like I said, I like the api more as documentation for the 
process of setting numa up.

Thanks

John

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file
  2015-09-16 19:56     ` Josh Cartwright
@ 2015-09-17 19:40       ` John Kacur
  0 siblings, 0 replies; 25+ messages in thread
From: John Kacur @ 2015-09-17 19:40 UTC (permalink / raw
  To: Josh Cartwright
  Cc: John Kacur, Clark Williams, linux-rt-users, Gratian Crisan



On Wed, 16 Sep 2015, Josh Cartwright wrote:

> On Wed, Sep 16, 2015 at 01:05:51AM +0200, John Kacur wrote:
> > On Mon, 31 Aug 2015, Josh Cartwright wrote:
> > > From: Gratian Crisan <gratian.crisan@ni.com>
> > > 
> > > 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 <gratian.crisan@ni.com>
> > > Signed-off-by: Josh Cartwright <joshc@ni.com>
> [..]
> > 
> > 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.
> 
> > 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 <gratian.crisan@ni.com>
> 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 <gratian.crisan@ni.com>
> Signed-off-by: Josh Cartwright <joshc@ni.com>
> ---
>  src/cyclictest/cyclictest.c | 96 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 59 insertions(+), 37 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 62f7cd9..5c9600c 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -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)
> -- 
> 2.5.1
> 
> 

Signed-off-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-09-17 19:40 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-31 16:35 [PATCH rt-tests 0/9] more rt-tests cleanups and a cyclictest feature Josh Cartwright
2015-08-31 16:35 ` [PATCH rt-tests 1/9] cyclictest: consistently make all functions 'static' Josh Cartwright
2015-09-02 13:03   ` John Kacur
2015-08-31 16:35 ` [PATCH rt-tests 2/9] cyclictest: fixup documentation for --priority option Josh Cartwright
2015-09-01 21:03   ` John Kacur
2015-09-01 21:11     ` Josh Cartwright
2015-08-31 16:35 ` [PATCH rt-tests 3/9] cyclictest: drop unnecessary numa_on_and_available() check Josh Cartwright
2015-09-15 21:50   ` John Kacur
2015-09-16 19:47     ` Josh Cartwright
2015-09-17 19:15       ` John Kacur
2015-08-31 16:35 ` [PATCH rt-tests 4/9] signaltest: drop unused tsnorm() Josh Cartwright
2015-09-15 22:00   ` John Kacur
2015-09-16 19:48     ` Josh Cartwright
2015-08-31 16:35 ` [PATCH rt-tests 5/9] cyclictest: use correct type when allocating cpu bitmask size Josh Cartwright
2015-09-15 22:19   ` John Kacur
2015-08-31 16:35 ` [PATCH rt-tests 6/9] cyclictest: drop impossible use_fifo conditional Josh Cartwright
2015-09-15 22:24   ` John Kacur
2015-08-31 16:35 ` [PATCH rt-tests 7/9] cyclictest: fail if use_fifo && thread creation failed Josh Cartwright
2015-09-15 22:25   ` John Kacur
2015-08-31 16:35 ` [PATCH rt-tests 8/9] error: mark fatal, err_exit, err_quit as being noreturn Josh Cartwright
2015-09-15 22:31   ` John Kacur
2015-08-31 16:35 ` [PATCH rt-tests 9/9] cyclictest: add option for dumping the histogram in a file Josh Cartwright
2015-09-15 23:05   ` John Kacur
2015-09-16 19:56     ` Josh Cartwright
2015-09-17 19:40       ` John Kacur

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.