All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] rt-tests: cyclictest: Add option to specify main pid affinity
@ 2021-05-18  8:37 Jonathan Schwender
  2021-05-18  8:37 ` [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function Jonathan Schwender
  2021-05-18  8:37 ` [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option Jonathan Schwender
  0 siblings, 2 replies; 10+ messages in thread
From: Jonathan Schwender @ 2021-05-18  8:37 UTC (permalink / raw)
  To: jkacur, williams; +Cc: linux-rt-users

Hi John,

Changes in v4:
- Rolled back the error wrapper changes around numa_sched_setaffinity.
  I still extracted the error handling code into a function, but since it's
  only used locally it looks a lot nicer.
- Rebased onto latest commit c9051a36 ("ssdd: Add JSON output feature")
  in unstable/devel/latest.


This patch adds the option --mainaffinity to specify the affinity of
the main pid.
This is mainly useful if you want to bind the main thread to a
different (e.g. housekeeping ) CPU than the measurement threads.
Some of the potential benefits of using this option are presented in
a previous email: https://lore.kernel.org/linux-rt-users/dd40b81d-7099-7740-c2ad-64b49e582234@gmail.com/

Jonathan Schwender (2):
  cyclictest: Move main pid setaffinity handling into a function
  cyclictest: Add --mainaffinity=[CPUSET] option.

 src/cyclictest/cyclictest.c | 39 ++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

-- 
2.31.1


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

* [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function
  2021-05-18  8:37 [PATCH v4 0/2] rt-tests: cyclictest: Add option to specify main pid affinity Jonathan Schwender
@ 2021-05-18  8:37 ` Jonathan Schwender
  2021-05-21 20:19   ` John Kacur
  2021-05-18  8:37 ` [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option Jonathan Schwender
  1 sibling, 1 reply; 10+ messages in thread
From: Jonathan Schwender @ 2021-05-18  8:37 UTC (permalink / raw)
  To: jkacur, williams; +Cc: linux-rt-users

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>
---
 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 59dda19..3bab3b2 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


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

* [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.
  2021-05-18  8:37 [PATCH v4 0/2] rt-tests: cyclictest: Add option to specify main pid affinity Jonathan Schwender
  2021-05-18  8:37 ` [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function Jonathan Schwender
@ 2021-05-18  8:37 ` Jonathan Schwender
  2021-05-19 15:55   ` Daniel Wagner
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Jonathan Schwender @ 2021-05-18  8:37 UTC (permalink / raw)
  To: jkacur, williams; +Cc: linux-rt-users

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>
---
 src/cyclictest/cyclictest.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 3bab3b2..a2103c7 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -836,6 +836,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,7 +947,7 @@ 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_LOOPS, OPT_MLOCKALL, OPT_REFRESH,
+	OPT_INTERVAL, OPT_LOOPS, OPT_MAINAFFINITY, 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,
@@ -981,6 +984,7 @@ static void process_options(int argc, char *argv[], int max_cpus)
 			{"interval",         required_argument, NULL, OPT_INTERVAL },
 			{"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 },
@@ -1083,6 +1087,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


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

* Re: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.
  2021-05-18  8:37 ` [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option Jonathan Schwender
@ 2021-05-19 15:55   ` Daniel Wagner
  2021-05-22  8:13     ` Jonathan Schwender
  2021-05-21 20:21   ` John Kacur
  2021-05-21 20:24   ` John Kacur
  2 siblings, 1 reply; 10+ messages in thread
From: Daniel Wagner @ 2021-05-19 15:55 UTC (permalink / raw)
  To: Jonathan Schwender; +Cc: jkacur, williams, linux-rt-users

Hi Jonathan,

On Tue, May 18, 2021 at 10:37:12AM +0200, Jonathan Schwender wrote:
> 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>
> ---
>  src/cyclictest/cyclictest.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 3bab3b2..a2103c7 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -836,6 +836,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"

The main thread is allowed to run on any CPU which CPUSET specifies. CPU
#N does not really make sense.

While at it, please update man page accordingly.

Thanks,
Daniel

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

* Re: [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function
  2021-05-18  8:37 ` [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function Jonathan Schwender
@ 2021-05-21 20:19   ` John Kacur
  2021-05-22  7:35     ` Jonathan Schwender
  0 siblings, 1 reply; 10+ messages in thread
From: John Kacur @ 2021-05-21 20:19 UTC (permalink / raw)
  To: Jonathan Schwender; +Cc: williams, linux-rt-users



On Tue, 18 May 2021, Jonathan Schwender wrote:

> 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>
> ---
>  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 59dda19..3bab3b2 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");
>  }
>  

How about set_thread_affinity() since you want to use the function
for the main thread plus other threads.

> +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
> 
> 


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

* Re: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.
  2021-05-18  8:37 ` [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option Jonathan Schwender
  2021-05-19 15:55   ` Daniel Wagner
@ 2021-05-21 20:21   ` John Kacur
  2021-05-21 20:24   ` John Kacur
  2 siblings, 0 replies; 10+ messages in thread
From: John Kacur @ 2021-05-21 20:21 UTC (permalink / raw)
  To: Jonathan Schwender; +Cc: williams, linux-rt-users



On Tue, 18 May 2021, Jonathan Schwender wrote:

> 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>
> ---
>  src/cyclictest/cyclictest.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 3bab3b2..a2103c7 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -836,6 +836,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,7 +947,7 @@ 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_LOOPS, OPT_MLOCKALL, OPT_REFRESH,
> +	OPT_INTERVAL, OPT_LOOPS, OPT_MAINAFFINITY, 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,
> @@ -981,6 +984,7 @@ static void process_options(int argc, char *argv[], int max_cpus)
>  			{"interval",         required_argument, NULL, OPT_INTERVAL },
>  			{"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 },
> @@ -1083,6 +1087,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);

Am I missing something here, if there is a main_affinity_mask we set that
but then skip over the affinity_mask. Don't we want to check both?

> +	} else if (affinity_mask != NULL) {
>  		set_main_thread_affinity(affinity_mask);
>  		if (verbose)
>  			printf("Using %u cpus.\n",
> -- 
> 2.31.1
> 
> 


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

* Re: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.
  2021-05-18  8:37 ` [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option Jonathan Schwender
  2021-05-19 15:55   ` Daniel Wagner
  2021-05-21 20:21   ` John Kacur
@ 2021-05-21 20:24   ` John Kacur
  2021-05-22  7:57     ` Jonathan Schwender
  2 siblings, 1 reply; 10+ messages in thread
From: John Kacur @ 2021-05-21 20:24 UTC (permalink / raw)
  To: Jonathan Schwender; +Cc: williams, linux-rt-users



On Tue, 18 May 2021, Jonathan Schwender wrote:

> 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>
> ---
>  src/cyclictest/cyclictest.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
> index 3bab3b2..a2103c7 100644
> --- a/src/cyclictest/cyclictest.c
> +++ b/src/cyclictest/cyclictest.c
> @@ -836,6 +836,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,7 +947,7 @@ 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_LOOPS, OPT_MLOCKALL, OPT_REFRESH,
> +	OPT_INTERVAL, OPT_LOOPS, OPT_MAINAFFINITY, 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,
> @@ -981,6 +984,7 @@ static void process_options(int argc, char *argv[], int max_cpus)
>  			{"interval",         required_argument, NULL, OPT_INTERVAL },
>  			{"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 },
> @@ -1083,6 +1087,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);

Am I missing something here, if there is a main_affinity_mask we set that
but then skip over the affinity_mask. Don't we want to check both?

> +	} else if (affinity_mask != NULL) {
>  		set_main_thread_affinity(affinity_mask);
>  		if (verbose)
>  			printf("Using %u cpus.\n",
> -- 
> 2.31.1
> 
> 


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

* Re: [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function
  2021-05-21 20:19   ` John Kacur
@ 2021-05-22  7:35     ` Jonathan Schwender
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Schwender @ 2021-05-22  7:35 UTC (permalink / raw)
  To: John Kacur; +Cc: williams, linux-rt-users


Am 21.05.2021 um 22:19 schrieb John Kacur:
> How about set_thread_affinity() since you want to use the function
> for the main thread plus other threads.
>
>> +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));
>> +}
>> +
>>   

Actually, I only intended to use this for the main thread.
I didn't touch the affinity setting of the timerthreads (and don't see a 
need to).

I'm not familiar with the whole cyclictest code and all options. Did you 
have
any specific threads in mind that also use numa_sched_setaffinity() and 
could
use this function?

Maybe you got that impression from the previous v3 iteration, where I 
moved the functionality
to rt_numa and added a parameter for the pid/tid. My motivation there 
was mainly,
that if I'm moving it into a library, then I should probably add a 
parameter for the pid to the function
in case someone has a usecase for that.



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

* Re: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.
  2021-05-21 20:24   ` John Kacur
@ 2021-05-22  7:57     ` Jonathan Schwender
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Schwender @ 2021-05-22  7:57 UTC (permalink / raw)
  To: John Kacur; +Cc: williams, linux-rt-users


Am 21.05.2021 um 22:24 schrieb John Kacur:
>
> On Tue, 18 May 2021, Jonathan Schwender wrote:
>
>> 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

>> @@ -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);
> Am I missing something here, if there is a main_affinity_mask we set that
> but then skip over the affinity_mask. Don't we want to check both?

So the idea here is that if --mainaffinity is specified, then we set the 
affinity of  the main thread
to the specified value, _regardless_ of --affinity. E.g.:

cyclictest --affinity=1,2,3 --mainaffinity=0 [other options]

The intended behavior is that the main thread is bound to CPU 0, and the 
other threads to CPUs 1-3.
I don't see a reason why mainaffinity should be restricted to a subset 
of affinity. E.g., if for some reason
(e.g. cyclictest in a container) you spawn multiple cyclictest instances 
with non-overlapping affinity cpusets
but want to offload the main pids to a common CPU (e.g. CPU 0). 
Restricting mainaffinity to a subset of
affinity would mean that now every cyclictest instance also must spawn a 
measurement thread on CPU 0,
even if you aren't really interested in the latencies of the 
housekeeping CPU. Due to the (potentially) high priority of
the timer threads, they might even negatively influence isolated CPUs by 
delaying offloaded tasks.

Another way to look at it is that --mainaffinity (if present) overrides 
--affinity when determining the target affinity for the
main thread. The affinity for the measurement threads is not affected by 
this in any way.

>
>> +	} else if (affinity_mask != NULL) {
>>   		set_main_thread_affinity(affinity_mask);
>>   		if (verbose)
>>   			printf("Using %u cpus.\n",
>> -- 
>> 2.31.1
>>
>>

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

* Re: [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option.
  2021-05-19 15:55   ` Daniel Wagner
@ 2021-05-22  8:13     ` Jonathan Schwender
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Schwender @ 2021-05-22  8:13 UTC (permalink / raw)
  To: Daniel Wagner; +Cc: jkacur, williams, linux-rt-users

Hi Daniel,

Am 19.05.2021 um 17:55 schrieb Daniel Wagner:
>
>> +	       "         --mainaffinity=[CPUSET]  Run the main thread on CPU #N. This only affects\n"
>> +	       "                           the main thread and not the measurement threads\n"
> The main thread is allowed to run on any CPU which CPUSET specifies. CPU
> #N does not really make sense.
>
> While at it, please update man page accordingly.
Thanks for catching this. I'll include it in the next iteration.
> Thanks,
> Daniel


Regards,

Jonathan

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

end of thread, other threads:[~2021-05-22  8:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-18  8:37 [PATCH v4 0/2] rt-tests: cyclictest: Add option to specify main pid affinity Jonathan Schwender
2021-05-18  8:37 ` [PATCH v4 1/2] cyclictest: Move main pid setaffinity handling into a function Jonathan Schwender
2021-05-21 20:19   ` John Kacur
2021-05-22  7:35     ` Jonathan Schwender
2021-05-18  8:37 ` [PATCH v4 2/2] cyclictest: Add --mainaffinity=[CPUSET] option Jonathan Schwender
2021-05-19 15:55   ` Daniel Wagner
2021-05-22  8:13     ` Jonathan Schwender
2021-05-21 20:21   ` John Kacur
2021-05-21 20:24   ` John Kacur
2021-05-22  7:57     ` Jonathan Schwender

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.