[PATCH 0/2] cyclictest: Restore CPU affinity for non-NUMA builds

[PATCH 0/2] cyclictest: Restore CPU affinity for non-NUMA builds

[Gary S. Robertson]

From: "Gary S. Robertson" <gary.robertson@linaro.org>

These patches restore the ability to use the legacy CPU affinity functionality in cyclictest when built without NUMA support.  When cyclictest was patched to add bitmask CPU affinity support with NUMA V2, it broke the existing CPU affinity support for non-NUMA builds.  That patch mapped the legacy CPU affinity behavior (choosing a single core or else all cores) onto the bit-mapped framework when cyclictest was built against NUMA V1 libraries.  I simply extended this mapping to include also the case where cyclictest was built with no NUMA support.

Unfortunately the addition of the extra conditional compilation clauses into the function definitions from that patch made the resulting source code difficult to follow - so I also re-organized the code to make the end result more understandable.  I simply created conditional compilation clauses for the three NUMA-related build configurations (V2, V1, or none).  Then I created separate clean function definitions for each configuration... an extension of the method already used for the non-NUMA configuration case..

The restructuring of the code makes the patch containing changes to rt_numa.h difficult to follow.  If you prefer I can re-work these changes as three patches instead of two... splitting the rt_numa.h patch into one patch which re-organizes the code and a second which applies the changes that fix the CPU affinity bug.


Gary S. Robertson (2):
  Restore CPU affinity function for non-NUMA builds
  Don't offer --numa option when unavailable

 src/cyclictest/cyclictest.c |    2 +
 src/cyclictest/rt_numa.h    |  188 +++++++++++++++++++++++++++----------------
 2 files changed, 121 insertions(+), 69 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] Restore CPU affinity function for non-NUMA builds

[Gary S. Robertson]

From: "Gary S. Robertson" <gary.robertson@linaro.org>

Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org>
---
 src/cyclictest/rt_numa.h |  188 +++++++++++++++++++++++++++++-----------------
 1 file changed, 119 insertions(+), 69 deletions(-)

diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
index 31a2b16..13f5140 100644
--- a/src/cyclictest/rt_numa.h
+++ b/src/cyclictest/rt_numa.h
@@ -6,6 +6,12 @@
  * They should also work correctly with older versions of the numactl lib
  * such as the one found on RHEL5, or with the newer version 2 and above.
  *
+ * The difference in behavior hinges on whether LIBNUMA_API_VERSION >= 2,
+ * in which case we will employ the bitmask affinity behavior -or-
+ * either LIBNUMA_API_VERSION < 2 or NUMA support is missing altogether,
+ * in which case we retain the older affinity behavior which can either
+ * specify a single CPU core or else use all cores.
+ *
  * (C) 2010 John Kacur <jkacur@redhat.com>
  * (C) 2010 Clark Williams <williams@redhat.com>
  *
@@ -19,19 +25,17 @@
 
 static int numa = 0;
 
-#ifdef NUMA
-#include <numa.h>
-
-#ifndef LIBNUMA_API_VERSION
-#define LIBNUMA_API_VERSION 1
-#endif
-
-#if LIBNUMA_API_VERSION < 2
 struct bitmask {
 	unsigned long size; /* number of bits in the map */
 	unsigned long *maskp;
 };
 #define BITS_PER_LONG	(8*sizeof(long))
+
+#ifdef NUMA
+#include <numa.h>
+
+#ifndef LIBNUMA_API_VERSION
+#define LIBNUMA_API_VERSION 1
 #endif
 
 static void *
@@ -61,13 +65,21 @@ static void rt_numa_set_numa_run_on_node(int node, int cpu)
 	return;
 }
 
-static void numa_on_and_available()
+static void *rt_numa_numa_alloc_onnode(size_t size, int node, int cpu)
 {
-	if (numa && numa_available() == -1)
-		fatal("--numa specified and numa functions not available.\n");
+	void *stack;
+	stack = numa_alloc_onnode(size, node);
+	if (stack == NULL)
+		fatal("failed to allocate %d bytes on node %d for cpu %d\n",
+				size, node, cpu);
+	return stack;
 }
 
 #if LIBNUMA_API_VERSION >= 2
+
+/*
+ * Use new bit mask CPU affinity behavior
+ */
 static int rt_numa_numa_node_of_cpu(int cpu)
 {
 	int node;
@@ -77,8 +89,38 @@ static int rt_numa_numa_node_of_cpu(int cpu)
 	return node;
 }
 
+static inline unsigned int rt_numa_bitmask_isbitset( const struct bitmask *mask,
+	unsigned long i)
+{
+	return numa_bitmask_isbitset(mask,i);
+}
+
+static inline struct bitmask* rt_numa_parse_cpustring(const char* s,
+	int max_cpus) 
+{
+#ifdef HAVE_PARSE_CPUSTRING_ALL		/* Currently not defined anywhere.  No
+					   autotools build. */
+	return numa_parse_cpustring_all(s);
+#else
+	/* We really need numa_parse_cpustring_all(), so we can assign threads
+	 * to cores which are part of an isolcpus set, but early 2.x versions of
+	 * libnuma do not have this function.  A work around should be to run
+	 * your command with e.g. taskset -c 9-15 <command>
+	 */
+	return numa_parse_cpustring(s);
+#endif 
+}
+
+static inline void rt_bitmask_free(struct bitmask *mask)
+{
+	numa_bitmask_free(mask);
+}
+
 #else	/* LIBNUMA_API_VERSION == 1 */
 
+/*
+ * Map legacy CPU affinity behavior onto bit mask infrastructure
+ */
 static int rt_numa_numa_node_of_cpu(int cpu)
 {
 	unsigned char cpumask[256];
@@ -108,60 +150,16 @@ static int rt_numa_numa_node_of_cpu(int cpu)
 	return -1;
 }
 
-#endif	/* LIBNUMA_API_VERSION */
-
-static void *rt_numa_numa_alloc_onnode(size_t size, int node, int cpu)
-{
-	void *stack;
-	stack = numa_alloc_onnode(size, node);
-	if (stack == NULL)
-		fatal("failed to allocate %d bytes on node %d for cpu %d\n",
-				size, node, cpu);
-	return stack;
-}
-
-
 static inline unsigned int rt_numa_bitmask_isbitset( const struct bitmask *mask,
 	unsigned long i)
 {
-#if LIBNUMA_API_VERSION >= 2
-	return numa_bitmask_isbitset(mask,i);
-#else
 	long bit = mask->maskp[i/BITS_PER_LONG] & (1<<(i % BITS_PER_LONG));
 	return (bit != 0);
-#endif
-}
-
-/** Returns number of bits set in mask. */
-static inline unsigned int rt_numa_bitmask_count(const struct bitmask *mask)
-{
-	unsigned int num_bits = 0, i;
-	for (i = 0; i < mask->size; i++) {
-		if (rt_numa_bitmask_isbitset(mask, i))
-			num_bits++;
-	}
-	/* Could stash this instead of recomputing every time. */
-	return num_bits;
 }
 
 static inline struct bitmask* rt_numa_parse_cpustring(const char* s,
 	int max_cpus) 
 {
-#if LIBNUMA_API_VERSION >= 2
-
-#ifdef HAVE_PARSE_CPUSTRING_ALL		/* Currently not defined anywhere.  No
-					   autotools build. */
-	return numa_parse_cpustring_all(s);
-#else
-	/* We really need numa_parse_cpustring_all(), so we can assign threads
-	 * to cores which are part of an isolcpus set, but early 2.x versions of
-	 * libnuma do not have this function.  A work around should be to run
-	 * your command with e.g. taskset -c 9-15 <command>
-	 */
-	return numa_parse_cpustring(s);
-#endif 
-
-#else /* LIBNUMA_API_VERSION == 1 */
 	int cpu;
 	struct bitmask *mask = NULL;
 	cpu = atoi(s);
@@ -184,35 +182,87 @@ static inline struct bitmask* rt_numa_parse_cpustring(const char* s,
 		}
 	}
 	return mask;
-#endif
 }
 
 static inline void rt_bitmask_free(struct bitmask *mask)
 {
-#if LIBNUMA_API_VERSION >= 2
-	numa_bitmask_free(mask);
-#else /* LIBNUMA_API_VERSION == 1 */
 	free(mask->maskp);
 	free(mask);
-#endif
 }
 
+#endif	/* LIBNUMA_API_VERSION */
+
 #else /* ! NUMA */
-struct bitmask { };
 static inline void *threadalloc(size_t size, int n) { return malloc(size); }
 static inline void threadfree(void *ptr, size_t s, int n) { free(ptr); }
 static inline void rt_numa_set_numa_run_on_node(int n, int c) { }
-static inline void numa_on_and_available() { };
 static inline int rt_numa_numa_node_of_cpu(int cpu) { return -1; }
 static void *rt_numa_numa_alloc_onnode(size_t s, int n, int c) { return NULL; }
-static inline unsigned int rt_numa_bitmask_isbitset(
-	const struct bitmask *affinity_mask, unsigned long i) { return 0; }
-static inline struct bitmask* rt_numa_parse_cpustring(const char* s, int m) 
-{ return NULL; }
-static inline unsigned int rt_numa_bitmask_count(const struct bitmask *mask)
-{ return 0; }
-static inline void rt_bitmask_free(struct bitmask *mask) { return; }
+
+/*
+ * Map legacy CPU affinity behavior onto bit mask infrastructure
+ */
+static inline unsigned int rt_numa_bitmask_isbitset( const struct bitmask *mask,
+	unsigned long i)
+{
+	long bit = mask->maskp[i/BITS_PER_LONG] & (1<<(i % BITS_PER_LONG));
+	return (bit != 0);
+}
+
+static inline struct bitmask* rt_numa_parse_cpustring(const char* s,
+	int max_cpus) 
+{
+	int cpu;
+	struct bitmask *mask = NULL;
+	cpu = atoi(s);
+	if (0 <= cpu && cpu < max_cpus) {
+		mask = malloc(sizeof(*mask));
+		if (mask) {
+			/* Round up to integral number of longs to contain
+			 * max_cpus bits */
+			int nlongs = (max_cpus+BITS_PER_LONG-1)/BITS_PER_LONG;
+
+			mask->maskp = calloc(nlongs, sizeof(long));
+			if (mask->maskp) {
+				mask->maskp[cpu/BITS_PER_LONG] |=
+					(1UL << (cpu % BITS_PER_LONG));
+				mask->size = max_cpus;
+			} else {
+				free(mask);
+				mask = NULL;
+			}
+		}
+	}
+	return mask;
+}
+
+static inline void rt_bitmask_free(struct bitmask *mask)
+{
+	free(mask->maskp);
+	free(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)
+{
+	unsigned int num_bits = 0, i;
+	for (i = 0; i < mask->size; i++) {
+		if (rt_numa_bitmask_isbitset(mask, i))
+			num_bits++;
+	}
+	/* Could stash this instead of recomputing every time. */
+	return num_bits;
+}
+
 #endif	/* _RT_NUMA_H */
-- 
1.7.9.5

[PATCH 2/2] Don't offer --numa option when unavailable

[Gary S. Robertson]

From: "Gary S. Robertson" <gary.robertson@linaro.org>

Signed-off-by: Gary S. Robertson <gary.robertson@linaro.org>
---
 src/cyclictest/cyclictest.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 686a635..4532b19 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1032,8 +1032,10 @@ static void display_help(int error)
 	       "-T TRACE --tracer=TRACER   set tracing function\n"
 	       "    configured tracers: %s\n"
 	       "-u       --unbuffered      force unbuffered output for live processing\n"
+#ifdef NUMA
 	       "-U       --numa            Standard NUMA testing (similar to SMP option)\n"
 	       "                           thread data structures allocated from local node\n"
+#endif	/* NUMA */
 	       "-v       --verbose         output values on stdout for statistics\n"
 	       "                           format: n:c:v n=tasknum c=count v=value in us\n"
 	       "-w       --wakeup          task wakeup tracing (used with -b)\n"
-- 
1.7.9.5