| Message ID | 20201007085653.11961-1-dwagner@suse.de |
|---|---|
| Headers | show |
| Series | Streamline command line | expand |
On Wed, 7 Oct 2020, Daniel Wagner wrote: > In order to be reuse the cpumask parsing code, move it to libary. > Several tools implement their own version of this. Let's use one > global one instead. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > Makefile | 12 +++--- > src/cyclictest/cyclictest.c | 68 +++++++-------------------------- > src/include/rt-utils.h | 3 ++ > src/lib/rt-utils.c | 76 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 98 insertions(+), 61 deletions(-) > > diff --git a/Makefile b/Makefile > index c3ebbd7b2a2e..3627084437c1 100644 > --- a/Makefile > +++ b/Makefile > @@ -20,9 +20,14 @@ sources = cyclictest.c \ > ssdd.c \ > oslat.c > > +# You have to have libnuma installed, which is fine to do even if you are > +# running on non-numa machines > +CFLAGS += -DNUMA > +NUMA_LIBS = -lnuma > + > TARGETS = $(sources:.c=) > LIBS = -lrt -lpthread > -RTTESTLIB = -lrttest -L$(OBJDIR) > +RTTESTLIB = -lrttest -L$(OBJDIR) $(NUMA_LIBS) Currently only cyclictest was compiled with NUMA_LIBS, this change will compile everything with NUMA_LIBS. I checked the size of the programs, and they don't grow that much with this change, but they are small programs to begin with, do we want to keep this functionality separate? > EXTRA_LIBS ?= -ldl # for get_cpu > DESTDIR ?= > prefix ?= /usr/local > @@ -79,11 +84,6 @@ ostype := $(lastword $(subst -, ,$(dumpmachine))) > machinetype := $(shell echo $(dumpmachine)| \ > sed -e 's/-.*//' -e 's/i.86/i386/' -e 's/mips.*/mips/' -e 's/ppc.*/powerpc/') > > -# You have to have libnuma installed, which is fine to do even if you are > -# running on non-numa machines > -CFLAGS += -DNUMA > -NUMA_LIBS = -lnuma > - > include src/arch/android/Makefile > > VPATH = src/cyclictest: > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index b41d42f13f24..02cb92813fb7 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -908,11 +908,6 @@ static int clocksources[] = { > CLOCK_REALTIME, > }; > > -static unsigned int is_cpumask_zero(const struct bitmask *mask) > -{ > - return (rt_numa_bitmask_count(mask) == 0); > -} > - > /* Get available cpus according to getaffinity or according to the > * intersection of getaffinity and the user specified affinity > * in the case of AFFINITY_SPECIFIED, the function has to be called > @@ -997,52 +992,6 @@ static int cpu_for_thread_ua(int thread_num, int max_cpus) > return 0; > } > > - > -/* After this function is called, affinity_mask is the intersection of the user > - * supplied affinity mask and the affinity mask from the run time environment */ > -static void use_current_cpuset(const int max_cpus) > -{ > - int i; > - pid_t pid; > - struct bitmask *curmask; > - > - pid = getpid(); > - > - curmask = numa_allocate_cpumask(); > - numa_sched_getaffinity(pid, curmask); > - > - /* Clear bits that are not set in both the cpuset from the environment, > - * and in the user specified affinity for cyclictest */ > - for (i=0; i < max_cpus; i++) { > - if ((!rt_numa_bitmask_isbitset(affinity_mask, i)) || (!rt_numa_bitmask_isbitset(curmask, i))) { > - numa_bitmask_clearbit(affinity_mask, i); > - } > - } > - > - numa_bitmask_free(curmask); > -} > - > -static void parse_cpumask(const char *option, const int max_cpus) > -{ > - affinity_mask = rt_numa_parse_cpustring(option, max_cpus); > - if (affinity_mask) { > - if (is_cpumask_zero(affinity_mask)) { > - rt_bitmask_free(affinity_mask); > - affinity_mask = NULL; > - } else { > - use_current_cpuset(max_cpus); > - } > - > - } > - if (!affinity_mask) > - display_help(1); > - > - if (verbose) { > - printf("%s: Using %u cpus.\n", __func__, > - rt_numa_bitmask_count(affinity_mask)); > - } > -} > - > static void handlepolicy(char *polname) > { > if (strncasecmp(polname, "other", 5) == 0) > @@ -1176,15 +1125,24 @@ static void process_options (int argc, char *argv[], int max_cpus) > if (smp) > break; > numa_initialize(); > - if (optarg != NULL) { > - parse_cpumask(optarg, max_cpus); > + if (optarg) { > + parse_cpumask(optarg, max_cpus, &affinity_mask); > setaffinity = AFFINITY_SPECIFIED; > - } else if (optind<argc && (atoi(argv[optind]) || argv[optind][0] == '0' || argv[optind][0] == '!')) { > - parse_cpumask(argv[optind], max_cpus); > + } else if (optind < argc && > + (atoi(argv[optind]) || > + argv[optind][0] == '0' || > + argv[optind][0] == '!')) { > + parse_cpumask(argv[optind], max_cpus, &affinity_mask); > setaffinity = AFFINITY_SPECIFIED; > } else { > setaffinity = AFFINITY_USEALL; > } > + > + if (setaffinity == AFFINITY_SPECIFIED && !affinity_mask) > + display_help(1); > + if (verbose) > + printf("Using %u cpus.\n", > + numa_bitmask_weight(affinity_mask)); > break; > case 'A': > case OPT_ALIGNED: > diff --git a/src/include/rt-utils.h b/src/include/rt-utils.h > index 4ed1cbc53ece..3b97655a20f2 100644 > --- a/src/include/rt-utils.h > +++ b/src/include/rt-utils.h > @@ -3,6 +3,7 @@ > #define __RT_UTILS_H > > #include <stdint.h> > +#include <numa.h> > > #define _STR(x) #x > #define STR(x) _STR(x) > @@ -28,6 +29,8 @@ uint32_t string_to_policy(const char *str); > pid_t gettid(void); > > int parse_time_string(char *val); > +int parse_mem_string(char *str, uint64_t *val); > +int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask); > > void enable_trace_mark(void); > void tracemark(char *fmt, ...) __attribute__((format(printf, 1, 2))); > diff --git a/src/lib/rt-utils.c b/src/lib/rt-utils.c > index f786588706cd..302b91388df5 100644 > --- a/src/lib/rt-utils.c > +++ b/src/lib/rt-utils.c > @@ -362,6 +362,82 @@ int parse_time_string(char *val) > return t; > } > > +int parse_mem_string(char *str, uint64_t *val) > +{ > + char *endptr; > + int v = strtol(str, &endptr, 10); > + > + if (!*endptr) > + return v; > + > + switch (*endptr) { > + case 'g': > + case 'G': > + v *= 1024; > + case 'm': > + case 'M': > + v *= 1024; > + case 'k': > + case 'K': > + v *= 1024; > + case 'b': > + case 'B': > + break; > + default: > + return -1; > + } > + > + *val = v; > + > + return 0; > +} > + > +/* > + * After this function is called, affinity_mask is the intersection of > + * the user supplied affinity mask and the affinity mask from the run > + * time environment > + */ > +static void use_current_cpuset(int max_cpus, struct bitmask *cpumask) > +{ > + struct bitmask *curmask; > + int i; > + > + curmask = numa_allocate_cpumask(); > + numa_sched_getaffinity(getpid(), curmask); > + > + /* > + * Clear bits that are not set in both the cpuset from the > + * environment, and in the user specified affinity. > + */ > + for (i = 0; i < max_cpus; i++) { > + if ((!numa_bitmask_isbitset(cpumask, i)) || > + (!numa_bitmask_isbitset(curmask, i))) > + numa_bitmask_clearbit(cpumask, i); > + } > + > + numa_bitmask_free(curmask); > +} > + > +int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask) > +{ > + struct bitmask *mask; > + > + mask = numa_parse_cpustring_all(str); > + if (!mask) > + return -ENOMEM; > + > + if (numa_bitmask_weight(mask) == 0) { > + numa_bitmask_free(mask); > + *cpumask = NULL; > + return 0; > + } > + > + use_current_cpuset(max_cpus, mask); > + *cpumask = mask; > + > + return 0; > +} > + > void open_tracemark_fd(void) > { > char path[MAX_PATH]; > -- > 2.28.0 > > This is a good idea, I'd just prefer to keep the libraries separate for now. John
On Wed, 7 Oct 2020, Daniel Wagner wrote: > The numa library has support for counting the bits in the affinity > mask and how many CPUs are available and usable by the tasks. Let's > use those helpers. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/cyclictest/cyclictest.c | 22 +++------------------- > 1 file changed, 3 insertions(+), 19 deletions(-) > > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index 02cb92813fb7..d7477c686435 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -915,26 +915,10 @@ static int clocksources[] = { > */ > static int get_available_cpus(void) > { > - int num_cpus = 0; > - int res; > - pthread_t thread; > - cpu_set_t cpuset; > - > - > - if (affinity_mask != NULL) { > - num_cpus = rt_numa_bitmask_count(affinity_mask); > - } else { > - CPU_ZERO(&cpuset); > - thread = pthread_self(); > - res = pthread_getaffinity_np(thread, sizeof(cpu_set_t), &cpuset); > - if (res != 0) { > - fatal("pthread_getaffinity_np failed: %s\n", strerror(res)); > - } > - num_cpus = CPU_COUNT(&cpuset); > - } > - > - return num_cpus; > + if (affinity_mask) > + return numa_bitmask_weight(affinity_mask); > > + return numa_num_task_cpus(); > } > > /* cpu_for_thread AFFINITY_SPECIFIED */ > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com> Thanks!
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/sched_deadline/deadline_test.8 | 37 ++++++++++++----------- > src/sched_deadline/deadline_test.c | 47 +++++++++++++++--------------- > 2 files changed, 41 insertions(+), 43 deletions(-) > > diff --git a/src/sched_deadline/deadline_test.8 b/src/sched_deadline/deadline_test.8 > index 5dc99c0bfc63..8f32c5b6feb6 100644 > --- a/src/sched_deadline/deadline_test.8 > +++ b/src/sched_deadline/deadline_test.8 > @@ -19,45 +19,44 @@ deadline_test \- High resolution test program > This program is used to test the deadline scheduler (SCHED_DEADLINE tasks) > .SH SYNOPSIS > .B deadline_test > -.RI "[ \-hb ] [ \-r prio ] [ \-c cpulist ] [ \-i interval ] [ \-p percent ] [ \-P percent ] [ \-t threads ] [ \-s step[us) ]" > - > +.RI "[ \-hb ] [ \-c CPUSET ] [ \-i INTV ] [ \-p PERCENT ] [ \-P PERCENT ] \ > +[ \-r PRIO ] [ \-s STEP ] [ \-t NUM ]" > .SH OPTIONS > .TP > -.B \-h > -Show this help menu > -.br > -.TP > .B \-b > Bind on the last cpu. (shortcut for -c <lastcpu>) > .br > .TP > -.B \-r prio > -Add an RT task with given prio to stress system > +.B \-c CPUSET > +Comma/hyphen separated list of CPUs to run deadline tasks on > .br > .TP > -.B \-c cpulist > -Comma/hyphen separated list of CPUs to run deadline tasks on > +.B \-h > +Show this help menu > .br > .TP > -.B \-i interval > +.B \-i INTV > The shortest deadline for the tasks > .br > .TP > -.B \-p percent > +.B \-p PERCENT > The percent of bandwidth to use (1-90%) > .br > .TP > -.B \-P percent > -The percent of runtime for execution completion > - (Default 100%) > +.B \-P PERCENT > +The percent of runtime for execution completion (default 100%) > .br > .TP > -.B \-t threads > -The number of threads to run as deadline (default 1) > +.B \-r PRIO > +Add an RT task with given prio to stress system > .br > .TP > -.B \-s step(us) > -The amount to increase the deadline for each task (default 500us) > +.B \-s STEP > +The amount to increase the deadline for each task in us (default 500us) > +.br > +.TP > +.B \-t NUM > +The number of threads to run as deadline (default 1) > .br > .SH AUTHOR > Deadline test was written by Steven Rostedt <rostedt@goodmis.org> > diff --git a/src/sched_deadline/deadline_test.c b/src/sched_deadline/deadline_test.c > index 4cef2609912e..060ac896aef2 100644 > --- a/src/sched_deadline/deadline_test.c > +++ b/src/sched_deadline/deadline_test.c > @@ -46,33 +46,30 @@ > > /** > * usage - show the usage of the program and exit. > - * @argv: The program passed in args > + * @error: Exit error code to be used > * > - * This is defined here to show people looking at this code how > + * This is defined here to show peoplde looking at this code how > * to use this program as well. > */ > -static void usage(char **argv) > +static void usage(int error) > { > - char *arg = argv[0]; > - char *p = arg+strlen(arg); > - > - while (p >= arg && *p != '/') > - p--; > - p++; > - > - printf("usage: %s [options]\n" > - " -h - Show this help menu\n" > - " -b - Bind on the last cpu. (shortcut for -c <lastcpu>)\n" > - " -r prio - Add an RT task with given prio to stress system\n" > - " -c cpulist - Comma/hyphen separated list of CPUs to run deadline tasks on\n" > - " -i interval - The shortest deadline for the tasks\n" > - " -p percent - The percent of bandwidth to use (1-90%%)\n" > - " -P percent - The percent of runtime for execution completion\n" > - " (Default 100%%)\n" > - " -t threads - The number of threads to run as deadline (default 1)\n" > - " -s step(us) - The amount to increase the deadline for each task (default 500us)\n" > - "\n", p); > - exit(-1); > + printf("deadline_test V %1.2f\n", VERSION); > + printf("Usage:\n" > + "deadline_test <options>\n" > + "-b Bind on the last cpu. (shortcut for -c <lastcpu>)\n" > + "-c CPUSET Comma/hyphen separated list of CPUs to run deadline\n" > + " tasks on\n" > + "-h Show this help menu\n" > + "-i INTV The shortest deadline for the tasks\n" > + "-p PERCENT The percent of bandwidth to use (1-90%%)\n" > + "-P PERCENT The percent of runtime for execution completion\n" > + " (default 100%%)\n" > + "-r PRIO Add an RT task with given prio to stress system\n" > + "-s STEP The amount to increase the deadline for each task in us\n" > + " (default 500us)\n" > + "-t NUM The number of threads to run as deadline (default 1)\n" > + ); > + exit(error); > } > > #define _STR(x) #x > @@ -1753,8 +1750,10 @@ int main (int argc, char **argv) > rt_task = atoi(optarg); > break; > case 'h': > + usage(0); > + break; > default: > - usage(argv); > + usage(1); > } > } > > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com Thanks!
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/oslat/oslat.8 | 2 +- > src/oslat/oslat.c | 78 ++++++++++++++++++++++------------------------- > 2 files changed, 38 insertions(+), 42 deletions(-) > > diff --git a/src/oslat/oslat.8 b/src/oslat/oslat.8 > index 85f2c5bcdf5c..b481d8d82783 100644 > --- a/src/oslat/oslat.8 > +++ b/src/oslat/oslat.8 > @@ -36,7 +36,7 @@ Total memory usage will be this value multiplies 2*N, > because there will be src/dst buffers for each thread, and > N is the number of processors for testing. > .TP > -.B \-t, \-\-runtime=SEC > +.B \-D, \-\-duration=TIME > Specify test duration, e.g., 60, 20m, 2H (m/M: minutes, h/H: hours, d/D: days). > By default the unit is s/second. > .TP > diff --git a/src/oslat/oslat.c b/src/oslat/oslat.c > index f1a82f2367d2..fafbdd694326 100644 > --- a/src/oslat/oslat.c > +++ b/src/oslat/oslat.c > @@ -507,42 +507,37 @@ static void handle_alarm(int code) > g.cmd = STOP; > } > > -const char *helpmsg = > -"Usage: %s [options]\n" > -"\n" > -"This is an OS latency detector by running busy loops on specified cores.\n" > -"Please run this tool using root.\n" > -"\n" > -"Available options:\n" > -"\n" > -" -b, --bucket-size Specify the number of the buckets (4-1024)\n" > -" -B, --bias Add a bias to all the buckets using the estimated mininum\n" > -" -c, --cpu-list Specify CPUs to run on, e.g. '1,3,5,7-15'\n" > -" -C, --cpu-main-thread Specify which CPU the main thread runs on. Default is cpu0.\n" > -" -f, --rtprio Using SCHED_FIFO priority (1-99)\n" > -" -m, --workload-mem Size of the memory to use for the workload (e.g., 4K, 1M).\n" > -" Total memory usage will be this value multiplies 2*N,\n" > -" because there will be src/dst buffers for each thread, and\n" > -" N is the number of processors for testing.\n" > -" -s, --single-preheat Use a single thread when measuring latency at preheat stage\n" > -" NOTE: please make sure the CPU frequency on all testing cores\n" > -" are locked before using this parmater. If you don't know how\n" > -" to lock the freq then please don't use this parameter.\n" > -" -t, --runtime Specify test duration, e.g., 60, 20m, 2H\n" > -" (m/M: minutes, h/H: hours, d/D: days)\n" > -" -T, --trace-threshold Stop the test when threshold triggered (in us),\n" > -" print a marker in ftrace and stop ftrace too.\n" > -" -v, --version Display the version of the software.\n" > -" -w, --workload Specify a kind of workload, default is no workload\n" > -" (options: no, memmove)\n" > -" -z, --zero-omit Don't display buckets in the output histogram if all zeros.\n" > -"\n" > -; > - > -static void usage(void) > +static void usage(int error) > { > - printf(helpmsg, g.app_name); > - exit(1); > + printf("oslat V %1.2f\n", VERSION); > + printf("Usage:\n" > + "oslat <options>\n\n" > + "This is an OS latency detector by running busy loops on specified cores.\n" > + "Please run this tool using root.\n\n" > + "Available options:\n\n" > + "-b, --bucket-size Specify the number of the buckets (4-1024)\n" > + "-B, --bias Add a bias to all the buckets using the estimated mininum\n" > + "-c, --cpu-list Specify CPUs to run on, e.g. '1,3,5,7-15'\n" > + "-C, --cpu-main-thread Specify which CPU the main thread runs on. Default is cpu0.\n" > + "-D, --duration Specify test duration, e.g., 60, 20m, 2H\n" > + " (m/M: minutes, h/H: hours, d/D: days)\n" > + "-f, --rtprio Using SCHED_FIFO priority (1-99)\n" > + "-m, --workload-mem Size of the memory to use for the workload (e.g., 4K, 1M).\n" > + " Total memory usage will be this value multiplies 2*N,\n" > + " because there will be src/dst buffers for each thread, and\n" > + " N is the number of processors for testing.\n" > + "-s, --single-preheat Use a single thread when measuring latency at preheat stage\n" > + " NOTE: please make sure the CPU frequency on all testing cores\n" > + " are locked before using this parmater. If you don't know how\n" > + " to lock the freq then please don't use this parameter.\n" > + "-T, --trace-threshold Stop the test when threshold triggered (in us),\n" > + " print a marker in ftrace and stop ftrace too.\n" > + "-v, --version Display the version of the software.\n" > + "-w, --workload Specify a kind of workload, default is no workload\n" > + " (options: no, memmove)\n" > + "-z, --zero-omit Don't display buckets in the output histogram if all zeros.\n" > + ); > + exit(error); > } > > /* TODO: use libnuma? */ > @@ -661,7 +656,7 @@ static void parse_options(int argc, char *argv[]) > { "bucket-size", required_argument, NULL, 'b' }, > { "cpu-list", required_argument, NULL, 'c' }, > { "cpu-main-thread", required_argument, NULL, 'C'}, > - { "runtime", required_argument, NULL, 't' }, > + { "duration", required_argument, NULL, 'D' }, > { "rtprio", required_argument, NULL, 'f' }, > { "help", no_argument, NULL, 'h' }, > { "trace-threshold", required_argument, NULL, 'T' }, > @@ -673,7 +668,7 @@ static void parse_options(int argc, char *argv[]) > { "version", no_argument, NULL, 'v'}, > { NULL, 0, NULL, 0 }, > }; > - int i, c = getopt_long(argc, argv, "b:Bc:C:f:hm:st:w:T:vz", > + int i, c = getopt_long(argc, argv, "b:Bc:C:D:f:hm:sw:T:vz", > options, NULL); > long ncores; > > @@ -704,7 +699,7 @@ static void parse_options(int argc, char *argv[]) > exit(1); > } > break; > - case 't': > + case 'D': > g.runtime = parse_runtime(optarg); > if (!g.runtime) { > printf("Illegal runtime: %s\n", optarg); > @@ -761,8 +756,11 @@ static void parse_options(int argc, char *argv[]) > case 'z': > g.output_omit_zero_buckets = 1; > break; > + case 'h': > + usage(0); > + break; > default: > - usage(); > + usage(1); > break; > } > } > @@ -821,8 +819,6 @@ int main(int argc, char *argv[]) > /* Run the main thread on cpu0 by default */ > g.cpu_main_thread = 0; > > - printf("\nVersion: %1.2f\n\n", VERSION); > - > parse_options(argc, argv); > > TEST(mlockall(MCL_CURRENT | MCL_FUTURE) == 0); > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com> Thanks In the future at least put one line in the text though, not just a title.
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Use available parse_time_string() instead locally implemented > version. While at it move the mem parser helper to the global utility > header. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/oslat/oslat.c | 67 ++--------------------------------------------- > 1 file changed, 2 insertions(+), 65 deletions(-) > > diff --git a/src/oslat/oslat.c b/src/oslat/oslat.c > index fafbdd694326..9df1a58f640d 100644 > --- a/src/oslat/oslat.c > +++ b/src/oslat/oslat.c > @@ -570,69 +570,6 @@ static int parse_cpu_list(char *cpu_list, cpu_set_t *cpu_set) > return n_cores; > } > > -static int parse_runtime(const char *str) > -{ > - char *endptr; > - int v = strtol(str, &endptr, 10); > - > - if (!*endptr) > - return v; > - > - switch (*endptr) { > - case 'd': > - case 'D': > - /* Days */ > - v *= 24; > - case 'h': > - case 'H': > - /* Hours */ > - v *= 60; > - case 'm': > - case 'M': > - /* Minutes */ > - v *= 60; > - case 's': > - case 'S': > - /* Seconds */ > - break; > - default: > - printf("Unknown runtime suffix: %s\n", endptr); > - v = 0; > - break; > - } > - > - return v; > -} > - > -static int parse_mem_size(char *str, uint64_t *val) > -{ > - char *endptr; > - int v = strtol(str, &endptr, 10); > - > - if (!*endptr) > - return v; > - > - switch (*endptr) { > - case 'g': > - case 'G': > - v *= 1024; > - case 'm': > - case 'M': > - v *= 1024; > - case 'k': > - case 'K': > - v *= 1024; > - case 'b': > - case 'B': > - break; > - default: > - return -1; > - } > - > - *val = v; > - > - return 0; > -} > > static int workload_select(char *name) > { > @@ -700,7 +637,7 @@ static void parse_options(int argc, char *argv[]) > } > break; > case 'D': > - g.runtime = parse_runtime(optarg); > + g.runtime = parse_time_string(optarg); > if (!g.runtime) { > printf("Illegal runtime: %s\n", optarg); > exit(1); > @@ -734,7 +671,7 @@ static void parse_options(int argc, char *argv[]) > } > break; > case 'm': > - if (parse_mem_size(optarg, &g.workload_mem_size)) { > + if (parse_mem_string(optarg, &g.workload_mem_size)) { > printf("Unknown workload memory size '%s'.\n\n", optarg); > exit(1); > } > -- > 2.28.0 > > This one is pending either your changes that I requested to the first patch, or an argument as to why I should accept the first patch. Cheers! John
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/ptsematest/ptsematest.8 | 5 ++- > src/ptsematest/ptsematest.c | 76 +++++++++++++++++++------------------ > 2 files changed, 44 insertions(+), 37 deletions(-) > > diff --git a/src/ptsematest/ptsematest.8 b/src/ptsematest/ptsematest.8 > index 5ee7d694d50d..05a0cdfd3657 100644 > --- a/src/ptsematest/ptsematest.8 > +++ b/src/ptsematest/ptsematest.8 > @@ -4,7 +4,7 @@ > \fBptsematest\fR \- Start two threads and measure the latency of interprocess communication with POSIX mutex. > .SH "SYNOPSIS" > .LP > -ptsematest [-a|--affinity [PROC]] [-b|--breaktrace USEC] [-d|--distance DIST] [-D|--duration TIME][-i|--interval INTV] [-l|--loops LOOPS] [-p|--prio PRIO] [-S|--smp] [-t|--threads [NUM]] > +ptsematest [-a|--affinity [PROC]] [-b|--breaktrace USEC] [-d|--distance DIST] [-D|--duration TIME] [-h|--help] [-i|--interval INTV] [-l|--loops LOOPS] [-p|--prio PRIO] [-S|--smp] [-t|--threads [NUM]] > .br > .SH "DESCRIPTION" > .LP > @@ -26,6 +26,9 @@ Specify a length for the test run. > .br > Append 'm', 'h', or 'd' to specify minutes, hours or days. > .TP > +.B \-h, \-\-help > +Print help message. > +.TP > .B \-i, \-\-interval=INTV > Set the base interval of the thread(s) in microseconds (default is 1000 us). This sets the interval of the first thread. See also -d. > .TP > diff --git a/src/ptsematest/ptsematest.c b/src/ptsematest/ptsematest.c > index de8ea2fe1b0c..02919e3f5eef 100644 > --- a/src/ptsematest/ptsematest.c > +++ b/src/ptsematest/ptsematest.c > @@ -141,29 +141,31 @@ void *semathread(void *param) > } > > > -static void display_help(void) > +static void display_help(int error) > { > printf("ptsematest V %1.2f\n", VERSION); > - puts("Usage: ptsematest <options>"); > - puts("Function: test POSIX threads mutex latency"); > - puts( > - "Options:\n" > - "-a [NUM] --affinity run thread #N on processor #N, if possible\n" > - " with NUM pin all threads to the processor NUM\n" > - "-b USEC --breaktrace=USEC send break trace command when latency > USEC\n" > - "-d DIST --distance=DIST distance of thread intervals in us default=500\n" > - "-D --duration=TIME specify a length for the test run.\n" > - " Append 'm', 'h', or 'd' to specify minutes, hours or days.\n" > - "-i INTV --interval=INTV base interval of thread in us default=1000\n" > - "-l LOOPS --loops=LOOPS number of loops: default=0(endless)\n" > - "-p PRIO --prio=PRIO priority\n" > - "-S --smp SMP testing: options -a -t and same priority\n" > - " of all threads\n" > - "-t --threads one thread per available processor\n" > - "-t [NUM] --threads=NUM number of threads:\n" > - " without NUM, threads = max_cpus\n" > - " without -t default = 1\n"); > - exit(1); > + printf("Usage:\n" > + "ptsematest <options>\n\n" > + "Function: test POSIX threads mutex latency\n\n" > + "Available options:\n" > + "-a [NUM] --affinity run thread #N on processor #N, if possible\n" > + " with NUM pin all threads to the processor NUM\n" > + "-b USEC --breaktrace=USEC send break trace command when latency > USEC\n" > + "-d DIST --distance=DIST distance of thread intervals in us default=500\n" > + "-D --duration=TIME specify a length for the test run.\n" > + " Append 'm', 'h', or 'd' to specify minutes, hours or\n" > + " days.\n" > + "-i INTV --interval=INTV base interval of thread in us default=1000\n" > + "-l LOOPS --loops=LOOPS number of loops: default=0(endless)\n" > + "-p PRIO --prio=PRIO priority\n" > + "-S --smp SMP testing: options -a -t and same priority\n" > + " of all threads\n" > + "-t --threads one thread per available processor\n" > + "-t [NUM] --threads=NUM number of threads:\n" > + " without NUM, threads = max_cpus\n" > + " without -t default = 1\n" > + ); > + exit(0); > } > > > @@ -188,16 +190,16 @@ static void process_options (int argc, char *argv[]) > int option_index = 0; > /** Options for getopt */ > static struct option long_options[] = { > - {"affinity", optional_argument, NULL, 'a'}, > - {"breaktrace", required_argument, NULL, 'b'}, > - {"distance", required_argument, NULL, 'd'}, > - {"interval", required_argument, NULL, 'i'}, > - {"loops", required_argument, NULL, 'l'}, > - {"duration", required_argument, NULL, 'D'}, > - {"priority", required_argument, NULL, 'p'}, > - {"smp", no_argument, NULL, 'S'}, > - {"threads", optional_argument, NULL, 't'}, > - {"help", no_argument, NULL, '?'}, > + {"affinity", optional_argument, NULL, 'a'}, > + {"breaktrace", required_argument, NULL, 'b'}, > + {"distance", required_argument, NULL, 'd'}, > + {"duration", required_argument, NULL, 'D'}, > + {"help", no_argument, NULL, 'h'}, > + {"interval", required_argument, NULL, 'i'}, > + {"loops", required_argument, NULL, 'l'}, > + {"priority", required_argument, NULL, 'p'}, > + {"smp", no_argument, NULL, 'S'}, > + {"threads", optional_argument, NULL, 't'}, > {NULL, 0, NULL, 0} > }; > int c = getopt_long (argc, argv, "a::b:d:i:l:D:p:St::h", > @@ -222,9 +224,11 @@ static void process_options (int argc, char *argv[]) > break; > case 'b': tracelimit = atoi(optarg); break; > case 'd': distance = atoi(optarg); break; > + case 'D': duration = parse_time_string(optarg); break; > case 'i': interval = atoi(optarg); break; > + case '?': > + case 'h': display_help(0); break; > case 'l': max_cycles = atoi(optarg); break; > - case 'D': duration = parse_time_string(optarg); break; > case 'p': priority = atoi(optarg); break; > case 'S': > smp = 1; > @@ -243,9 +247,9 @@ static void process_options (int argc, char *argv[]) > else > num_threads = max_cpus; > break; > - case 'h': > - display_help(); > - case '?': error = 1; break; > + default: > + display_help(1); > + break; > } > } > > @@ -275,7 +279,7 @@ static void process_options (int argc, char *argv[]) > sameprio = 1; > > if (error) > - display_help(); > + display_help(error); > } > > > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com>
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/rt-migrate-test/rt-migrate-test.8 | 41 +++++++------- > src/rt-migrate-test/rt-migrate-test.c | 82 +++++++++++++-------------- > 2 files changed, 59 insertions(+), 64 deletions(-) > > diff --git a/src/rt-migrate-test/rt-migrate-test.8 b/src/rt-migrate-test/rt-migrate-test.8 > index b9c07f1ed80a..5bbacb5333b8 100644 > --- a/src/rt-migrate-test/rt-migrate-test.8 > +++ b/src/rt-migrate-test/rt-migrate-test.8 > @@ -1,11 +1,11 @@ > .\" > -.TH RT-MIGRATE-TEST 8 "April 21, 2016" > +.TH RT-MIGRATE-TEST 8 "September 18, 2020" > .\" Please adjust this date whenever editing this manpage > .SH NAME > rt-migrate-test \- real-time task migration program > .SH SYNOPSIS > -.B rt-migrate-test > -.RI "[\-ceh] [\-p prio] [\-r time] [\-s time] [\-m time] [\-l loops] [nr_tasks] > +.LP > +rt-migrate-test [-c|--check] [-D|--duration TIME] [-e|--equal] [-h|--help] [-l|--loops LOOPS] [-m|--maxerr TIME] [-p|--prio PRIO] [-r|--run-time TIME] [-s|--sleep-time TIME] [NR_TASKS] > .SH DESCRIPTION > Test real-time multiprocessor scheduling of tasks to ensure the highest priority tasks are running on all available CPUs > .SH OPTIONS > @@ -15,44 +15,43 @@ This program follows the usual GNU command line syntax, with long options starti > In the summary of options, a value in brackets (), indicates a default value > .br > .TP > +.B \-c, \-\-check > +Stop if lower prio task is quicker than higher (off) > +.TP > .B \-D, \-\-duration=TIME > Specify a length for the test run. > .br > Append 'm', 'h', or 'd' to specify minutes, hours or days. > .TP > -.B \-p, \-\-prio=prio > -base priority to start RT tasks with (2) > +.B \-e, \-\-equal > +Use equal prio for #CPU-1 tasks (requires > 2 CPUS) > .br > .TP > -.B \-r, \-\-run\-time=time > -Run time (ms) to busy loop the threads (20) > +.B \-h, \-\-help > +Display usage > .br > .TP > -.B \-s, \-\-sleep\-time=time > -Sleep time (ms) between intervals (100) > +.B \-l \-\-loops=LOOPS > +Number of iterations to run (50) > .br > .TP > -.B \-m, \-\-maxerr=time > +.B \-m, \-\-maxerr=TIME > Max allowed error (microsecs) > .br > .TP > -.B \-l \-\-loops=loops > -Number of iterations to run (50) > -.br > -.TP > -.B \-e > -Use equal prio for #CPU-1 tasks (requires > 2 CPUS) > +.B \-p, \-\-prio=PRIO > +base priority to start RT tasks with (2) > .br > .TP > -.B \-c, \-\-check > -Stop if lower prio task is quicker than higher (off) > +.B \-r, \-\-run\-time=TIME > +Run time (ms) to busy loop the threads (20) > .br > .TP > -.B \-h, \-\-help > -Display usage > +.B \-s, \-\-sleep\-time=TIME > +Sleep time (ms) between intervals (100) > .br > .TP > -.B [nr\-tasks] > +.B [NR_TASKS] > number of tasks to run (number of cpus + 1) > .br > .SH AUTHOR > diff --git a/src/rt-migrate-test/rt-migrate-test.c b/src/rt-migrate-test/rt-migrate-test.c > index 4863238edeb4..68824606faf1 100644 > --- a/src/rt-migrate-test/rt-migrate-test.c > +++ b/src/rt-migrate-test/rt-migrate-test.c > @@ -146,71 +146,67 @@ static void print_progress_bar(int percent) > fflush(stderr); > } > > -static void usage(char **argv) > +static void usage(int error) > { > - char *arg = argv[0]; > - char *p = arg+strlen(arg); > - > - while (p >= arg && *p != '/') > - p--; > - p++; > - > - printf("%s %1.2f\n", p, VERSION); > + printf("rt-migrate-test %1.2f\n", VERSION); > printf("Usage:\n" > - "%s <options> nr_tasks\n\n" > - "-p prio --prio prio base priority to start RT tasks with (2)\n" > - "-r time --run-time time Run time (ms) to busy loop the threads (20)\n" > - "-s time --sleep-time time Sleep time (ms) between intervals (100)\n" > - "-m time --maxerr time Max allowed error (microsecs)\n" > - "-l loops --loops loops Number of iterations to run (50)\n" > - "-D --duration=TIME specify a length for the test run.\n" > - " Append 'm', 'h', or 'd' to specify minutes, hours or days.\n" > - "-e Use equal prio for #CPU-1 tasks (requires > 2 CPUS)\n" > - "-c --check Stop if lower prio task is quicker than higher (off)\n" > - "-h --help\n" > - " () above are defaults \n", > - p); > - exit(0); > + "rt-migrate-test <options> [NR_TASKS]\n\n" > + "-c --check Stop if lower prio task is quicker than higher (off)\n" > + "-D TIME --duration=TIME Specify a length for the test run.\n" > + " Append 'm', 'h', or 'd' to specify minutes, hours or\n" > + " days.\n" > + "-e --equal Use equal prio for #CPU-1 tasks (requires > 2 CPUS)\n" > + "-h --help Print this help message\n" > + "-l LOOPS --loops=LOOPS Number of iterations to run (50)\n" > + "-m TIME --maxerr=TIME Max allowed error (microsecs)\n" > + "-p PRIO --prio=PRIO base priority to start RT tasks with (2)\n" > + "-r TIME --run-time=TIME Run time (ms) to busy loop the threads (20)\n" > + "-s TIME --sleep-time=TIME Sleep time (ms) between intervals (100)\n\n" > + " () above are defaults \n" > + ); > + exit(error); > } > > -static void parse_options (int argc, char *argv[]) > +static void parse_options(int argc, char *argv[]) > { > for (;;) { > int option_index = 0; > /** Options for getopt */ > static struct option long_options[] = { > - {"prio", required_argument, NULL, 'p'}, > - {"run-time", required_argument, NULL, 'r'}, > - {"sleep-time", required_argument, NULL, 's'}, > - {"maxerr", required_argument, NULL, 'm'}, > - {"loops", required_argument, NULL, 'l'}, > - {"duration", required_argument, NULL, 'D'}, > - {"check", no_argument, NULL, 'c'}, > - {"help", no_argument, NULL, '?'}, > + {"check", no_argument, NULL, 'c'}, > + {"duration", required_argument, NULL, 'D'}, > + {"equal", no_argument, NULL, 'e'}, > + {"help", no_argument, NULL, 'h'}, > + {"loops", required_argument, NULL, 'l'}, > + {"maxerr", required_argument, NULL, 'm'}, > + {"prio", required_argument, NULL, 'p'}, > + {"run-time", required_argument, NULL, 'r'}, > + {"sleep-time", required_argument, NULL, 's'}, > {NULL, 0, NULL, 0} > }; > - int c = getopt_long (argc, argv, "p:r:s:m:l:D:ech", > + int c = getopt_long(argc, argv, "cD:ehl:m:p:r:s:", > long_options, &option_index); > if (c == -1) > break; > switch (c) { > - case 'p': prio_start = atoi(optarg); break; > - case 'r': > - run_interval = atoi(optarg); > - break; > - case 's': interval = atoi(optarg); break; > - case 'l': nr_runs = atoi(optarg); break; > + case 'c': check = 1; break; > case 'D': duration = parse_time_string(optarg); break; > - case 'm': max_err = usec2nano(atoi(optarg)); break; > case 'e': equal = 1; break; > - case 'c': check = 1; break; > case '?': > case 'h': > - usage(argv); > + usage(0); > break; > + case 'l': nr_runs = atoi(optarg); break; > + case 'm': max_err = usec2nano(atoi(optarg)); break; > + case 'p': prio_start = atoi(optarg); break; > + case 'r': > + run_interval = atoi(optarg); > + break; > + case 's': interval = atoi(optarg); break; > + default: > + usage(1); > } > } > - > } > > static unsigned long long get_time(void) > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com>
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/ssdd/ssdd.8 | 35 +++++++++++++++++----------------- > src/ssdd/ssdd.c | 50 +++++++++++++++++++++++++++---------------------- > 2 files changed, 45 insertions(+), 40 deletions(-) > > diff --git a/src/ssdd/ssdd.8 b/src/ssdd/ssdd.8 > index 99f30145d079..5c29d0abb732 100644 > --- a/src/ssdd/ssdd.8 > +++ b/src/ssdd/ssdd.8 > @@ -1,31 +1,30 @@ > -.TH SSDD 8 "June 13, 2019" > +.TH SSDD 8 "September 19, 2020" > .SH NAME > ssdd \- have a tracer do a bunch of PTRACE_SINGLESTEPs > .SH SYNOPSIS > -.B ssdd > -.RI "<options>" > +.LP > +ssdd [-f|--forks NUM] [-h|--help] [-i|--iters NUM] > .SH DESCRIPTION > -Have a tracer do a bunch of PTRACE_SINGLESTEPs against > -a tracee as fast as possible. Create several of these > -tracer/tracee pairs and see if they can be made to > -interfere with each other. > -The tracer waits on each PTRACE_SINGLESTEP with a waitpid(2) > -and checks that waitpid's return values for correctness. > +Have a tracer do a bunch of PTRACE_SINGLESTEPs against a tracee as > +fast as possible. Create several of these tracer/tracee pairs and > +see if they can be made to interfere with each other. The tracer > +waits on each PTRACE_SINGLESTEP with a waitpid(2) and checks that > +waitpid's return values for correctness. > .SH OPTIONS > -.B \-f, \-\-forks > -number of tracer/tracee pairs to fork off. > -Default is 10. > -.br > .TP > -.B \-i, \-\-iters > -number of PTRACE_SINGLESTEP iterations to > -do before declaring success, for each tracer/ > -tracee pair set up. Default is 10,000. > +.B \-f, \-\-forks=NUM > +number of tracer/tracee pairs to fork off. > .br > +Default is 10. > .TP > .B \-h, \-\-help > Display usage > - > +.TP > +.B \-i, \-\-iters=NUM > +number of PTRACE_SINGLESTEP iterations to do before declaring > +success, for each tracer tracee pair set up. > +.br > +Default is 10,000. > .SH AUTHOR > ssdd was written by Joe Korty <joe.korty@concurrent-rt.com> > .PP > diff --git a/src/ssdd/ssdd.c b/src/ssdd/ssdd.c > index f165da96e23a..66d6009dc572 100644 > --- a/src/ssdd/ssdd.c > +++ b/src/ssdd/ssdd.c > @@ -68,13 +68,16 @@ static int got_sigchld; > > enum option_value { OPT_NFORKS=1, OPT_NITERS, OPT_HELP }; > > -static void usage() > +static void usage(int error) > { > - printf("ssdd <options>\n"); > - printf("\t-f --forks=<number of forks>\n"); > - printf("\t-i --iters=<number of iterations>\n"); > - printf("\t-h --help\n"); > - exit(0); > + printf("ssdd V %1.2f\n", VERSION); > + printf("Usage:\n" > + "ssdd <options>\n\n" > + "-f --forks=NUM number of forks\n" > + "-h --help print this message\n" > + "-i --iters=NUM number of iterations\n" > + ); > + exit(error); > } > > static int do_wait(pid_t *wait_pid, int *ret_sig) > @@ -292,27 +295,30 @@ int main(int argc, char **argv) > int option_index = 0; > > static struct option long_options[] = { > - {"forks", required_argument, NULL, OPT_NFORKS}, > - {"iters", required_argument, NULL, OPT_NITERS}, > - {"help", no_argument, NULL, OPT_HELP}, > + {"forks", required_argument, NULL, OPT_NFORKS}, > + {"help", no_argument, NULL, OPT_HELP}, > + {"iters", required_argument, NULL, OPT_NITERS}, > {NULL, 0, NULL, 0}, > }; > - int c = getopt_long(argc, argv, "f:i:h", long_options, &option_index); > + int c = getopt_long(argc, argv, "f:hi:", long_options, &option_index); > if (c == -1) > break; > switch(c) { > - case 'f': > - case OPT_NFORKS: > - nforks = atoi(optarg); > - break; > - case 'i': > - case OPT_NITERS: > - nsteps = atoi(optarg); > - break; > - case 'h': > - case OPT_HELP: > - usage(); > - break; > + case 'f': > + case OPT_NFORKS: > + nforks = atoi(optarg); > + break; > + case 'h': > + case OPT_HELP: > + usage(0); > + break; > + case 'i': > + case OPT_NITERS: > + nsteps = atoi(optarg); > + break; > + default: > + usage(1); > + break; > } > } > > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com>
On Wed, 7 Oct 2020, Daniel Wagner wrote: > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/hackbench/hackbench.8 | 70 ++++++++++++++------------ > src/hackbench/hackbench.c | 102 ++++++++++++++++++-------------------- > 2 files changed, 86 insertions(+), 86 deletions(-) > > diff --git a/src/hackbench/hackbench.8 b/src/hackbench/hackbench.8 > index aaf572359704..4c2c8ad9cb1a 100644 > --- a/src/hackbench/hackbench.8 > +++ b/src/hackbench/hackbench.8 > @@ -1,13 +1,17 @@ > -.TH "hackbench" "8" "February 23, 2010" "" "" > +.TH "hackbench" "8" "September 19, 2020" "" "" > .SH "NAME" > hackbench \- scheduler benchmark/stress test > .SH "SYNOPSIS" > .B hackbench > -.RI "[\-p|\-\-pipe] [\-s|\-\-datasize " <bytes> "] " > -.RI "[\-l|\-\-loops " <num\-loops> "] " > -.RI "[\-g|\-\-groups "<num\-groups> "] " > -.RI "[\-f|\-\-fds <num\-fds>] " > -.RI "[\-T|\-\-threads] [\-P|\-\-process] [\-F|\-\-fifo] [\-\-help]" > +.RI "[\-f|\-\-fds NUM] " > +.RI "[\-F|\-\-fifo] " > +.RI "[\-g|\-\-groups NUM] " > +.RI "[\-h|\-\-help] " > +.RI "[\-l|\-\-loops LOOPS] " > +.RI "[\-p|\-\-pipe] " > +.RI "[\-s|\-\-datasize SIZE] " > +.RI "[\-T|\-\-threads]" > +.RI "[\-P|\-\-process]" > > .SH "DESCRIPTION" > Hackbench is both a benchmark and a stress test for the Linux kernel > @@ -19,45 +23,45 @@ each pair to send data back and forth. > .SH "OPTIONS" > These programs follow the usual GNU command line syntax, with long > options starting with two dashes ("\-\-"). > -.br > +.br > A summary of options is included below. > -.TP > +.TP > +.B \-f, \-\-fds=NUM > +Defines how many file descriptors each child should use. > +Note that the effective number will be twice the amount you set here, > +as the sender and receiver children will each open the given amount of > +file descriptors. > +.TP > +.B \-F,\-\-fifo > +Change the main thread to SCHED_FIFO after creating workers. > +This allows the management thread to run after many workers are created. > +.TP > +.B \-g, \-\-groups=NUM > +Defines how many groups of senders and receivers should be started > +.TP > +.B \-h, \-\-help > +.TP > +.B \-l, \-\-loops=LOOPS > +How many messages each sender/receiver pair should send > +.TP > .B \-p, \-\-pipe > Sends the data via a pipe instead of the socket (default) > -.TP > -.B \-s, \-\-datasize=<size in bytes> > +.TP > +.B \-s, \-\-datasize=SIZE > Sets the amount of data to send in each message > -.TP > -.B \-l, \-\-loops=<number of loops> > -How many messages each sender/receiver pair should send > -.TP > -.B \-g, \-\-groups=<number of groups> > -Defines how many groups of senders and receivers should be started > -.TP > -.B \-f, \-\-fds=<number of file descriptors> > -Defines how many file descriptors each child should use. > -Note that the effective number will be twice the amount you set here, > -as the sender and receiver children will each open the given amount of > -file descriptors. > -.TP > +.TP > .B \-T, \-\-threads > Each sender/receiver child will be a POSIX thread of the parent. > -.TP > +.TP > .B \-P, \-\-process > Hackbench will use fork() on all children (default behaviour) > -.TP > -.B \-F,\-\-fifo > -Change the main thread to SCHED_FIFO after creating workers. > -This allows the management thread to run after many workers are created. > -.TP > -.B \-\-help > -.br > +.br > Shows a simple help screen > .SH "EXAMPLES" > -.LP > +.LP > Running hackbench without any options will give default behaviour, > using fork() and sending data between senders and receivers via sockets. > -.LP > +.LP > user@host: ~ $ hackbench > .br > Running in process mode with 10 groups using 40 file descriptors each (== 400 tasks) > diff --git a/src/hackbench/hackbench.c b/src/hackbench/hackbench.c > index 2cddff654df6..268c23233004 100644 > --- a/src/hackbench/hackbench.c > +++ b/src/hackbench/hackbench.c > @@ -88,12 +88,22 @@ static void barf(const char *msg) > exit(1); > } > > -static void print_usage_exit() > +static void print_usage_exit(int error) > { > - printf("Usage: hackbench [-p|--pipe] [-s|--datasize <bytes>] [-l|--loops <num loops>]\n" > - "\t\t [-g|--groups <num groups] [-f|--fds <num fds>]\n" > - "\t\t [-T|--threads] [-P|--process] [-F|--fifo] [--help]\n"); > - exit(1); > + printf("hackbench V %1.2f\n", VERSION); > + printf("Usage:\n" > + "hackbench <options>\n\n" > + "-f --fds=NUM number of fds\n" > + "-F --fifo use SCHED_FIFO for main thread\n" > + "-g --groups=NUM number of groups to be used\n" > + "-h --help print this message\n" > + "-l --loops=LOOPS how many message should be send\n" > + "-p --pipe send data via a pipe\n" > + "-s --datasize=SIZE message size\n" > + "-T --threads use POSIX threads\n" > + "-P --process use fork (default)\n" > + ); > + exit(error); > } > > static void fdpair(int fds[2]) > @@ -342,86 +352,72 @@ static unsigned int group(childinfo_t *child, > return num_fds * 2; > } > > -static void process_options (int argc, char *argv[]) > +static void process_options(int argc, char *argv[]) > { > - int error = 0; > - > - while( 1 ) { > + for(;;) { > int optind = 0; > > static struct option longopts[] = { > - {"pipe", no_argument, NULL, 'p'}, > - {"datasize", required_argument, NULL, 's'}, > - {"loops", required_argument, NULL, 'l'}, > - {"groups", required_argument, NULL, 'g'}, > - {"fds", required_argument, NULL, 'f'}, > - {"threads", no_argument, NULL, 'T'}, > - {"processes", no_argument, NULL, 'P'}, > - {"fifo", no_argument, NULL, 'F'}, > - {"help", no_argument, NULL, 'h'}, > + {"fds", required_argument, NULL, 'f'}, > + {"fifo", no_argument, NULL, 'F'}, > + {"groups", required_argument, NULL, 'g'}, > + {"help", no_argument, NULL, 'h'}, > + {"loops", required_argument, NULL, 'l'}, > + {"pipe", no_argument, NULL, 'p'}, > + {"datasize", required_argument, NULL, 's'}, > + {"threads", no_argument, NULL, 'T'}, > + {"processes", no_argument, NULL, 'P'}, > {NULL, 0, NULL, 0} > }; > > - int c = getopt_long(argc, argv, "ps:l:g:f:TPFh", > + int c = getopt_long(argc, argv, "f:Fg:hl:ps:TP", > longopts, &optind); > if (c == -1) { > break; > } > switch (c) { > - case 'p': > - use_pipes = 1; > + case 'f': > + if (!(argv[optind] && (num_fds = atoi(optarg)) > 0)) { > + fprintf(stderr, "%s: --fds|-f requires an integer > 0\n", argv[0]); > + print_usage_exit(1); > + } > break; > - > - case 's': > - if (!(argv[optind] && (datasize = atoi(optarg)) > 0)) { > - fprintf(stderr, "%s: --datasize|-s requires an integer > 0\n", argv[0]); > - error = 1; > + case 'F': > + fifo = 1; > + break; > + case 'g': > + if (!(argv[optind] && (num_groups = atoi(optarg)) > 0)) { > + fprintf(stderr, "%s: --groups|-g requires an integer > 0\n", argv[0]); > + print_usage_exit(1); > } > break; > - > + case 'h': > + print_usage_exit(0); > case 'l': > if (!(argv[optind] && (loops = atoi(optarg)) > 0)) { > fprintf(stderr, "%s: --loops|-l requires an integer > 0\n", argv[0]); > - error = 1; > + print_usage_exit(1); > } > break; > - > - case 'g': > - if (!(argv[optind] && (num_groups = atoi(optarg)) > 0)) { > - fprintf(stderr, "%s: --groups|-g requires an integer > 0\n", argv[0]); > - error = 1; > - } > + case 'p': > + use_pipes = 1; > break; > - > - case 'f': > - if (!(argv[optind] && (num_fds = atoi(optarg)) > 0)) { > - fprintf(stderr, "%s: --fds|-f requires an integer > 0\n", argv[0]); > - error = 1; > + case 's': > + if (!(argv[optind] && (datasize = atoi(optarg)) > 0)) { > + fprintf(stderr, "%s: --datasize|-s requires an integer > 0\n", argv[0]); > + print_usage_exit(1); > } > break; > - > case 'T': > process_mode = THREAD_MODE; > break; > case 'P': > process_mode = PROCESS_MODE; > break; > - > - case 'F': > - fifo = 1; > - break; > - > - case 'h': > - print_usage_exit(); > - > default: > - error = 1; > + print_usage_exit(1); > } > } > - > - if( error ) { > - exit(1); > - } > } > > void sigcatcher(int sig) { > -- > 2.28.0 > > Signed-off-by: John Kacur <jkacur@redhat.com>
Hi John, On Fri, Oct 23, 2020 at 11:46:54AM -0400, John Kacur wrote: > > TARGETS = $(sources:.c=) > > LIBS = -lrt -lpthread > > -RTTESTLIB = -lrttest -L$(OBJDIR) > > +RTTESTLIB = -lrttest -L$(OBJDIR) $(NUMA_LIBS) > > Currently only cyclictest was compiled with NUMA_LIBS, this change will > compile everything with NUMA_LIBS. I checked the size of the programs, and > they don't grow that much with this change, but they are small programs to > begin with, do we want to keep this functionality separate? My thinking is, that the most important program for testing seems to be cyclictest. Everyone will run cyclictest on the target platform. Thus libnuma will be available. So there wont be any new unresolved dependencies. I traded the size increase for simplification in the code base and build setup. Looking at the actual increase (x86_64, stripped) is not too bad: program old new diff --------------------------------------------------- cyclicdeadline 35488 35552 64 0.18% cyclictest 57632 57632 0 0.0% deadline_test 43712 43776 64 0.15% hackbench 19168 19168 0 0.0% oslat 36040 36072 32 0.089% pip_stress 27296 27360 64 0.23% pi_stress 44296 48456 4160 9.4% pmqtest 31864 31928 64 0.2% ptsematest 31752 31816 64 0.2% queuelat 14600 14600 0 0.0% rt-migrate-test 31696 31728 32 0.1% signaltest 31712 31776 64 0.2% sigwaittest 31792 31856 64 0.2% ssdd 14744 14744 0 0.0% svsematest 31856 31920 64 0.2% pi_stress is a bit odd though. Not sure what's happening there. Will look into it. So I would prefer to go this route and makes things simpler in the code base. Thanks, Daniel
On Fri, Oct 23, 2020 at 01:25:56PM -0400, John Kacur wrote: > This one is pending either your changes that I requested to the first > patch, or an argument as to why I should accept the first patch. Ok. Let me know what you think about my argument in my reply to the first patch.
On Mon, 26 Oct 2020, Daniel Wagner wrote: > Hi John, > > On Fri, Oct 23, 2020 at 11:46:54AM -0400, John Kacur wrote: > > > TARGETS = $(sources:.c=) > > > LIBS = -lrt -lpthread > > > -RTTESTLIB = -lrttest -L$(OBJDIR) > > > +RTTESTLIB = -lrttest -L$(OBJDIR) $(NUMA_LIBS) > > > > Currently only cyclictest was compiled with NUMA_LIBS, this change will > > compile everything with NUMA_LIBS. I checked the size of the programs, and > > they don't grow that much with this change, but they are small programs to > > begin with, do we want to keep this functionality separate? > > My thinking is, that the most important program for testing seems to be > cyclictest. Everyone will run cyclictest on the target platform. Thus > libnuma will be available. So there wont be any new unresolved > dependencies. > > I traded the size increase for simplification in the code base and build > setup. Looking at the actual increase (x86_64, stripped) is not too bad: > > > program old new diff > --------------------------------------------------- > cyclicdeadline 35488 35552 64 0.18% > cyclictest 57632 57632 0 0.0% > deadline_test 43712 43776 64 0.15% > hackbench 19168 19168 0 0.0% > oslat 36040 36072 32 0.089% > pip_stress 27296 27360 64 0.23% > pi_stress 44296 48456 4160 9.4% > pmqtest 31864 31928 64 0.2% > ptsematest 31752 31816 64 0.2% > queuelat 14600 14600 0 0.0% > rt-migrate-test 31696 31728 32 0.1% > signaltest 31712 31776 64 0.2% > sigwaittest 31792 31856 64 0.2% > ssdd 14744 14744 0 0.0% > svsematest 31856 31920 64 0.2% > > > pi_stress is a bit odd though. Not sure what's happening there. Will > look into it. > > So I would prefer to go this route and makes things simpler in the code > base. > > Thanks, > Daniel > You're really just simplifying the Makefile, not the code. :) Well, that and I guess this means I'm asking you to separate any common numa functionality into a separate lib. If you want you can just pull out parse_time_string(), and parse_mem_string() for now until we hash out what we want to do with the numa functionality later. Does that work for you? John
On Wed, Oct 07, 2020 at 10:56:46AM +0200, Daniel Wagner wrote:
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
Hi, Daniel,
This seems to have broken oslat with "oslat -v". Would you help recover the
old behavior somehow?
Thanks,
--
Peter Xu
On Wed, Feb 10, 2021 at 11:08:47AM -0500, Peter Xu wrote: > On Wed, Oct 07, 2020 at 10:56:46AM +0200, Daniel Wagner wrote: > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > > Hi, Daniel, > > This seems to have broken oslat with "oslat -v". Would you help recover the > old behavior somehow? To be explicit, I didn't mean only "oslat -v". It dumps nothing now which definitely is not right.. Also I explicitly dump version for every oslat run because I wanted that to be with the output, so when user reports issue with oslat we'll always know which version is it. So besides the "oslat -v" breakage, I'd really appreciate if you can add the version dump back with every oslat run, too. Thanks, -- Peter Xu
So this is only about oslat, right? I ask because you respone to the email for ptsematest.
On Wed, Feb 10, 2021 at 05:30:57PM +0100, Daniel Wagner wrote: > So this is only about oslat, right? I ask because you respone to the > email for ptsematest. Sorry I replied to the wrong subject - yes it's about oslat, only. Thanks, -- Peter Xu
--- a/src/oslat/oslat.c
+++ b/src/oslat/oslat.c
@@ -655,16 +655,10 @@ static void parse_options(int argc, char *argv[])
*/
g.single_preheat_thread = true;
break;
- case 'v':
- /*
- * Because we always dump the version even before parsing options,
- * what we need to do is to quit..
- */
- exit(0);
- break;
case 'z':
g.output_omit_zero_buckets = 1;
break;
+ case 'v':
case 'h':
usage(0);
break;
$ ./oslat -v
oslat V 1.10
Usage:
oslat <options>
This is an OS latency detector by running busy loops on specified cores.
Please run this tool using root.
Available options:
-b, --bucket-size Specify the number of the buckets (4-1024)
-B, --bias Add a bias to all the buckets using the estimated mininum
-c, --cpu-list Specify CPUs to run on, e.g. '1,3,5,7-15'
-C, --cpu-main-thread Specify which CPU the main thread runs on. Default is cpu0.
-D, --duration Specify test duration, e.g., 60, 20m, 2H
(m/M: minutes, h/H: hours, d/D: days)
-f, --rtprio Using SCHED_FIFO priority (1-99)
-m, --workload-mem Size of the memory to use for the workload (e.g., 4K, 1M).
Total memory usage will be this value multiplies 2*N,
because there will be src/dst buffers for each thread, and
N is the number of processors for testing.
-s, --single-preheat Use a single thread when measuring latency at preheat stage
NOTE: please make sure the CPU frequency on all testing cores
are locked before using this parmater. If you don't know how
to lock the freq then please don't use this parameter.
-T, --trace-threshold Stop the test when threshold triggered (in us),
print a marker in ftrace and stop ftrace too.
-v, --version Display the version of the software.
-w, --workload Specify a kind of workload, default is no workload
(options: no, memmove)
-z, --zero-omit Don't display buckets in the output histogram if all zeros.
The other tests print also the usage text which included the version if
you provided '--version'. So this would make it behave in the same way.
Good enough?
On Wed, Feb 10, 2021 at 05:35:37PM +0100, Daniel Wagner wrote: > --- a/src/oslat/oslat.c > +++ b/src/oslat/oslat.c > @@ -655,16 +655,10 @@ static void parse_options(int argc, char *argv[]) > */ > g.single_preheat_thread = true; > break; > - case 'v': > - /* > - * Because we always dump the version even before parsing options, > - * what we need to do is to quit.. > - */ > - exit(0); > - break; > case 'z': > g.output_omit_zero_buckets = 1; > break; > + case 'v': > case 'h': > usage(0); > break; > > > > $ ./oslat -v > oslat V 1.10 > Usage: > oslat <options> > > This is an OS latency detector by running busy loops on specified cores. > Please run this tool using root. > > Available options: > > -b, --bucket-size Specify the number of the buckets (4-1024) > -B, --bias Add a bias to all the buckets using the estimated mininum > -c, --cpu-list Specify CPUs to run on, e.g. '1,3,5,7-15' > -C, --cpu-main-thread Specify which CPU the main thread runs on. Default is cpu0. > -D, --duration Specify test duration, e.g., 60, 20m, 2H > (m/M: minutes, h/H: hours, d/D: days) > -f, --rtprio Using SCHED_FIFO priority (1-99) > -m, --workload-mem Size of the memory to use for the workload (e.g., 4K, 1M). > Total memory usage will be this value multiplies 2*N, > because there will be src/dst buffers for each thread, and > N is the number of processors for testing. > -s, --single-preheat Use a single thread when measuring latency at preheat stage > NOTE: please make sure the CPU frequency on all testing cores > are locked before using this parmater. If you don't know how > to lock the freq then please don't use this parameter. > -T, --trace-threshold Stop the test when threshold triggered (in us), > print a marker in ftrace and stop ftrace too. > -v, --version Display the version of the software. > -w, --workload Specify a kind of workload, default is no workload > (options: no, memmove) > -z, --zero-omit Don't display buckets in the output histogram if all zeros. > > > > The other tests print also the usage text which included the version if > you provided '--version'. So this would make it behave in the same way. > Good enough? It makes sense to make all binaries in rt-tests behave similarly, so regarding "oslat -v" even if it's not my preference, it's fine to me. Again - Would you consider also dump the version in the output as before even for a normal run? -- Peter Xu
> It makes sense to make all binaries in rt-tests behave similarly, so regarding > "oslat -v" even if it's not my preference, it's fine to me. We could also update all other tools to just print the version string for '--version'. I don't have a strong preference. My main concern is that I don't want the tools to differ in the usage pattern. BTW, I think I would drop '-v' as short command line option as for example cyclictest uses '-v' for verbose. > Again - Would you consider also dump the version in the output as before even > for a normal run? Again, I don't have a strong opinion here. Just the same argument as before, all tools should behave in the same way.