| Message ID | 20210708151827.21430-2-dwagner@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix a few fallouts | expand |
On Thu, 8 Jul 2021, Daniel Wagner wrote: > pthread_getaffinity_np() prevents static builds as glibc does not > expose it for this configuration. Instead use sched_getaffinity() > which is always present and has the exact same semantics. > > Fixes: f240656b056b ("rt-tests: cyclictest: Fix -t without a user specified [NUM]") > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/lib/rt-numa.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c > index babcc634d57e..bb0121a65eca 100644 > --- a/src/lib/rt-numa.c > +++ b/src/lib/rt-numa.c > @@ -68,15 +68,13 @@ int cpu_for_thread_sp(int thread_num, int max_cpus, struct bitmask *cpumask) > int cpu_for_thread_ua(int thread_num, int max_cpus) > { > int res, num_cpus, i, m, cpu; > - pthread_t thread; > cpu_set_t cpuset; > > - thread = pthread_self(); > CPU_ZERO(&cpuset); > > - res = pthread_getaffinity_np(thread, sizeof(cpu_set_t), &cpuset); > + res = sched_getaffinity(0, sizeof(cpu_set_t), &cpuset); > if (res != 0) > - fatal("pthread_getaffinity_np failed: %s\n", strerror(res)); > + fatal("sched_getaffinity failed: %s\n", strerror(res)); > > num_cpus = CPU_COUNT(&cpuset); > m = thread_num % num_cpus; > -- > 2.32.0 > > It looks okay on the surface but I would like to know where you tested. Also, in your message you explain that you can't do a static build but we never made static builds a requirment for rt-tests. I know from your cover letter than you are trying to do something with arm, if this is your motivation you should put it in your message. Even if this is your motivation is there a reason we have to have a static build there? Finally, you put "fixes f240656b056b", it is useful to know where this was introduced in trying to figure out whether we can really replace a pthread call with the sched call, but then phrase it that way, because as far as I know that commit is not a problem needing fixing, so put "pthread_getaffinity_np call introduced in f240656b056b" or something like that. Thanks John
On Fri, Jul 09, 2021 at 01:58:51PM -0400, John Kacur wrote: > It looks okay on the surface but I would like to know where you tested. x86_64 and armv7. FWIW, pthread_getaffinity_np() is a fancy wrapper around sched_getaffinity(): https://github.com/bminor/glibc/blob/master/nptl/pthread_getaffinity.c#L34 > Also, in your message you explain that you can't do a static build > but we never made static builds a requirment for rt-tests. So you are saying static builds are not supported at all? > I know from > your cover letter than you are trying to do something with arm, if this > is your motivation you should put it in your message. Even if this is your > motivation is there a reason we have to have a static build there? Sure thing, I'll add more to the commit message. I just find it strange that you are starting to argue static builds are not supported. This is very handy especially since the linker dependency on libnuma. I can drop the binary on the target without the need to install libnuma into the rootfs. It is not available on all distro for armv7. Another user of static builds is the LAVA test suite. It ships the rt-tests programs as static builds so that the test can run without the need to pre populate the rootfs with the correct binary: https://github.com/Linaro/test-definitions/tree/master/automated/linux/cyclictest/bin https://github.com/Linaro/test-definitions/blob/master/automated/linux/cyclictest/cyclictest.sh#L41 I am not really a big fan of this either and wont defend it. I am just listening existing users. > Finally, you put "fixes f240656b056b", it is useful to know where this > was introduced in trying to figure out whether we can really replace a > pthread call with the sched call, but then phrase it that way, because > as far as I know that commit is not a problem needing fixing, so put > "pthread_getaffinity_np call introduced in f240656b056b" > or something like that. From my POV it is fixing a bug. If you decide static builds are not supported, please say so. This allows me to discuss this issue with the LAVA folks. Thanks, Daniel
diff --git a/src/lib/rt-numa.c b/src/lib/rt-numa.c index babcc634d57e..bb0121a65eca 100644 --- a/src/lib/rt-numa.c +++ b/src/lib/rt-numa.c @@ -68,15 +68,13 @@ int cpu_for_thread_sp(int thread_num, int max_cpus, struct bitmask *cpumask) int cpu_for_thread_ua(int thread_num, int max_cpus) { int res, num_cpus, i, m, cpu; - pthread_t thread; cpu_set_t cpuset; - thread = pthread_self(); CPU_ZERO(&cpuset); - res = pthread_getaffinity_np(thread, sizeof(cpu_set_t), &cpuset); + res = sched_getaffinity(0, sizeof(cpu_set_t), &cpuset); if (res != 0) - fatal("pthread_getaffinity_np failed: %s\n", strerror(res)); + fatal("sched_getaffinity failed: %s\n", strerror(res)); num_cpus = CPU_COUNT(&cpuset); m = thread_num % num_cpus;
pthread_getaffinity_np() prevents static builds as glibc does not expose it for this configuration. Instead use sched_getaffinity() which is always present and has the exact same semantics. Fixes: f240656b056b ("rt-tests: cyclictest: Fix -t without a user specified [NUM]") Signed-off-by: Daniel Wagner <dwagner@suse.de> --- src/lib/rt-numa.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)