2020-06-29 18:39:16

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 1/3] perf bench numa: fix incorrect NUMA toplogy assumption

The current code assumes that CPUs are evenly spread among
NUMA nodes. That is generally incorrect and leads to failure
on systems that have different NUMA topology.

Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
tools/perf/bench/numa.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/tools/perf/bench/numa.c b/tools/perf/bench/numa.c
index 5797253..5497c74 100644
--- a/tools/perf/bench/numa.c
+++ b/tools/perf/bench/numa.c
@@ -247,12 +247,13 @@ static int is_node_present(int node)
*/
static bool node_has_cpus(int node)
{
- struct bitmask *cpu = numa_allocate_cpumask();
+ struct bitmask *cpumask = numa_allocate_cpumask();
unsigned int i;

- if (cpu && !numa_node_to_cpus(node, cpu)) {
- for (i = 0; i < cpu->size; i++) {
- if (numa_bitmask_isbitset(cpu, i))
+ BUG_ON(cpumask);
+ if (!numa_node_to_cpus(node, cpumask)) {
+ for (i = 0; i < cpumask->size; i++) {
+ if (numa_bitmask_isbitset(cpumask, i))
return true;
}
}
@@ -288,14 +289,10 @@ static cpu_set_t bind_to_cpu(int target_cpu)

static cpu_set_t bind_to_node(int target_node)
{
- int cpus_per_node = g->p.nr_cpus / nr_numa_nodes();
cpu_set_t orig_mask, mask;
int cpu;
int ret;

- BUG_ON(cpus_per_node * nr_numa_nodes() != g->p.nr_cpus);
- BUG_ON(!cpus_per_node);
-
ret = sched_getaffinity(0, sizeof(orig_mask), &orig_mask);
BUG_ON(ret);

@@ -305,13 +302,15 @@ static cpu_set_t bind_to_node(int target_node)
for (cpu = 0; cpu < g->p.nr_cpus; cpu++)
CPU_SET(cpu, &mask);
} else {
- int cpu_start = (target_node + 0) * cpus_per_node;
- int cpu_stop = (target_node + 1) * cpus_per_node;
-
- BUG_ON(cpu_stop > g->p.nr_cpus);
+ struct bitmask *cpumask = numa_allocate_cpumask();

- for (cpu = cpu_start; cpu < cpu_stop; cpu++)
- CPU_SET(cpu, &mask);
+ BUG_ON(!cpumask);
+ if (!numa_node_to_cpus(target_node, cpumask)) {
+ for (cpu = 0; cpu < (int)cpumask->size; cpu++) {
+ if (numa_bitmask_isbitset(cpumask, cpu))
+ CPU_SET(cpu, &mask);
+ }
+ }
}

ret = sched_setaffinity(0, sizeof(mask), &mask);
--
1.8.3.1


2020-06-30 16:19:30

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf bench numa: fix incorrect NUMA toplogy assumption

On Mon, Jun 29, 2020 at 05:50:07PM +0200, Alexander Gordeev wrote:
> The current code assumes that CPUs are evenly spread among
> NUMA nodes. That is generally incorrect and leads to failure
> on systems that have different NUMA topology.
>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Signed-off-by: Alexander Gordeev <[email protected]>
> ---
> tools/perf/bench/numa.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/bench/numa.c b/tools/perf/bench/numa.c
> index 5797253..5497c74 100644
> --- a/tools/perf/bench/numa.c
> +++ b/tools/perf/bench/numa.c
> @@ -247,12 +247,13 @@ static int is_node_present(int node)
> */
> static bool node_has_cpus(int node)
> {
> - struct bitmask *cpu = numa_allocate_cpumask();
> + struct bitmask *cpumask = numa_allocate_cpumask();
> unsigned int i;
>
> - if (cpu && !numa_node_to_cpus(node, cpu)) {
> - for (i = 0; i < cpu->size; i++) {
> - if (numa_bitmask_isbitset(cpu, i))
> + BUG_ON(cpumask);

self-nack - should have been !cpumask,
will send the fixed version

> + if (!numa_node_to_cpus(node, cpumask)) {
> + for (i = 0; i < cpumask->size; i++) {
> + if (numa_bitmask_isbitset(cpumask, i))
> return true;
> }
> }
> @@ -288,14 +289,10 @@ static cpu_set_t bind_to_cpu(int target_cpu)
>
> static cpu_set_t bind_to_node(int target_node)
> {
> - int cpus_per_node = g->p.nr_cpus / nr_numa_nodes();
> cpu_set_t orig_mask, mask;
> int cpu;
> int ret;
>
> - BUG_ON(cpus_per_node * nr_numa_nodes() != g->p.nr_cpus);
> - BUG_ON(!cpus_per_node);
> -
> ret = sched_getaffinity(0, sizeof(orig_mask), &orig_mask);
> BUG_ON(ret);
>
> @@ -305,13 +302,15 @@ static cpu_set_t bind_to_node(int target_node)
> for (cpu = 0; cpu < g->p.nr_cpus; cpu++)
> CPU_SET(cpu, &mask);
> } else {
> - int cpu_start = (target_node + 0) * cpus_per_node;
> - int cpu_stop = (target_node + 1) * cpus_per_node;
> -
> - BUG_ON(cpu_stop > g->p.nr_cpus);
> + struct bitmask *cpumask = numa_allocate_cpumask();
>
> - for (cpu = cpu_start; cpu < cpu_stop; cpu++)
> - CPU_SET(cpu, &mask);
> + BUG_ON(!cpumask);
> + if (!numa_node_to_cpus(target_node, cpumask)) {
> + for (cpu = 0; cpu < (int)cpumask->size; cpu++) {
> + if (numa_bitmask_isbitset(cpumask, cpu))
> + CPU_SET(cpu, &mask);
> + }
> + }
> }
>
> ret = sched_setaffinity(0, sizeof(mask), &mask);
> --
> 1.8.3.1
>

2020-06-30 16:21:11

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH v2 1/3] perf bench numa: fix incorrect NUMA toplogy assumption

The current code assumes that CPUs are evenly spread among
NUMA nodes. That is generally incorrect and leads to failure
on systems that have different NUMA topology.

Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
tools/perf/bench/numa.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/tools/perf/bench/numa.c b/tools/perf/bench/numa.c
index 5797253..23e224e 100644
--- a/tools/perf/bench/numa.c
+++ b/tools/perf/bench/numa.c
@@ -247,12 +247,13 @@ static int is_node_present(int node)
*/
static bool node_has_cpus(int node)
{
- struct bitmask *cpu = numa_allocate_cpumask();
+ struct bitmask *cpumask = numa_allocate_cpumask();
unsigned int i;

- if (cpu && !numa_node_to_cpus(node, cpu)) {
- for (i = 0; i < cpu->size; i++) {
- if (numa_bitmask_isbitset(cpu, i))
+ BUG_ON(!cpumask);
+ if (!numa_node_to_cpus(node, cpumask)) {
+ for (i = 0; i < cpumask->size; i++) {
+ if (numa_bitmask_isbitset(cpumask, i))
return true;
}
}
@@ -288,14 +289,10 @@ static cpu_set_t bind_to_cpu(int target_cpu)

static cpu_set_t bind_to_node(int target_node)
{
- int cpus_per_node = g->p.nr_cpus / nr_numa_nodes();
cpu_set_t orig_mask, mask;
int cpu;
int ret;

- BUG_ON(cpus_per_node * nr_numa_nodes() != g->p.nr_cpus);
- BUG_ON(!cpus_per_node);
-
ret = sched_getaffinity(0, sizeof(orig_mask), &orig_mask);
BUG_ON(ret);

@@ -305,13 +302,15 @@ static cpu_set_t bind_to_node(int target_node)
for (cpu = 0; cpu < g->p.nr_cpus; cpu++)
CPU_SET(cpu, &mask);
} else {
- int cpu_start = (target_node + 0) * cpus_per_node;
- int cpu_stop = (target_node + 1) * cpus_per_node;
-
- BUG_ON(cpu_stop > g->p.nr_cpus);
+ struct bitmask *cpumask = numa_allocate_cpumask();

- for (cpu = cpu_start; cpu < cpu_stop; cpu++)
- CPU_SET(cpu, &mask);
+ BUG_ON(!cpumask);
+ if (!numa_node_to_cpus(target_node, cpumask)) {
+ for (cpu = 0; cpu < (int)cpumask->size; cpu++) {
+ if (numa_bitmask_isbitset(cpumask, cpu))
+ CPU_SET(cpu, &mask);
+ }
+ }
}

ret = sched_setaffinity(0, sizeof(mask), &mask);
--
1.8.3.1