linux-kernel - Re: [PATCH 2/2 v2] perf bench futex: add NUMA support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Oct 2016 20:03:06 -0700
From:   Davidlohr Bueso <dave@...olabs.net>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>, Jiri Olsa <jolsa@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        Davidlohr Bueso <dbueso@...e.de>
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