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

[John Kacur <jkacur@redhat.com>]

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