| Message ID | 20201218161843.1764-2-dwagner@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [rt-tests,v2,v2,01/20] cyclictest: Always use libnuma | expand |
On Fri, 18 Dec 2020, Daniel Wagner wrote: > libnuma is hard dependency for cyclictest. Thus we can always call > numa_initialize(). This allows us to remove the global 'numa' variable > to track if libnuma has been initialized or not. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > src/cyclictest/cyclictest.c | 63 +++++++++++++++++-------------------- > src/cyclictest/rt_numa.h | 2 -- > 2 files changed, 29 insertions(+), 36 deletions(-) > > diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c > index f38c453f1975..514ed7b20fdb 100644 > --- a/src/cyclictest/cyclictest.c > +++ b/src/cyclictest/cyclictest.c > @@ -1018,9 +1018,6 @@ static void process_options(int argc, char *argv[], int max_cpus) > /* smp sets AFFINITY_USEALL in OPT_SMP */ > if (smp) > break; > - if (numa_initialize()) > - fatal("Couldn't initialize libnuma"); > - numa = 1; > if (optarg) { > parse_cpumask(optarg, max_cpus, &affinity_mask); > setaffinity = AFFINITY_SPECIFIED; > @@ -1126,8 +1123,6 @@ static void process_options(int argc, char *argv[], int max_cpus) > use_system = MODE_SYS_OFFSET; break; > case 'S': > case OPT_SMP: /* SMP testing */ > - if (numa) > - fatal("numa and smp options are mutually exclusive\n"); > smp = 1; > num_threads = -1; /* update after parsing */ > setaffinity = AFFINITY_USEALL; > @@ -1201,16 +1196,17 @@ static void process_options(int argc, char *argv[], int max_cpus) > > /* if smp wasn't requested, test for numa automatically */ > if (!smp) { > - if (numa_initialize()) > - fatal("Couldn't initialize libnuma"); > - numa = 1; > if (setaffinity == AFFINITY_UNSPECIFIED) > setaffinity = AFFINITY_USEALL; > } > > - if (option_affinity) { > - if (smp) > - warn("-a ignored due to smp mode\n"); > + if (option_affinity && smp) { > + warn("-a ignored due to smp mode\n"); > + if (affinity_mask) { > + numa_bitmask_free(affinity_mask); > + affinity_mask = NULL; > + } > + setaffinity = AFFINITY_USEALL; > } > > if (smi) { > @@ -1744,6 +1740,12 @@ int main(int argc, char **argv) > int max_cpus = sysconf(_SC_NPROCESSORS_ONLN); > int i, ret = -1; > int status; > + void *stack; > + void *currstk; > + size_t stksize; > + > + if (numa_initialize()) > + fatal("Couldn't initialize libnuma"); > > process_options(argc, argv, max_cpus); > > @@ -1926,34 +1928,27 @@ int main(int argc, char **argv) > default: cpu = -1; > } > > - node = -1; > - if (numa) { > - void *stack; > - void *currstk; > - size_t stksize; > + /* find the memory node associated with the cpu i */ > + node = rt_numa_numa_node_of_cpu(cpu); > > - /* find the memory node associated with the cpu i */ > - node = rt_numa_numa_node_of_cpu(cpu); > + /* get the stack size set for this thread */ > + if (pthread_attr_getstack(&attr, &currstk, &stksize)) > + fatal("failed to get stack size for thread %d\n", i); > > - /* get the stack size set for this thread */ > - if (pthread_attr_getstack(&attr, &currstk, &stksize)) > - fatal("failed to get stack size for thread %d\n", i); > + /* if the stack size is zero, set a default */ > + if (stksize == 0) > + stksize = PTHREAD_STACK_MIN * 2; > > - /* if the stack size is zero, set a default */ > - if (stksize == 0) > - stksize = PTHREAD_STACK_MIN * 2; > + /* allocate memory for a stack on appropriate node */ > + stack = rt_numa_numa_alloc_onnode(stksize, node, cpu); > > - /* allocate memory for a stack on appropriate node */ > - stack = rt_numa_numa_alloc_onnode(stksize, node, cpu); > + /* touch the stack pages to pre-fault them in */ > + memset(stack, 0, stksize); > > - /* touch the stack pages to pre-fault them in */ > - memset(stack, 0, stksize); > - > - /* set the thread's stack */ > - if (pthread_attr_setstack(&attr, stack, stksize)) > - fatal("failed to set stack addr for thread %d to 0x%x\n", > - i, stack+stksize); > - } > + /* set the thread's stack */ > + if (pthread_attr_setstack(&attr, stack, stksize)) > + fatal("failed to set stack addr for thread %d to 0x%x\n", > + i, stack+stksize); > > /* allocate the thread's parameter block */ > parameters[i] = par = threadalloc(sizeof(struct thread_param), node); > diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h > index 46690941e0a6..8d02f419ed6d 100644 > --- a/src/cyclictest/rt_numa.h > +++ b/src/cyclictest/rt_numa.h > @@ -13,8 +13,6 @@ > #include "rt-utils.h" > #include "error.h" > > -static int numa = 0; > - > #include <numa.h> > > static void * > -- > 2.29.2 > > Signed-off-by: John Kacur <jkacur@redhat.com>
Hi, On Fri Dec 18 2020, Daniel Wagner wrote: > libnuma is hard dependency for cyclictest. Thus we can always call > numa_initialize(). This allows us to remove the global 'numa' variable > to track if libnuma has been initialized or not. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> It seems like with this particular commit, it's not possible to run cyclictest on arm32 systems anymore. I guess due to missing NUMA support? Just tested on a dual core Cyclone V: root@tsn:~/rt-tests# ./cyclictest -S -m -p 99 --secaligned FATAL: Couldn't initialize libnuma root@tsn:~/rt-tests# I've used the current unstable/devel/latest branch. Any suggestions? Thanks, Kurt
Hi Kurt, On Fri, Feb 19, 2021 at 02:44:36PM +0100, Kurt Kanzenbach wrote: > It seems like with this particular commit, it's not possible to run > cyclictest on arm32 systems anymore. I guess due to missing NUMA > support? Yes, your distro needs to provide libnuma. cyclictest runs fine on arm32 with the library. > Just tested on a dual core Cyclone V: > > root@tsn:~/rt-tests# ./cyclictest -S -m -p 99 --secaligned > FATAL: Couldn't initialize libnuma I think you would see the same error when trying to use the '-a' option without the patch. The dependency is not new. > I've used the current unstable/devel/latest branch. Any suggestions? The simplest thing is obviously to get libnuma on your system. I assume this is not so simple in your case. In this case you could build cyclictest a static binary. First, build numactl as static libary: ./configure --enable-static && make and then rt-tests with CFLAGS="-static -L../numactl/.libs/" make Thanks, Daniel
Hi Daniel, On Fri Feb 19 2021, Daniel Wagner wrote: > Hi Kurt, > > On Fri, Feb 19, 2021 at 02:44:36PM +0100, Kurt Kanzenbach wrote: >> It seems like with this particular commit, it's not possible to run >> cyclictest on arm32 systems anymore. I guess due to missing NUMA >> support? > > Yes, your distro needs to provide libnuma. cyclictest runs fine on arm32 > with the library. I'm using Debian and it provides libnuma. > >> Just tested on a dual core Cyclone V: >> >> root@tsn:~/rt-tests# ./cyclictest -S -m -p 99 --secaligned >> FATAL: Couldn't initialize libnuma > > I think you would see the same error when trying to use the '-a' option > without the patch. The dependency is not new. > >> I've used the current unstable/devel/latest branch. Any suggestions? > > The simplest thing is obviously to get libnuma on your system. I assume > this is not so simple in your case. In this case you could build > cyclictest a static binary. > > First, build numactl as static libary: > > ./configure --enable-static && make > > and then rt-tests with > > CFLAGS="-static -L../numactl/.libs/" make Static building with the newest numactl library also doesn't work: |root@tsn:~/rt-tests# file cyclictest |cyclictest: ELF 32-bit LSB executable, ARM, EABI5 version 1 (GNU/Linux), statically linked, for GNU/Linux 3.2.0, BuildID[sha1]=248eaf8847544423dc51c6ceea18bbffc487991e, with debug_info, not stripped |root@tsn:~/rt-tests# ./cyclictest |FATAL: Couldn't initialize libnuma |root@tsn:~/rt-tests# Hmm. Thanks, Kurt
On Fri, Feb 19, 2021 at 03:39:45PM +0100, Kurt Kanzenbach wrote: > I'm using Debian and it provides libnuma. Hmm, that is strange. Indeed. I have to update my BBB first to a more recent Debian release to try this out. > Static building with the newest numactl library also doesn't work: > > |root@tsn:~/rt-tests# file cyclictest > |cyclictest: ELF 32-bit LSB executable, ARM, EABI5 version 1 (GNU/Linux), statically linked, for GNU/Linux 3.2.0, BuildID[sha1]=248eaf8847544423dc51c6ceea18bbffc487991e, with debug_info, not stripped > |root@tsn:~/rt-tests# ./cyclictest > |FATAL: Couldn't initialize libnuma I try to reproduce this here. Building libnuma from source right now.
On 2021-02-19 15:54:22 [+0100], Daniel Wagner wrote:
> I try to reproduce this here. Building libnuma from source right now.
|~# uname -m
|x86_64
|~# ./cyclictest
|FATAL: Couldn't initialize libnuma~#
Just do
# CONFIG_NUMA is not set
in your .config and it is done.
Sebastian
CONFIG_NUMA is not available on ARM32: https://cateee.net/lkddb/web-lkddb/NUMA.html regards, Christian On Friday, 19 February 2021, 16:17:17 CET, Sebastian Andrzej Siewior wrote: > On 2021-02-19 15:54:22 [+0100], Daniel Wagner wrote: > > I try to reproduce this here. Building libnuma from source right now. > > |~# uname -m > |x86_64 > |~# ./cyclictest > |FATAL: Couldn't initialize libnuma~# > > Just do > # CONFIG_NUMA is not set > > in your .config and it is done. > > Sebastian >
On Fri, Feb 19, 2021 at 04:21:46PM +0100, Christian Eggers wrote:
> CONFIG_NUMA is not available on ARM32:
libnuma is not happy if the kernel doesn't have the config
option. Well, in this case we just revert the patch I suppose.
On Fri, 19 Feb 2021, Daniel Wagner wrote: > On Fri, Feb 19, 2021 at 04:21:46PM +0100, Christian Eggers wrote: > > CONFIG_NUMA is not available on ARM32: > > libnuma is not happy if the kernel doesn't have the config > option. Well, in this case we just revert the patch I suppose. > Yeah, I'm not happy with this. To be sure Daniel did a lot of good work cleaning-up some of the numa calls and cleaning up the somewhat artificial smp / numa divide. That work needs to continue, but I want the ability to build cyclictest without NUMA, and that also means we need the small helpers. John
On Friday, 19 February 2021, 17:21:21 CET, John Kacur wrote: > > On Fri, 19 Feb 2021, Daniel Wagner wrote: > > > On Fri, Feb 19, 2021 at 04:21:46PM +0100, Christian Eggers wrote: > > > CONFIG_NUMA is not available on ARM32: > > > > libnuma is not happy if the kernel doesn't have the config > > option. Well, in this case we just revert the patch I suppose. > > > > Yeah, I'm not happy with this. > To be sure Daniel did a lot of good work cleaning-up some of the numa > calls and cleaning up the somewhat artificial smp / numa divide. > > That work needs to continue, but I want the ability to build cyclictest > without NUMA, and that also means we need the small helpers. Just for clarification: Does this mean that cyclictest shall link against libnuma (libnuma can be built on ARM without problems), but not call it functions at runtime? From the numa.h: /* NUMA support available. If this returns a negative value all other function in this library are undefined. */ int numa_available(void); regards, Christian
On 19.02.21 17:27, Christian Eggers wrote: > Just for clarification: Does this mean that cyclictest shall link against > libnuma (libnuma can be built on ARM without problems), but not call it > functions at runtime? Yes, I think that is the idea of libnuma.
On Fri, 19 Feb 2021, Christian Eggers wrote: > On Friday, 19 February 2021, 17:21:21 CET, John Kacur wrote: > > > > On Fri, 19 Feb 2021, Daniel Wagner wrote: > > > > > On Fri, Feb 19, 2021 at 04:21:46PM +0100, Christian Eggers wrote: > > > > CONFIG_NUMA is not available on ARM32: > > > > > > libnuma is not happy if the kernel doesn't have the config > > > option. Well, in this case we just revert the patch I suppose. > > > > > > > Yeah, I'm not happy with this. > > To be sure Daniel did a lot of good work cleaning-up some of the numa > > calls and cleaning up the somewhat artificial smp / numa divide. > > > > That work needs to continue, but I want the ability to build cyclictest > > without NUMA, and that also means we need the small helpers. > > Just for clarification: Does this mean that cyclictest shall link against > libnuma (libnuma can be built on ARM without problems), but not call it > functions at runtime? > > From the numa.h: > > /* NUMA support available. If this returns a negative value all other function > in this library are undefined. */ > int numa_available(void); > > regards, > Christian We used to be able to build cyclictest without libnuma. The small helpers that wrapped the calls to numa contained versions without numa calls. Most people did have libnuma, so we were requiring libnuma say on x86_64. I think at some point before Daniel starting removing the wrappers this support was broken. So, I mean the ability to build cyclitest without libnuma at least on some architectures. John
On Fri, 19 Feb 2021, Kurt Kanzenbach wrote: > Hi, > > On Fri Dec 18 2020, Daniel Wagner wrote: > > libnuma is hard dependency for cyclictest. Thus we can always call > > numa_initialize(). This allows us to remove the global 'numa' variable > > to track if libnuma has been initialized or not. Just a small note, the global 'numa' variable was NOT used to track whether libnuma had been intialized or not. The user used to specify smp or numa, then we decided that we would remove the numa option. What happened was that if numa was available and smp was not specified the program would automatically use numa else it would use smp. If smp was specified then smp was used. The numa variable would track that. the smp option used to just collect a common set of options -a -t and priorities all the same. It made sense way back when, but it's less useful now-a-days > > > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > > It seems like with this particular commit, it's not possible to run > cyclictest on arm32 systems anymore. I guess due to missing NUMA > support? > > Just tested on a dual core Cyclone V: > > root@tsn:~/rt-tests# ./cyclictest -S -m -p 99 --secaligned > FATAL: Couldn't initialize libnuma > root@tsn:~/rt-tests# > > I've used the current unstable/devel/latest branch. Any suggestions? > > Thanks, > Kurt >
On Fri, 19 Feb 2021, Christian Eggers wrote: > On Friday, 19 February 2021, 17:21:21 CET, John Kacur wrote: > > > > On Fri, 19 Feb 2021, Daniel Wagner wrote: > > > > > On Fri, Feb 19, 2021 at 04:21:46PM +0100, Christian Eggers wrote: > > > > CONFIG_NUMA is not available on ARM32: > > > > > > libnuma is not happy if the kernel doesn't have the config > > > option. Well, in this case we just revert the patch I suppose. > > > > > > > Yeah, I'm not happy with this. > > To be sure Daniel did a lot of good work cleaning-up some of the numa > > calls and cleaning up the somewhat artificial smp / numa divide. > > > > That work needs to continue, but I want the ability to build cyclictest > > without NUMA, and that also means we need the small helpers. > > Just for clarification: Does this mean that cyclictest shall link against > libnuma (libnuma can be built on ARM without problems), but not call it > functions at runtime? > > From the numa.h: > > /* NUMA support available. If this returns a negative value all other function > in this library are undefined. */ > int numa_available(void); On the other hand, this is what was working for you before the changes right? We required libnuma for building but not for runtime? That would be easier to maintain. > > regards, > Christian > > > >
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c index f38c453f1975..514ed7b20fdb 100644 --- a/src/cyclictest/cyclictest.c +++ b/src/cyclictest/cyclictest.c @@ -1018,9 +1018,6 @@ static void process_options(int argc, char *argv[], int max_cpus) /* smp sets AFFINITY_USEALL in OPT_SMP */ if (smp) break; - if (numa_initialize()) - fatal("Couldn't initialize libnuma"); - numa = 1; if (optarg) { parse_cpumask(optarg, max_cpus, &affinity_mask); setaffinity = AFFINITY_SPECIFIED; @@ -1126,8 +1123,6 @@ static void process_options(int argc, char *argv[], int max_cpus) use_system = MODE_SYS_OFFSET; break; case 'S': case OPT_SMP: /* SMP testing */ - if (numa) - fatal("numa and smp options are mutually exclusive\n"); smp = 1; num_threads = -1; /* update after parsing */ setaffinity = AFFINITY_USEALL; @@ -1201,16 +1196,17 @@ static void process_options(int argc, char *argv[], int max_cpus) /* if smp wasn't requested, test for numa automatically */ if (!smp) { - if (numa_initialize()) - fatal("Couldn't initialize libnuma"); - numa = 1; if (setaffinity == AFFINITY_UNSPECIFIED) setaffinity = AFFINITY_USEALL; } - if (option_affinity) { - if (smp) - warn("-a ignored due to smp mode\n"); + if (option_affinity && smp) { + warn("-a ignored due to smp mode\n"); + if (affinity_mask) { + numa_bitmask_free(affinity_mask); + affinity_mask = NULL; + } + setaffinity = AFFINITY_USEALL; } if (smi) { @@ -1744,6 +1740,12 @@ int main(int argc, char **argv) int max_cpus = sysconf(_SC_NPROCESSORS_ONLN); int i, ret = -1; int status; + void *stack; + void *currstk; + size_t stksize; + + if (numa_initialize()) + fatal("Couldn't initialize libnuma"); process_options(argc, argv, max_cpus); @@ -1926,34 +1928,27 @@ int main(int argc, char **argv) default: cpu = -1; } - node = -1; - if (numa) { - void *stack; - void *currstk; - size_t stksize; + /* find the memory node associated with the cpu i */ + node = rt_numa_numa_node_of_cpu(cpu); - /* find the memory node associated with the cpu i */ - node = rt_numa_numa_node_of_cpu(cpu); + /* get the stack size set for this thread */ + if (pthread_attr_getstack(&attr, &currstk, &stksize)) + fatal("failed to get stack size for thread %d\n", i); - /* get the stack size set for this thread */ - if (pthread_attr_getstack(&attr, &currstk, &stksize)) - fatal("failed to get stack size for thread %d\n", i); + /* if the stack size is zero, set a default */ + if (stksize == 0) + stksize = PTHREAD_STACK_MIN * 2; - /* if the stack size is zero, set a default */ - if (stksize == 0) - stksize = PTHREAD_STACK_MIN * 2; + /* allocate memory for a stack on appropriate node */ + stack = rt_numa_numa_alloc_onnode(stksize, node, cpu); - /* allocate memory for a stack on appropriate node */ - stack = rt_numa_numa_alloc_onnode(stksize, node, cpu); + /* touch the stack pages to pre-fault them in */ + memset(stack, 0, stksize); - /* touch the stack pages to pre-fault them in */ - memset(stack, 0, stksize); - - /* set the thread's stack */ - if (pthread_attr_setstack(&attr, stack, stksize)) - fatal("failed to set stack addr for thread %d to 0x%x\n", - i, stack+stksize); - } + /* set the thread's stack */ + if (pthread_attr_setstack(&attr, stack, stksize)) + fatal("failed to set stack addr for thread %d to 0x%x\n", + i, stack+stksize); /* allocate the thread's parameter block */ parameters[i] = par = threadalloc(sizeof(struct thread_param), node); diff --git a/src/cyclictest/rt_numa.h b/src/cyclictest/rt_numa.h index 46690941e0a6..8d02f419ed6d 100644 --- a/src/cyclictest/rt_numa.h +++ b/src/cyclictest/rt_numa.h @@ -13,8 +13,6 @@ #include "rt-utils.h" #include "error.h" -static int numa = 0; - #include <numa.h> static void *
libnuma is hard dependency for cyclictest. Thus we can always call numa_initialize(). This allows us to remove the global 'numa' variable to track if libnuma has been initialized or not. Signed-off-by: Daniel Wagner <dwagner@suse.de> --- src/cyclictest/cyclictest.c | 63 +++++++++++++++++-------------------- src/cyclictest/rt_numa.h | 2 -- 2 files changed, 29 insertions(+), 36 deletions(-)