[PATCH 1/3] cyclictest: Move main pid setaffinity handling into a function

[PATCH 1/3] cyclictest: Move main pid setaffinity handling into a function

[John Kacur]

From: Jonathan Schwender <schwenderjonathan@gmail.com>

Move error handling for setting the affinity of the main thread
into a separate function.
This prevents duplicating the code in the next commit,
where the main thread pid can be restricted to one of
two bitmasks depending on the passed parameters.

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index a561443fa67b..edb0c707d35c 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -1768,6 +1768,17 @@ static void write_stats(FILE *f, void *data)
 	fprintf(f, "  }\n");
 }
 
+static void set_main_thread_affinity(struct bitmask *cpumask)
+{
+	int res;
+
+	errno = 0;
+	res = numa_sched_setaffinity(getpid(), cpumask);
+	if (res != 0)
+		warn("Couldn't setaffinity in main thread: %s\n",
+		     strerror(errno));
+}
+
 int main(int argc, char **argv)
 {
 	sigset_t sigset;
@@ -1792,13 +1803,7 @@ int main(int argc, char **argv)
 
 	/* Restrict the main pid to the affinity specified by the user */
 	if (affinity_mask != NULL) {
-		int res;
-
-		errno = 0;
-		res = numa_sched_setaffinity(getpid(), affinity_mask);
-		if (res != 0)
-			warn("Couldn't setaffinity in main thread: %s\n", strerror(errno));
-
+		set_main_thread_affinity(affinity_mask);
 		if (verbose)
 			printf("Using %u cpus.\n",
 				numa_bitmask_weight(affinity_mask));
-- 
2.31.1

[PATCH 2/3] Subject: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.

[John Kacur]

From: Jonathan Schwender <schwenderjonathan@gmail.com>

This allows the user to specify a separate cpuset for the main pid,
e.g. on a housekeeping CPU.
If --mainaffinity is not specified, but --affinity is, then the
current behaviour is preserved and the main thread is bound
to the cpuset specified by --affinity

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
- Little fix-up applying patch
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index edb0c707d35c..3477cd137c15 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -837,6 +837,8 @@ static void display_help(int error)
 	       "	 --laptop	   Save battery when running cyclictest\n"
 	       "			   This will give you poorer realtime results\n"
 	       "			   but will not drain your battery so quickly\n"
+	       "         --mainaffinity=[CPUSET]  Run the main thread on CPU #N. This only affects\n"
+	       "                           the main thread and not the measurement threads\n"
 	       "-m       --mlockall        lock current and future memory allocations\n"
 	       "-M       --refresh_on_max  delay updating the screen until a new max\n"
 	       "			   latency is hit. Useful for low bandwidth.\n"
@@ -891,6 +893,7 @@ static int quiet;
 static int interval = DEFAULT_INTERVAL;
 static int distance = -1;
 static struct bitmask *affinity_mask = NULL;
+static struct bitmask *main_affinity_mask = NULL;
 static int smp = 0;
 static int setaffinity = AFFINITY_UNSPECIFIED;
 
@@ -944,8 +947,8 @@ enum option_values {
 	OPT_AFFINITY=1, OPT_BREAKTRACE, OPT_CLOCK,
 	OPT_DISTANCE, OPT_DURATION, OPT_LATENCY,
 	OPT_FIFO, OPT_HISTOGRAM, OPT_HISTOFALL, OPT_HISTFILE,
-	OPT_INTERVAL, OPT_JSON, OPT_LOOPS, OPT_MLOCKALL, OPT_REFRESH,
-	OPT_NANOSLEEP, OPT_NSECS, OPT_OSCOPE, OPT_PRIORITY,
+	OPT_INTERVAL, OPT_JSON, OPT_MAINAFFINITY, OPT_LOOPS, OPT_MLOCKALL,
+	OPT_REFRESH, OPT_NANOSLEEP, OPT_NSECS, OPT_OSCOPE, OPT_PRIORITY,
 	OPT_QUIET, OPT_PRIOSPREAD, OPT_RELATIVE, OPT_RESOLUTION,
 	OPT_SYSTEM, OPT_SMP, OPT_THREADS, OPT_TRIGGER,
 	OPT_TRIGGER_NODES, OPT_UNBUFFERED, OPT_NUMA, OPT_VERBOSE,
@@ -982,6 +985,7 @@ static void process_options(int argc, char *argv[], int max_cpus)
 			{"json",             required_argument, NULL, OPT_JSON },
 			{"laptop",	     no_argument,	NULL, OPT_LAPTOP },
 			{"loops",            required_argument, NULL, OPT_LOOPS },
+			{"mainaffinity",     required_argument, NULL, OPT_MAINAFFINITY},
 			{"mlockall",         no_argument,       NULL, OPT_MLOCKALL },
 			{"refresh_on_max",   no_argument,       NULL, OPT_REFRESH },
 			{"nsecs",            no_argument,       NULL, OPT_NSECS },
@@ -1086,6 +1090,16 @@ static void process_options(int argc, char *argv[], int max_cpus)
 		case 'l':
 		case OPT_LOOPS:
 			max_cycles = atoi(optarg); break;
+		case OPT_MAINAFFINITY:
+			if (optarg) {
+				parse_cpumask(optarg, max_cpus, &main_affinity_mask);
+			} else if (optind < argc &&
+			           (atoi(argv[optind]) ||
+			            argv[optind][0] == '0' ||
+			            argv[optind][0] == '!')) {
+				parse_cpumask(argv[optind], max_cpus, &main_affinity_mask);
+			}
+			break;
 		case 'm':
 		case OPT_MLOCKALL:
 			lockall = 1; break;
@@ -1802,7 +1816,9 @@ int main(int argc, char **argv)
 	}
 
 	/* Restrict the main pid to the affinity specified by the user */
-	if (affinity_mask != NULL) {
+	if (main_affinity_mask != NULL) {
+		set_main_thread_affinity(main_affinity_mask);
+	} else if (affinity_mask != NULL) {
 		set_main_thread_affinity(affinity_mask);
 		if (verbose)
 			printf("Using %u cpus.\n",
-- 
2.31.1

[PATCH 3/3] rt-tests: cyclictest: Add entry for mainaffinity in the manpage

[John Kacur]

Add an entry for mainaffinity in the manpage.
Tweek the output in cyclictest for mainaffinity as well.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 src/cyclictest/cyclictest.8 | 3 +++
 src/cyclictest/cyclictest.c | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/cyclictest/cyclictest.8 b/src/cyclictest/cyclictest.8
index 4fe42e502535..abc56e8e4970 100644
--- a/src/cyclictest/cyclictest.8
+++ b/src/cyclictest/cyclictest.8
@@ -94,6 +94,9 @@ Set the number of loops. The default is 0 (endless). This option is useful for a
 .B \-\-laptop
 Save battery when running cyclictest. This will give you poorer realtime results, but will not drain your battery so quickly.
 .TP
+.B \-\-mainaffinity=CPUSET
+Run the main thread on CPU #N. This only affects the main thread and not the measurement threads
+.TP
 .B \-m, \-\-mlockall
 Lock current and future memory allocations to prevent being paged out
 .TP
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 3477cd137c15..be8285a072b4 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -837,7 +837,8 @@ static void display_help(int error)
 	       "	 --laptop	   Save battery when running cyclictest\n"
 	       "			   This will give you poorer realtime results\n"
 	       "			   but will not drain your battery so quickly\n"
-	       "         --mainaffinity=[CPUSET]  Run the main thread on CPU #N. This only affects\n"
+	       "         --mainaffinity=CPUSET\n"
+	       "			   Run the main thread on CPU #N. This only affects\n"
 	       "                           the main thread and not the measurement threads\n"
 	       "-m       --mlockall        lock current and future memory allocations\n"
 	       "-M       --refresh_on_max  delay updating the screen until a new max\n"
-- 
2.31.1

Re: [PATCH 3/3] rt-tests: cyclictest: Add entry for mainaffinity in the manpage

[Daniel Wagner]

Hi John,

On Wed, Jun 16, 2021 at 07:18:00PM -0400, John Kacur wrote:
> Add an entry for mainaffinity in the manpage.
> Tweek the output in cyclictest for mainaffinity as well.
> 
> Signed-off-by: John Kacur <jkacur@redhat.com>
> ---
>  src/cyclictest/cyclictest.8 | 3 +++
>  src/cyclictest/cyclictest.c | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cyclictest/cyclictest.8 b/src/cyclictest/cyclictest.8
> index 4fe42e502535..abc56e8e4970 100644
> --- a/src/cyclictest/cyclictest.8
> +++ b/src/cyclictest/cyclictest.8
> @@ -94,6 +94,9 @@ Set the number of loops. The default is 0 (endless). This option is useful for a
>  .B \-\-laptop
>  Save battery when running cyclictest. This will give you poorer realtime results, but will not drain your battery so quickly.
>  .TP
> +.B \-\-mainaffinity=CPUSET
> +Run the main thread on CPU #N. This only affects the main thread and not the measurement threads

Nitpick: the .RI section is missing the --mainaffinity.

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Re: [PATCH 1/3] cyclictest: Move main pid setaffinity handling into a function

[Daniel Wagner]

On Wed, Jun 16, 2021 at 07:17:58PM -0400, John Kacur wrote:
> From: Jonathan Schwender <schwenderjonathan@gmail.com>
> 
> Move error handling for setting the affinity of the main thread
> into a separate function.
> This prevents duplicating the code in the next commit,
> where the main thread pid can be restricted to one of
> two bitmasks depending on the passed parameters.
> 
> Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
> Signed-off-by: John Kacur <jkacur@redhat.com>

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Re: [PATCH 2/3] Subject: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.

[Daniel Wagner]

On Wed, Jun 16, 2021 at 07:17:59PM -0400, John Kacur wrote:
> From: Jonathan Schwender <schwenderjonathan@gmail.com>
> 
> This allows the user to specify a separate cpuset for the main pid,
> e.g. on a housekeeping CPU.
> If --mainaffinity is not specified, but --affinity is, then the
> current behaviour is preserved and the main thread is bound
> to the cpuset specified by --affinity
> 
> Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
> - Little fix-up applying patch
> Signed-off-by: John Kacur <jkacur@redhat.com>

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Re: [PATCH 3/3] rt-tests: cyclictest: Add entry for mainaffinity in the manpage

[John Kacur]

On Thu, 17 Jun 2021, Daniel Wagner wrote:

> Hi John,
> 
> On Wed, Jun 16, 2021 at 07:18:00PM -0400, John Kacur wrote:
> > Add an entry for mainaffinity in the manpage.
> > Tweek the output in cyclictest for mainaffinity as well.
> > 
> > Signed-off-by: John Kacur <jkacur@redhat.com>
> > ---
> >  src/cyclictest/cyclictest.8 | 3 +++
> >  src/cyclictest/cyclictest.c | 3 ++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/cyclictest/cyclictest.8 b/src/cyclictest/cyclictest.8
> > index 4fe42e502535..abc56e8e4970 100644
> > --- a/src/cyclictest/cyclictest.8
> > +++ b/src/cyclictest/cyclictest.8
> > @@ -94,6 +94,9 @@ Set the number of loops. The default is 0 (endless). This option is useful for a
> >  .B \-\-laptop
> >  Save battery when running cyclictest. This will give you poorer realtime results, but will not drain your battery so quickly.
> >  .TP
> > +.B \-\-mainaffinity=CPUSET
> > +Run the main thread on CPU #N. This only affects the main thread and not the measurement threads
> 
> Nitpick: the .RI section is missing the --mainaffinity.

The Synopsis section doesn't have to be exhaustive, it just has to show 
typical usage. However, that section is crufty and could use some love.
For example, while we have the long version --policy, the short version -y
no longer exists but is mentioned there. Have a go at it if you're bored.

John

> 
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
>