Move common code to helper libraries

Add helper to parse common memory prefixes on strings.

Signed-off-by Daniel Wagner <[email protected]>

---
 src/include/rt-utils.h |  1 +
 src/lib/rt-utils.c     | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

src/include/rt-utils.h +1 -0

index c6fb220cdb0a..39ddbd9eae29 100644
@@ -28,6 +28,7 @@ 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);
 
 void enable_trace_mark(void);
 void tracemark(char *fmt, ...) __attribute__((format(printf, 1, 2)));

src/lib/rt-utils.c +30 -0

index a30e1e4ce436..2d68d62cd875 100644
@@ -362,6 +362,36 @@ 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;
+}
+
 static void open_tracemark_fd(void)
 {
 	char path[MAX_PATH];

Signature · 1 line

2.29.2

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 <[email protected]>

---
 src/oslat/oslat.c | 67 ++---------------------------------------------
 1 file changed, 2 insertions(+), 65 deletions(-)

src/oslat/oslat.c +2 -65

index 0018c472e0c1..b2aa15001be2 100644
@@ -572,69 +572,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)
 {

@@ -702,7 +639,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);

@@ -736,7 +673,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);
 			}

Signature · 1 line

2.29.2

The NUMA library is always present for cyclictest, thus NUMA config is always defined. Remove deadcode.

Signed-off-by Daniel Wagner <[email protected]>

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

src/cyclictest/cyclictest.c +0 -5

index 3282ab1b856b..46d88ed155c5 100644
@@ -1327,14 +1327,9 @@ static void process_options(int argc, char *argv[], int max_cpus)
 
 	/* if smp wasn't requested, test for numa automatically */
 	if (!smp) {
-#ifdef NUMA
 		numa_initialize();
 		if (setaffinity == AFFINITY_UNSPECIFIED)
 			setaffinity = AFFINITY_USEALL;
-#else
-		warn("cyclictest was not built with the numa option\n");
-		numa = 0;
-#endif
 	}
 
 	if (option_affinity) {

Signature · 1 line

2.29.2

A common task is to parse the provided cpumask from the command line. Add a helper which uses libnuma. Since we don't want to add unnecessary dependencies for tests which don't need this helper create a new library containing the NUMA bits.

Signed-off-by Daniel Wagner <[email protected]>

---
 Makefile              | 18 +++++++-------
 src/include/rt-numa.h |  9 +++++++
 src/lib/rt-numa.c     | 56 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 9 deletions(-)
 create mode 100644 src/include/rt-numa.h
 create mode 100644 src/lib/rt-numa.c

Makefile +9 -9

index 24895b7f9697..3afdfd4d53a7 100644
@@ -24,6 +24,7 @@ TARGETS = $(sources:.c=)
 LIBS	= -lrt -lpthread
 RTTESTLIB = -lrttest -L$(OBJDIR)
 EXTRA_LIBS ?= -ldl	# for get_cpu
+RTTESTNUMA = -lrttestnuma -lnuma
 DESTDIR	?=
 prefix  ?= /usr/local
 bindir  ?= $(prefix)/bin

@@ -84,11 +85,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:

@@ -122,8 +118,8 @@ all: $(TARGETS) hwlatdetect get_cyclictest_snapshot | $(OBJDIR)
 # Include dependency files, automatically generate them if needed.
 -include $(addprefix $(OBJDIR)/,$(sources:.c=.d))
 
-cyclictest: $(OBJDIR)/cyclictest.o $(OBJDIR)/librttest.a
-	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBS) $(RTTESTLIB) $(NUMA_LIBS)
+cyclictest: $(OBJDIR)/cyclictest.o $(OBJDIR)/librttest.a $(OBJDIR)/librttestnuma.a
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBS) $(RTTESTLIB) $(RTTESTNUMA)
 
 cyclicdeadline: $(OBJDIR)/cyclicdeadline.o $(OBJDIR)/librttest.a
 	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBS) $(RTTESTLIB)

@@ -172,8 +168,8 @@ queuelat: $(OBJDIR)/queuelat.o $(OBJDIR)/librttest.a
 ssdd: $(OBJDIR)/ssdd.o $(OBJDIR)/librttest.a
 	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBS) $(RTTESTLIB)
 
-oslat: $(OBJDIR)/oslat.o $(OBJDIR)/librttest.a
-	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBS) $(RTTESTLIB) $(NUMA_LIBS)
+oslat: $(OBJDIR)/oslat.o $(OBJDIR)/librttest.a $(OBJDIR)/librttestnuma.a
+	$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBS) $(RTTESTLIB) $(RTTESTNUMA)
 
 %.8.gz: %.8
 	gzip -nc $< > $@

@@ -185,6 +181,10 @@ LIBOBJS =$(addprefix $(OBJDIR)/,error.o rt-get_cpu.o rt-sched.o rt-utils.o)
 $(OBJDIR)/librttest.a: $(LIBOBJS)
 	$(AR) rcs $@ $^
 
+LIBNUMAOBJS =$(addprefix $(OBJDIR)/,rt-numa.o)
+$(OBJDIR)/librttestnuma.a: $(LIBNUMAOBJS)
+	$(AR) rcs $@ $^
+
 CLEANUP  = $(TARGETS) *.o .depend *.*~ *.orig *.rej *.d *.a *.8.gz *.8.bz2
 CLEANUP += $(if $(wildcard .git), ChangeLog)
 

src/include/rt-numa.h new file +9 -0

new file mode 100644
index 000000000000..047c8b6257cc
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#ifndef __RT_NUMA_H
+#define __RT_NUMA_H
+
+#include <numa.h>
+
+int parse_cpumask(char *str, int max_cpus, struct bitmask **cpumask);
+
+#endif

src/lib/rt-numa.c new file +56 -0

new file mode 100644
index 000000000000..a52a56e8aadd
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2020 Daniel Wagner <[email protected]>
+ */
+
+#include <sys/types.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "rt-numa.h"
+
+/*
+ * 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;
+}

Signature · 1 line

2.29.2

Adding my name to the copyright in the new file src/lib/rt-numa.c as some of the functions moved here are ones that I've written or modified / maintained etc.

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

Also, wondering if we should combine the two rt-numa.h files into one?

Use the helper funtion from rt-numa.h to parse the cpumask from the command line.

Signed-off-by Daniel Wagner <[email protected]>

---
 src/cyclictest/cyclictest.c | 70 ++++++++-----------------------------
 1 file changed, 14 insertions(+), 56 deletions(-)

src/cyclictest/cyclictest.c +14 -56

index 46d88ed155c5..f10f064f7a8e 100644
@@ -36,6 +36,7 @@
 #include "rt_numa.h"
 
 #include "rt-utils.h"
+#include "rt-numa.h"
 
 #include <bionic.h>
 

@@ -905,11 +906,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

@@ -976,53 +972,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)

@@ -1156,15 +1105,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:

Signature · 1 line

2.29.2

Use the helper funtion from rt-numa.h to parse the cpumask from the command line.

Signed-off-by Daniel Wagner <[email protected]>

---
 src/cyclictest/cyclictest.c | 70 ++++++++-----------------------------
 1 file changed, 14 insertions(+), 56 deletions(-)

src/cyclictest/cyclictest.c +14 -56

index 46d88ed155c5..f10f064f7a8e 100644
@@ -36,6 +36,7 @@
 #include "rt_numa.h"
 
 #include "rt-utils.h"
+#include "rt-numa.h"
 
 #include <bionic.h>
 

@@ -905,11 +906,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

@@ -976,53 +972,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)

@@ -1156,15 +1105,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:

Signature · 1 line

2.29.2

Use the common parse_cpumask() helper and use struct bitmask directly instead transforming it into a CPU_SET first.

Signed-off-by Daniel Wagner <[email protected]>

---
 src/oslat/oslat.c | 55 +++++++++++++++--------------------------------
 1 file changed, 17 insertions(+), 38 deletions(-)

src/oslat/oslat.c +17 -38

index b2aa15001be2..5b7e0d5b5d5c 100644
@@ -42,6 +42,7 @@
 #include <sys/syscall.h>
 
 #include "rt-utils.h"
+#include "rt-numa.h"
 #include "error.h"
 
 #ifdef __GNUC__

@@ -542,37 +543,6 @@ static void usage(int error)
 	exit(error);
 }
 
-/* TODO: use libnuma? */
-static int parse_cpu_list(char *cpu_list, cpu_set_t *cpu_set)
-{
-	struct bitmask *cpu_mask;
-	int i, n_cores;
-
-	n_cores = sysconf(_SC_NPROCESSORS_CONF);
-
-	if (!cpu_list) {
-		for (i = 0; i < n_cores; i++)
-			CPU_SET(i, cpu_set);
-		return n_cores;
-	}
-
-	cpu_mask = numa_parse_cpustring_all(cpu_list);
-	if (cpu_mask) {
-		for (i = 0; i < n_cores; i++) {
-			if (numa_bitmask_isbitset(cpu_mask, i))
-				CPU_SET(i, cpu_set);
-		}
-		numa_bitmask_free(cpu_mask);
-	} else {
-		warn("Unknown cpu-list: %s, using all available cpus\n", cpu_list);
-		for (i = 0; i < n_cores; i++)
-			CPU_SET(i, cpu_set);
-	}
-
-	return n_cores;
-}
-
-
 static int workload_select(char *name)
 {
 	int i = 0;

@@ -745,15 +715,18 @@ int main(int argc, char *argv[])
 {
 	struct thread *threads;
 	int i, n_cores;
-	cpu_set_t cpu_set;
+	struct bitmask *cpu_set = NULL;
+	int max_cpus = sysconf(_SC_NPROCESSORS_ONLN);
 
 #ifdef FRC_MISSING
 	printf("This architecture is not yet supported. "
 	       "Please implement frc() function first for %s.\n", argv[0]);
 	return 0;
 #endif
-
-	CPU_ZERO(&cpu_set);
+	if (numa_available() == -1) {
+		printf("ERROR: Could not initialize libnuma\n");
+		exit(1);
+	}
 
 	g.app_name = argv[0];
 	g.rtprio = 0;

@@ -768,16 +741,22 @@ int main(int argc, char *argv[])
 
 	TEST(mlockall(MCL_CURRENT | MCL_FUTURE) == 0);
 
-	n_cores = parse_cpu_list(g.cpu_list, &cpu_set);
+	if (!g.cpu_list)
+		g.cpu_list = strdup("all");
+	if (parse_cpumask(g.cpu_list, max_cpus, &cpu_set))
+		exit(1);
+	n_cores = numa_bitmask_weight(cpu_set);
 
-	TEST(threads = calloc(1, CPU_COUNT(&cpu_set) * sizeof(threads[0])));
+	TEST(threads = calloc(1, n_cores * sizeof(threads[0])));
 	for (i = 0; i < n_cores; ++i)
-		if (CPU_ISSET(i, &cpu_set) && move_to_core(i) == 0)
+		if (numa_bitmask_isbitset(cpu_set, i) && move_to_core(i) == 0)
 			threads[g.n_threads_total++].core_i = i;
 
-	if (CPU_ISSET(0, &cpu_set) && g.rtprio)
+	if (numa_bitmask_isbitset(cpu_set, 0) && g.rtprio)
 		printf("WARNING: Running SCHED_FIFO workload on CPU 0 may hang the thread\n");
 
+	numa_bitmask_free(cpu_set);
+
 	TEST(move_to_core(g.cpu_main_thread) == 0);
 
 	signal(SIGALRM, handle_alarm);

Signature · 1 line

2.29.2

Use the common parse_cpumask() helper and use struct bitmask directly instead transforming it into a CPU_SET first.

Signed-off-by Daniel Wagner <[email protected]>

This seems to have broken oslat with --cpu-list...

xz-x1:rt-tests [master]$ sudo ./oslat -c 1-2

Total runtime:          1 seconds
Thread priority:        default
CPU list:               1-2
CPU for main thread:    0
Workload:               no
Workload mem:           0 (KiB)
Preheat cores:          1