linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Affinity devel and fixes
@ 2020-03-02 21:44 John Kacur
  2020-03-02 21:44 ` [PATCH 1/6] rt-tests: cyclictest: Set errno to 0 before call to mlock John Kacur
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

These are changes to improve affinity on cyclictest

There are some fixes to the parsing of -a CPUSET that occurred in a few
exceptional instances.

Also, cyclictest was written as if it owned the whole machine. So before
these changes we would run on all cpus or on the CPUSET that the user
passed with the command line option even if those cpus were not in the
CPUSET from the affinity in the runtime environment. These changes here
fix that. For example,

Before the changes
taskset 4-7 cyclictest -t -a 1-6
the taskset 4-7 would only apply to the main cyclictest thread, and then
the 1-6 would only apply to every newly created thread.

After the changes
taskset 4-7 cyclictest -t -a 1-6
Now, 4-7 still applies to the main thread, but all newly created threads
run on the interesection of the cpus 4-7 and 1-6, in other words on 4,5
and 6

In addition if the user doesn't pass and affinity with the command line
option, the runtime environment is respected.

For example

Before
taskset 0-4 cyclictest -t
Only the main cyclictest thread would run on cpus 0-4 and the other
threads could run on any cpu

After
taskset 0-4 cyclictest -t
All cyclictest threads run on 0-4


John Kacur (6):
  rt-tests: cyclictest: Set errno to 0 before call to mlock
  rt-tests: cyclictest: Report all errors from pthread_setaffinity_np
  rt-tests: cyclictest: Fix parsing affinity with a space and a leading
    zero
  rt-tests: cyclictest: Fix parsing affinity with leading exclamation
    mark
  rt-tests: cyclictest: Only run on available cpus according to the
    affinity
  rt-tests: cyclictest: Only run on runtime affinity and user supplied
    affinity

 src/cyclictest/cyclictest.c | 78 +++++++++++++++++++++++++++++++++----
 1 file changed, 71 insertions(+), 7 deletions(-)

-- 
2.20.1


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

* [PATCH 1/6] rt-tests: cyclictest: Set errno to 0 before call to mlock
  2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
@ 2020-03-02 21:44 ` John Kacur
  2020-03-02 21:44 ` [PATCH 2/6] rt-tests: cyclictest: Report all errors from pthread_setaffinity_np John Kacur
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

Set errno to 0 before the call to mlock in rstat_mlock()

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 01ae72f8cdf1..c66755dd4d87 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1868,9 +1868,8 @@ static int rstat_mlock(void *mptr)
 {
 	int err;
 
-	err = mlock(mptr, _SC_PAGE_SIZE);
-
 	errno = 0;
+	err = mlock(mptr, _SC_PAGE_SIZE);
 	if (err == -1) {
 		fprintf(stderr, "ERROR, mlock %s\n", strerror(errno));
 	}
-- 
2.20.1


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

* [PATCH 2/6] rt-tests: cyclictest: Report all errors from pthread_setaffinity_np
  2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
  2020-03-02 21:44 ` [PATCH 1/6] rt-tests: cyclictest: Set errno to 0 before call to mlock John Kacur
@ 2020-03-02 21:44 ` John Kacur
  2020-03-02 21:44 ` [PATCH 3/6] rt-tests: cyclictest: Fix parsing affinity with a space and a leading zero John Kacur
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

On error pthread_setaffinity_np returns a non-zero number.
Make sure cyclictest prints a warning in all such cases.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index c66755dd4d87..bd1fcd1092aa 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -650,7 +650,7 @@ static void *timerthread(void *param)
 		CPU_ZERO(&mask);
 		CPU_SET(par->cpu, &mask);
 		thread = pthread_self();
-		if (pthread_setaffinity_np(thread, sizeof(mask), &mask) == -1)
+		if (pthread_setaffinity_np(thread, sizeof(mask), &mask) != 0)
 			warn("Could not set CPU affinity to CPU #%d\n",
 			     par->cpu);
 	}
-- 
2.20.1


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

* [PATCH 3/6] rt-tests: cyclictest: Fix parsing affinity with a space and a leading zero
  2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
  2020-03-02 21:44 ` [PATCH 1/6] rt-tests: cyclictest: Set errno to 0 before call to mlock John Kacur
  2020-03-02 21:44 ` [PATCH 2/6] rt-tests: cyclictest: Report all errors from pthread_setaffinity_np John Kacur
@ 2020-03-02 21:44 ` John Kacur
  2020-03-02 21:44 ` [PATCH 4/6] rt-tests: cyclictest: Fix parsing affinity with leading exclamation mark John Kacur
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

-/cyclictest -t -a1-4
./cyclictest -t -a 1-4
./cyclictest -t -a0-4
all work as expected, but
./cyclictest -t -a 0-4
did not work, and instead AFFINITY_USEALL was set.

This is because atoi(argv[optind]) returns 0 for parsing non-numbers,
and this was treated as an error.

This missed the case where the user intentionally passes a 0 which
should not be treated as an error.

Fix this by testing for this case.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index bd1fcd1092aa..1d2962fda777 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1199,7 +1199,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
 			if (optarg != NULL) {
 				parse_cpumask(optarg, max_cpus);
 				setaffinity = AFFINITY_SPECIFIED;
-			} else if (optind<argc && atoi(argv[optind])) {
+			} else if (optind<argc && (atoi(argv[optind]) || argv[optind][0] == '0')) {
 				parse_cpumask(argv[optind], max_cpus);
 				setaffinity = AFFINITY_SPECIFIED;
 			} else {
-- 
2.20.1


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

* [PATCH 4/6] rt-tests: cyclictest: Fix parsing affinity with leading exclamation mark
  2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
                   ` (2 preceding siblings ...)
  2020-03-02 21:44 ` [PATCH 3/6] rt-tests: cyclictest: Fix parsing affinity with a space and a leading zero John Kacur
@ 2020-03-02 21:44 ` John Kacur
  2020-03-02 21:44 ` [PATCH 5/6] rt-tests: cyclictest: Only run on available cpus according to the affinity John Kacur
  2020-03-02 21:44 ` [PATCH 6/6] rt-tests: cyclictest: Only run on runtime affinity and user supplied affinity John Kacur
  5 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

The case in which the user specifies the affinity with a space and a
leading exclamation mark was not working. For example

./cyclictest -t -a'!4-5'
was working correctly, but the following was not.
./cyclictest -t -a '!4-5'

Fix this by recognizing this input as a non-error case.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 1d2962fda777..18c383689dc2 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1199,7 +1199,7 @@ static void process_options (int argc, char *argv[], int max_cpus)
 			if (optarg != NULL) {
 				parse_cpumask(optarg, max_cpus);
 				setaffinity = AFFINITY_SPECIFIED;
-			} else if (optind<argc && (atoi(argv[optind]) || argv[optind][0] == '0')) {
+			} else if (optind<argc && (atoi(argv[optind]) || argv[optind][0] == '0' || argv[optind][0] == '!')) {
 				parse_cpumask(argv[optind], max_cpus);
 				setaffinity = AFFINITY_SPECIFIED;
 			} else {
-- 
2.20.1


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

* [PATCH 5/6] rt-tests: cyclictest: Only run on available cpus according to the affinity
  2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
                   ` (3 preceding siblings ...)
  2020-03-02 21:44 ` [PATCH 4/6] rt-tests: cyclictest: Fix parsing affinity with leading exclamation mark John Kacur
@ 2020-03-02 21:44 ` John Kacur
  2020-03-02 21:44 ` [PATCH 6/6] rt-tests: cyclictest: Only run on runtime affinity and user supplied affinity John Kacur
  5 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

Right now cyclictest acts as if it owns the machine. If you don't
specify the affinity with -a [CPUSET] it will run threads on all cpus.

Change this to only run on cpus according to the affinity.

For example.

taskset -c 0-3 ./cyclictest -t
will only affect the main thread and limit it that to cpus 0-3 but the
threads that are spun off can run anywhere, with this change all threads
will be limited to cpus 0-3

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

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 18c383689dc2..ce93ad0643cd 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1040,7 +1040,8 @@ static unsigned int is_cpumask_zero(const struct bitmask *mask)
 	return (rt_numa_bitmask_count(mask) == 0);
 }
 
-static int cpu_for_thread(int thread_num, int max_cpus)
+/* cpu_for_thread AFFINITY_SPECIFIED */
+static int cpu_for_thread_sp(int thread_num, int max_cpus)
 {
 	unsigned int m, cpu, i, num_cpus;
 	num_cpus = rt_numa_bitmask_count(affinity_mask);
@@ -1059,6 +1060,36 @@ static int cpu_for_thread(int thread_num, int max_cpus)
 	return 0;
 }
 
+/* cpu_for_thread AFFINITY_USEALL */
+static int cpu_for_thread_ua(int thread_num, int max_cpus)
+{
+	int res, num_cpus, i, m, cpu;
+	pthread_t thread;
+	cpu_set_t cpuset;
+
+	thread = pthread_self();
+	CPU_ZERO(&cpuset);
+
+	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);
+	m = thread_num % num_cpus;
+
+	for (i = 0, cpu = 0; i < max_cpus; i++) {
+		if (CPU_ISSET(i, &cpuset)) {
+			if (cpu == m)
+				return i;
+			cpu++;
+		}
+	}
+
+	fprintf(stderr, "Bug in cpu mask handling code.\n");
+	return 0;
+}
+
 
 static void parse_cpumask(const char *option, const int max_cpus)
 {
@@ -2086,11 +2117,13 @@ int main(int argc, char **argv)
 		switch (setaffinity) {
 		case AFFINITY_UNSPECIFIED: cpu = -1; break;
 		case AFFINITY_SPECIFIED:
-			cpu = cpu_for_thread(i, max_cpus);
+			cpu = cpu_for_thread_sp(i, max_cpus);
 			if (verbose)
 				printf("Thread %d using cpu %d.\n", i, cpu);
 			break;
-		case AFFINITY_USEALL: cpu = i % max_cpus; break;
+		case AFFINITY_USEALL:
+			cpu = cpu_for_thread_ua(i, max_cpus);
+			break;
 		default: cpu = -1;
 		}
 
-- 
2.20.1


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

* [PATCH 6/6] rt-tests: cyclictest: Only run on runtime affinity and user supplied affinity
  2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
                   ` (4 preceding siblings ...)
  2020-03-02 21:44 ` [PATCH 5/6] rt-tests: cyclictest: Only run on available cpus according to the affinity John Kacur
@ 2020-03-02 21:44 ` John Kacur
  5 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2020-03-02 21:44 UTC (permalink / raw)
  To: rt-users; +Cc: Clark Williams, John Kacur

Currently if the user passes the affinity to cyclictest, threads are run
there even if they should be excluded according to the affinity of the
runtime environment.

For example

taskset -c 4-7 cyclictest -t -a 1-6

Should run on the interesection of this, that is on cpus 4,5,6

Fix this so it works as expected

Note, one caveat, this apply to the threads that cyclictest creates, but
only the initial taskset applies to the main cyclictest thread, so the
main thread can run on cpus 4,5,6 and 7

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

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index ce93ad0643cd..13d2b1722890 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1044,8 +1044,13 @@ static unsigned int is_cpumask_zero(const struct bitmask *mask)
 static int cpu_for_thread_sp(int thread_num, int max_cpus)
 {
 	unsigned int m, cpu, i, num_cpus;
+
 	num_cpus = rt_numa_bitmask_count(affinity_mask);
 
+	if (num_cpus == 0) {
+		fatal("No allowable cpus to run on\n");
+	}
+
 	m = thread_num % num_cpus;
 
 	/* there are num_cpus bits set, we want position of m'th one */
@@ -1091,6 +1096,30 @@ static int cpu_for_thread_ua(int thread_num, int max_cpus)
 }
 
 
+/* 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_bitmask_alloc(sizeof(struct bitmask));
+	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);
@@ -1098,7 +1127,10 @@ static void parse_cpumask(const char *option, const int max_cpus)
 		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);
-- 
2.20.1


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

end of thread, other threads:[~2020-03-02 21:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-02 21:44 [PATCH 0/6] Affinity devel and fixes John Kacur
2020-03-02 21:44 ` [PATCH 1/6] rt-tests: cyclictest: Set errno to 0 before call to mlock John Kacur
2020-03-02 21:44 ` [PATCH 2/6] rt-tests: cyclictest: Report all errors from pthread_setaffinity_np John Kacur
2020-03-02 21:44 ` [PATCH 3/6] rt-tests: cyclictest: Fix parsing affinity with a space and a leading zero John Kacur
2020-03-02 21:44 ` [PATCH 4/6] rt-tests: cyclictest: Fix parsing affinity with leading exclamation mark John Kacur
2020-03-02 21:44 ` [PATCH 5/6] rt-tests: cyclictest: Only run on available cpus according to the affinity John Kacur
2020-03-02 21:44 ` [PATCH 6/6] rt-tests: cyclictest: Only run on runtime affinity and user supplied affinity John Kacur

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).