[PATCH 0/3] Revert always use libnuma patches

[PATCH 0/3] Revert always use libnuma patches

[John Kacur]

libnuma is a build requirement but not a runtime requirement.

Test branch unstable/devel/no-numa-runtime

I would appreciate some testing before I push commits upstream

John


John Kacur (3):
  Revert "cyclictest: Use affinity_mask for steering thread placement"
  Revert "cyclictest: Always use libnuma"
  Revert "signaltest: Always use libnuma"

 src/cyclictest/cyclictest.c | 97 ++++++++++++++++++++++++-------------
 src/cyclictest/rt_numa.h    |  2 +
 src/signaltest/signaltest.c | 10 ++--
 3 files changed, 71 insertions(+), 38 deletions(-)

-- 
2.26.2

[PATCH 1/3] Revert "cyclictest: Use affinity_mask for steering thread placement"

[John Kacur]

This reverts commit 8305e65092deeaf900366c6250d4b319f6c274b6.

Reverting to put back the code that allows us to run on machines
without libnuma.

However, there were some ideas in this patch that were not directly
related to that and could be revisited.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 38 ++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index c3d45f3ae31b..e2753db39c02 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -892,6 +892,7 @@ static int interval = DEFAULT_INTERVAL;
 static int distance = -1;
 static struct bitmask *affinity_mask = NULL;
 static int smp = 0;
+static int setaffinity = AFFINITY_UNSPECIFIED;
 
 static int clocksources[] = {
 	CLOCK_MONOTONIC,
@@ -1019,12 +1020,22 @@ static void process_options(int argc, char *argv[], int max_cpus)
 				break;
 			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, &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:
@@ -1117,6 +1128,7 @@ static void process_options(int argc, char *argv[], int max_cpus)
 		case OPT_SMP: /* SMP testing */
 			smp = 1;
 			num_threads = -1; /* update after parsing */
+			setaffinity = AFFINITY_USEALL;
 			break;
 		case 't':
 		case OPT_THREADS:
@@ -1185,16 +1197,23 @@ static void process_options(int argc, char *argv[], int max_cpus)
 		use_nanosleep = MODE_CLOCK_NANOSLEEP;
 	}
 
+	/* if smp wasn't requested, test for numa automatically */
+	if (!smp) {
+		if (setaffinity == AFFINITY_UNSPECIFIED)
+			setaffinity = AFFINITY_USEALL;
+	}
+
 	if (option_affinity && smp) {
 		warn("-a ignored due to smp mode\n");
 		if (affinity_mask) {
 			numa_bitmask_free(affinity_mask);
 			affinity_mask = NULL;
 		}
+		setaffinity = AFFINITY_USEALL;
 	}
 
 	if (smi) {
-		if (affinity_mask)
+		if (setaffinity == AFFINITY_UNSPECIFIED)
 			fatal("SMI counter relies on thread affinity\n");
 
 		if (!has_smi_counter())
@@ -1777,7 +1796,7 @@ int main(int argc, char **argv)
 	}
 
 	/* Restrict the main pid to the affinity specified by the user */
-	if (affinity_mask) {
+	if (affinity_mask != NULL) {
 		int res;
 
 		errno = 0;
@@ -1940,13 +1959,18 @@ int main(int argc, char **argv)
 		if (status != 0)
 			fatal("error from pthread_attr_init for thread %d: %s\n", i, strerror(status));
 
-		if (affinity_mask)
+		switch (setaffinity) {
+		case AFFINITY_UNSPECIFIED: cpu = -1; break;
+		case AFFINITY_SPECIFIED:
 			cpu = cpu_for_thread_sp(i, max_cpus, affinity_mask);
-		else
+			if (verbose)
+				printf("Thread %d using cpu %d.\n", i, cpu);
+			break;
+		case AFFINITY_USEALL:
 			cpu = cpu_for_thread_ua(i, max_cpus);
-
-		if (verbose)
-			printf("Thread %d using cpu %d.\n", i, cpu);
+			break;
+		default: cpu = -1;
+		}
 
 		/* find the memory node associated with the cpu i */
 		node = rt_numa_numa_node_of_cpu(cpu);
-- 
2.26.2

[PATCH 2/3] Revert "cyclictest: Always use libnuma"

[John Kacur]

This reverts commit 512d2b74561d5bdab15cacb7b1931cd0aa01d6b5.

libnuma is a buildtime requirement but not a runtime environment

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 63 ++++++++++++++++++++-----------------
 src/cyclictest/rt_numa.h    |  2 ++
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index e2753db39c02..3e31937f7088 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1018,6 +1018,9 @@ static void process_options(int argc, char *argv[], int max_cpus)
 			/* smp sets AFFINITY_USEALL in OPT_SMP */
 			if (smp)
 				break;
+			if (numa_initialize())
+				fatal("Couldn't initialize libnuma");
+			numa = 1;
 			if (optarg) {
 				parse_cpumask(optarg, max_cpus, &affinity_mask);
 				setaffinity = AFFINITY_SPECIFIED;
@@ -1126,6 +1129,8 @@ static void process_options(int argc, char *argv[], int max_cpus)
 			use_system = MODE_SYS_OFFSET; break;
 		case 'S':
 		case OPT_SMP: /* SMP testing */
+			if (numa)
+				fatal("numa and smp options are mutually exclusive\n");
 			smp = 1;
 			num_threads = -1; /* update after parsing */
 			setaffinity = AFFINITY_USEALL;
@@ -1199,17 +1204,16 @@ static void process_options(int argc, char *argv[], int max_cpus)
 
 	/* if smp wasn't requested, test for numa automatically */
 	if (!smp) {
+		if (numa_initialize())
+			fatal("Couldn't initialize libnuma");
+		numa = 1;
 		if (setaffinity == AFFINITY_UNSPECIFIED)
 			setaffinity = AFFINITY_USEALL;
 	}
 
-	if (option_affinity && smp) {
-		warn("-a ignored due to smp mode\n");
-		if (affinity_mask) {
-			numa_bitmask_free(affinity_mask);
-			affinity_mask = NULL;
-		}
-		setaffinity = AFFINITY_USEALL;
+	if (option_affinity) {
+		if (smp)
+			warn("-a ignored due to smp mode\n");
 	}
 
 	if (smi) {
@@ -1778,12 +1782,6 @@ int main(int argc, char **argv)
 	int online_cpus = sysconf(_SC_NPROCESSORS_ONLN);
 	int i, ret = -1;
 	int status;
-	void *stack;
-	void *currstk;
-	size_t stksize;
-
-	if (numa_initialize())
-		fatal("Couldn't initialize libnuma");
 
 	process_options(argc, argv, max_cpus);
 
@@ -1972,27 +1970,34 @@ int main(int argc, char **argv)
 		default: cpu = -1;
 		}
 
-		/* find the memory node associated with the cpu i */
-		node = rt_numa_numa_node_of_cpu(cpu);
+		node = -1;
+		if (numa) {
+			void *stack;
+			void *currstk;
+			size_t stksize;
 
-		/* get the stack size set for this thread */
-		if (pthread_attr_getstack(&attr, &currstk, &stksize))
-			fatal("failed to get stack size for thread %d\n", i);
+			/* find the memory node associated with the cpu i */
+			node = rt_numa_numa_node_of_cpu(cpu);
 
-		/* if the stack size is zero, set a default */
-		if (stksize == 0)
-			stksize = PTHREAD_STACK_MIN * 2;
+			/* get the stack size set for this thread */
+			if (pthread_attr_getstack(&attr, &currstk, &stksize))
+				fatal("failed to get stack size for thread %d\n", i);
 
-		/*  allocate memory for a stack on appropriate node */
-		stack = rt_numa_numa_alloc_onnode(stksize, node, cpu);
+			/* if the stack size is zero, set a default */
+			if (stksize == 0)
+				stksize = PTHREAD_STACK_MIN * 2;
 
-		/* touch the stack pages to pre-fault them in */
-		memset(stack, 0, stksize);
+			/*  allocate memory for a stack on appropriate node */
+			stack = rt_numa_numa_alloc_onnode(stksize, node, cpu);
 
-		/* set the thread's stack */
-		if (pthread_attr_setstack(&attr, stack, stksize))
-			fatal("failed to set stack addr for thread %d to 0x%x\n",
-				i, stack+stksize);
+			/* touch the stack pages to pre-fault them in */
+			memset(stack, 0, stksize);
+
+			/* set the thread's stack */
+			if (pthread_attr_setstack(&attr, stack, stksize))
+				fatal("failed to set stack addr for thread %d to 0x%x\n",
+				      i, stack+stksize);
+		}
 
 		/* allocate the thread's parameter block  */
 		parameters[i] = par = threadalloc(sizeof(struct thread_param), node);
diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h
index 87f5d3b66458..4cbd979c21e8 100644
--- a/src/cyclictest/rt_numa.h
+++ b/src/cyclictest/rt_numa.h
@@ -13,6 +13,8 @@
 #include "rt-utils.h"
 #include "rt-error.h"
 
+static int numa = 0;
+
 #include <numa.h>
 
 static void *
-- 
2.26.2

[PATCH 3/3] Revert "signaltest: Always use libnuma"

[John Kacur]

This reverts commit 3079f1b10d086b878f52607035b328f450b8033e.

libnuma is a build-time requirement but not a runtime requirement

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/signaltest/signaltest.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/signaltest/signaltest.c b/src/signaltest/signaltest.c
index 1d1b070cf12b..4f8e7caea2c1 100644
--- a/src/signaltest/signaltest.c
+++ b/src/signaltest/signaltest.c
@@ -207,6 +207,7 @@ static int quiet;
 static int lockall;
 static struct bitmask *affinity_mask = NULL;
 static int smp = 0;
+static int numa = 0;
 static int setaffinity = AFFINITY_UNSPECIFIED;
 static char outfile[MAX_PATH];
 
@@ -222,7 +223,6 @@ static void process_options(int argc, char *argv[], unsigned int max_cpus)
 {
 	int option_affinity = 0;
 	int error = 0;
-	int numa = 0;
 
 	for (;;) {
 		int option_index = 0;
@@ -253,6 +253,8 @@ static void process_options(int argc, char *argv[], unsigned int max_cpus)
 			/* smp sets AFFINITY_USEALL in OPT_SMP */
 			if (smp)
 				break;
+			if (numa_initialize())
+				fatal("Couldn't initialize libnuma");
 			numa = 1;
 			if (optarg) {
 				parse_cpumask(optarg, max_cpus, &affinity_mask);
@@ -337,6 +339,9 @@ static void process_options(int argc, char *argv[], unsigned int max_cpus)
 
 	/* if smp wasn't requested, test for numa automatically */
 	if (!smp) {
+		if (numa_initialize())
+			fatal("Couldn't initialize libnuma");
+		numa = 1;
 		if (setaffinity == AFFINITY_UNSPECIFIED)
 			setaffinity = AFFINITY_USEALL;
 	}
@@ -412,9 +417,6 @@ int main(int argc, char **argv)
 	int status, cpu;
 	int max_cpus = sysconf(_SC_NPROCESSORS_ONLN);
 
-	if (numa_initialize())
-		fatal("Couldn't initialize libnuma");
-
 	process_options(argc, argv, max_cpus);
 
 	if (check_privs())
-- 
2.26.2

Re: [PATCH 0/3] Revert always use libnuma patches

[Christian Eggers]

On Friday, 19 February 2021, 19:47:58 CET, John Kacur wrote:
> libnuma is a build requirement but not a runtime requirement.
> 
> Test branch unstable/devel/no-numa-runtime
> 
> I would appreciate some testing before I push commits upstream
> 
Now it is the same situation as in v1.10: 
With the --smp option set, cyclictest runs on arm32. Otherwise I still get the 
"FATAL: Couldn't initialize libnuma" (newline missing!)

regards
Christian

Re: [PATCH 0/3] Revert always use libnuma patches

[John Kacur]

On Fri, 19 Feb 2021, Christian Eggers wrote:

> On Friday, 19 February 2021, 19:47:58 CET, John Kacur wrote:
> > libnuma is a build requirement but not a runtime requirement.
> > 
> > Test branch unstable/devel/no-numa-runtime
> > 
> > I would appreciate some testing before I push commits upstream
> > 
> Now it is the same situation as in v1.10: 
> With the --smp option set, cyclictest runs on arm32. Otherwise I still get the 
> "FATAL: Couldn't initialize libnuma" (newline missing!)
> 
> regards
> Christian
> 
> 
> 

I think I see the problem, the patch
rt-numa: Move thread placement code to rt-numa library

added the fatal if numa_failed.

Thanks for testing. I'll send a patch to put it back the old way.

John

Re: [PATCH 0/3] Revert always use libnuma patches

[Daniel Wagner]

On Fri, Feb 19, 2021 at 04:30:39PM -0500, John Kacur wrote:
> I think I see the problem, the patch
> rt-numa: Move thread placement code to rt-numa library
> 
> added the fatal if numa_failed.

Sorry about the mess, I thought we can relay on libnuma. The concept to
depend on compile time on a library and not during runtime escaped my
attention.

> Thanks for testing. I'll send a patch to put it back the old way.

Thanks John!

Re: [PATCH 0/3] Revert always use libnuma patches

[John Kacur]

On Mon, 22 Feb 2021, Daniel Wagner wrote:

> On Fri, Feb 19, 2021 at 04:30:39PM -0500, John Kacur wrote:
> > I think I see the problem, the patch
> > rt-numa: Move thread placement code to rt-numa library
> > 
> > added the fatal if numa_failed.
> 
> Sorry about the mess, I thought we can relay on libnuma. The concept to
> depend on compile time on a library and not during runtime escaped my
> attention.

No problem, I should have caught this myself in code review.
I had honestly forgotten that when we made numa a compile time requirement
that we made it so that it was not a runtime requirement, even though I
wrote most of the code to achieve that.

> 
> > Thanks for testing. I'll send a patch to put it back the old way.
> 
> Thanks John!
>