| Message ID | 20210518083712.8237-3-schwenderjonathan@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | rt-tests: cyclictest: Add option to specify main pid affinity | expand |
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
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 > >
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 > >
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 >> >>
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
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",
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(-)