| Message ID | YqnExv0GkIKTSwoI@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | cyclictest: Delay setting of main_affinity_mask after creating threads. | expand |
Hello Sebastian, On 6/15/22 13:38, Sebastian Andrzej Siewior wrote: > Assuming the current affinity mask of the task is 0x31 (CPUs 0, 4, 5). > Starting cyclictest with '-S' option will fork the following threads: > - monitor, mask 0x31 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x10 > - measure thread 3, mask 0x20 > > works as expected. Using the options '-S --mainaffinity=0' leads to: > - monitor, mask 0x01 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x01 > - measure thread 3, mask 0x01 > > because the mask of the main thread has been reset early to 0x01 and > does not allow a CPU mask outside of this mask while setting the > affinity for the new threads. > > Delay setting the affinity of the main/ monitor thread after the > measuring threads have been deployed. This leads to the following Since starting the threads represents some work that can disturb the threads, shouldn't the main thread be migrated to the 'mainaffinity' mask early as it is right now ? I think it would be better to: 1. Save the initial mask of the main thread somewhere 2. Set the affinity of the main thread 3. Use the initial mask when setting the affinity of the threads Regards, Pierre > state: > - monitor, mask 0x01 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x10 > - measure thread 3, mask 0x20 > > Signed-off-by: Sebastian Andrzej Savior <bigeasy@linutronix.de> > --- > src/cyclictest/cyclictest.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index be8285a072b4e..cbf8d00293ec6 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -1816,10 +1816,7 @@ int main(int argc, char **argv) > printf("Online CPUs = %d\n", online_cpus); > } > > - /* Restrict the main pid to the affinity specified by the user */ > - if (main_affinity_mask != NULL) { > - set_main_thread_affinity(main_affinity_mask); > - } else if (affinity_mask != NULL) { > + if (affinity_mask != NULL) { > set_main_thread_affinity(affinity_mask); > if (verbose) > printf("Using %u cpus.\n", > @@ -2094,6 +2091,10 @@ int main(int argc, char **argv) > fatal("failed to create thread %d: %s\n", i, strerror(status)); > > } > + /* Restrict the main pid to the affinity specified by the user */ > + if (main_affinity_mask != NULL) > + set_main_thread_affinity(main_affinity_mask); > + > if (use_fifo) { > status = pthread_create(&fifo_threadid, NULL, fifothread, NULL); > if (status)
On 2022-06-15 17:18:55 [+0200], Pierre Gondois wrote: > > If this is *that* important I would lean towards always doing the > > globalt_barr so all threads start once everything is set up. If I > > remember correctly, the first few cycles are not counted and just fill > > the cache. > > Ok yes, I didn't see the globalt_barr barrier. FWIW I tested > the patch and it works well. The globalt_barr isn't enabled by default. If that is important then I recommend doing this by default. I can post patch if you want me to. > > > > > Regards, > > > Pierre Sebastian
On 6/15/22 13:38, Sebastian Andrzej Siewior wrote: > Assuming the current affinity mask of the task is 0x31 (CPUs 0, 4, 5). > Starting cyclictest with '-S' option will fork the following threads: > - monitor, mask 0x31 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x10 > - measure thread 3, mask 0x20 > > works as expected. Using the options '-S --mainaffinity=0' leads to: > - monitor, mask 0x01 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x01 > - measure thread 3, mask 0x01 > > because the mask of the main thread has been reset early to 0x01 and > does not allow a CPU mask outside of this mask while setting the > affinity for the new threads. > > Delay setting the affinity of the main/ monitor thread after the > measuring threads have been deployed. This leads to the following > state: > - monitor, mask 0x01 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x10 > - measure thread 3, mask 0x20 > > Signed-off-by: Sebastian Andrzej Savior <bigeasy@linutronix.de> > --- > src/cyclictest/cyclictest.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index be8285a072b4e..cbf8d00293ec6 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -1816,10 +1816,7 @@ int main(int argc, char **argv) > printf("Online CPUs = %d\n", online_cpus); > } > > - /* Restrict the main pid to the affinity specified by the user */ > - if (main_affinity_mask != NULL) { > - set_main_thread_affinity(main_affinity_mask); > - } else if (affinity_mask != NULL) { > + if (affinity_mask != NULL) { > set_main_thread_affinity(affinity_mask); 2 other remarks (sorry for the spam): - In the documentation, '-a' seems to indicate a mask for the threads, not for the main process. Should the main thread be restricted to the mask obtained for '-a' ? - I think there is a bug (unrelated to the patch) when issuing: ./cyclictest -l20000 -v -a -t10 This is due to trying to print a NULL the affinity_mask at: @@ -1088,9 +1093,6 @@ static void process_options(int argc, char *argv[], int max_cpus) if (setaffinity == AFFINITY_SPECIFIED && !affinity_mask) display_help(1); - if (verbose) - printf("Using %u cpus.\n", - numa_bitmask_weight(affinity_mask)); break; case 'A': case OPT_ALIGNED: The command: ./cyclictest -l20000 -a -t10 -v will work since '-v' is parsed at last and the 'if (verbose)' statement is ignored. > if (verbose) > printf("Using %u cpus.\n", > @@ -2094,6 +2091,10 @@ int main(int argc, char **argv) > fatal("failed to create thread %d: %s\n", i, strerror(status)); > > } > + /* Restrict the main pid to the affinity specified by the user */ > + if (main_affinity_mask != NULL) > + set_main_thread_affinity(main_affinity_mask); > + > if (use_fifo) { > status = pthread_create(&fifo_threadid, NULL, fifothread, NULL); > if (status)
On Wed, 15 Jun 2022, Sebastian Andrzej Siewior wrote: > Assuming the current affinity mask of the task is 0x31 (CPUs 0, 4, 5). > Starting cyclictest with '-S' option will fork the following threads: > - monitor, mask 0x31 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x10 > - measure thread 3, mask 0x20 > > works as expected. Using the options '-S --mainaffinity=0' leads to: > - monitor, mask 0x01 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x01 > - measure thread 3, mask 0x01 > > because the mask of the main thread has been reset early to 0x01 and > does not allow a CPU mask outside of this mask while setting the > affinity for the new threads. > > Delay setting the affinity of the main/ monitor thread after the > measuring threads have been deployed. This leads to the following > state: > - monitor, mask 0x01 > - measure thread 1, mask 0x01 > - measure thread 2, mask 0x10 > - measure thread 3, mask 0x20 > > Signed-off-by: Sebastian Andrzej Savior <bigeasy@linutronix.de> > --- > src/cyclictest/cyclictest.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index be8285a072b4e..cbf8d00293ec6 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -1816,10 +1816,7 @@ int main(int argc, char **argv) > printf("Online CPUs = %d\n", online_cpus); > } > > - /* Restrict the main pid to the affinity specified by the user */ > - if (main_affinity_mask != NULL) { > - set_main_thread_affinity(main_affinity_mask); > - } else if (affinity_mask != NULL) { > + if (affinity_mask != NULL) { > set_main_thread_affinity(affinity_mask); > if (verbose) > printf("Using %u cpus.\n", > @@ -2094,6 +2091,10 @@ int main(int argc, char **argv) > fatal("failed to create thread %d: %s\n", i, strerror(status)); > > } > + /* Restrict the main pid to the affinity specified by the user */ > + if (main_affinity_mask != NULL) > + set_main_thread_affinity(main_affinity_mask); > + > if (use_fifo) { > status = pthread_create(&fifo_threadid, NULL, fifothread, NULL); > if (status) > -- > 2.36.1 > > Signed-off-by: John Kacur <jkacur@redhat.com>
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c index be8285a072b4e..cbf8d00293ec6 100644 --- a/src/cyclictest/cyclictest.c +++ b/src/cyclictest/cyclictest.c @@ -1816,10 +1816,7 @@ int main(int argc, char **argv) printf("Online CPUs = %d\n", online_cpus); } - /* Restrict the main pid to the affinity specified by the user */ - if (main_affinity_mask != NULL) { - set_main_thread_affinity(main_affinity_mask); - } else if (affinity_mask != NULL) { + if (affinity_mask != NULL) { set_main_thread_affinity(affinity_mask); if (verbose) printf("Using %u cpus.\n", @@ -2094,6 +2091,10 @@ int main(int argc, char **argv) fatal("failed to create thread %d: %s\n", i, strerror(status)); } + /* Restrict the main pid to the affinity specified by the user */ + if (main_affinity_mask != NULL) + set_main_thread_affinity(main_affinity_mask); + if (use_fifo) { status = pthread_create(&fifo_threadid, NULL, fifothread, NULL); if (status)
Assuming the current affinity mask of the task is 0x31 (CPUs 0, 4, 5). Starting cyclictest with '-S' option will fork the following threads: - monitor, mask 0x31 - measure thread 1, mask 0x01 - measure thread 2, mask 0x10 - measure thread 3, mask 0x20 works as expected. Using the options '-S --mainaffinity=0' leads to: - monitor, mask 0x01 - measure thread 1, mask 0x01 - measure thread 2, mask 0x01 - measure thread 3, mask 0x01 because the mask of the main thread has been reset early to 0x01 and does not allow a CPU mask outside of this mask while setting the affinity for the new threads. Delay setting the affinity of the main/ monitor thread after the measuring threads have been deployed. This leads to the following state: - monitor, mask 0x01 - measure thread 1, mask 0x01 - measure thread 2, mask 0x10 - measure thread 3, mask 0x20 Signed-off-by: Sebastian Andrzej Savior <bigeasy@linutronix.de> --- src/cyclictest/cyclictest.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)