Messages in this thread | | | Date | Thu, 20 Oct 2016 20:03:06 -0700 | From | Davidlohr Bueso <> | Subject | Re: [PATCH 2/2 v2] perf bench futex: add NUMA support |
| |
On Mon, 17 Oct 2016, Sebastian Andrzej Siewior wrote:
>+#ifdef HAVE_LIBNUMA_SUPPORT >+#include <numa.h> >+#endif
In futex.h
>+static int numa_node = -1;
In futex.h (perhaps rename to futexbench_numa_node?)
>+#ifndef HAVE_LIBNUMA_SUPPORT >+static int numa_run_on_node(int node __maybe_unused) { return 0; } >+static int numa_node_of_cpu(int node __maybe_unused) { return 0; } >+static void *numa_alloc_local(size_t size) { return malloc(size); } >+static void numa_free(void *p, size_t size __maybe_unused) { return free(p); } >+#endif >+ >+static bool cpu_is_local(int cpu) >+{ >+ if (numa_node < 0) >+ return true; >+ if (numa_node_of_cpu(cpu) == numa_node) >+ return true; >+ return false; >+}
In futex.h
>- if (!nthreads) /* default to the number of CPUs */ >- nthreads = ncpus; >+ if (!nthreads) { >+ /* default to the number of CPUs per NUMA node */
This comment should go...
>+ if (numa_node < 0) { >+ nthreads = ncpus; >+ } else {
here.
>+ for (i = 0; i < ncpus; i++) { >+ if (cpu_is_local(i)) >+ nthreads++; >+ } >+ if (!nthreads) >+ err(EXIT_FAILURE, "No online CPUs for this node"); >+ } >+ } else { >+ int cpu_available = 0; > >- worker = calloc(nthreads, sizeof(*worker)); >+ for (i = 0; i < ncpus && !cpu_available; i++) { >+ if (cpu_is_local(i)) >+ cpu_available = 1; >+ }
Is this really necessary? If the user passes the number of threads, then we shouldn't care about ncpus; we just run all the threads on the specified node. Wouldn't the below numa_run_on_node() ensure that the node is not, for example, CPU-less?
>+ if (!cpu_available) >+ err(EXIT_FAILURE, "No online CPUs for this node"); >+ } >+ >+ if (numa_node >= 0) { >+ ret = numa_run_on_node(numa_node); >+ if (ret < 0) >+ err(EXIT_FAILURE, "numa_run_on_node");
Thanks, Davidlohr
| |