As the title suggests, make omp_get_device_distances more
robust - fixing issues mentioned by Thomas.
In particular, it now handles a concurrent calls to the function
before the distance array has been filled. And it honors that some
systems prefer node 0 and 8 to node 0 and 1 (aka noncontiguous
node numbers).
And I found a new reason why numbering does not work:
'No NUMA configuration found' (reported by the kernel, e.g. via dmesg)
(I now get this on my laptop; I am pretty sure it worked three
weeks ago.) In that case, -1 is reported.
I tried it on an PowerPC system (that has '0,8' nodes) albeit with some
debug output as the PCI system does not contain NUMA data (that's
consistent with 'nvidia-smi topo -m' that also lacks that data). And I tried it on a
'0-1' node system with AMD GPUs. I also checked that busy wait works,
again adding some debug printf output. If virtualized - or numa is not
available for other reasons, '-1' is the reported distance by the kernel
- which GCC turns into '10' (default value of inner-node distance) for
the reported distance as documented at
https://gcc.gnu.org/onlinedocs/libgomp/omp_005fget_005fdevice_005fdistances.html
Comments before I commit this patch later today? Tobias PS: I believe
this patch addresses all follow-up remarks of the previous commit, cf.
also PR125877.
libgomp: Robustify omp_get_device_distances [PR125877]
libgomp/ChangeLog:
PR libgomp/125877
* config/linux/numa.c (gomp_get_numa_distance): Avoid data race issue
during initalization and handle non-contiguous node numbering.
libgomp/config/linux/numa.c | 186 ++++++++++++++++++++++++++++++++------------
1 file changed, 136 insertions(+), 50 deletions(-)
diff --git a/libgomp/config/linux/numa.c b/libgomp/config/linux/numa.c
index a3d84870e6e..85214e56dc4 100644
--- a/libgomp/config/linux/numa.c
+++ b/libgomp/config/linux/numa.c
@@ -37,9 +37,6 @@
#include <sys/syscall.h>
-static int num_numa_nodes = 0;
-static int *numa_distances = NULL;
-
int
gomp_get_current_numa_node ()
{
@@ -51,59 +48,148 @@ gomp_get_current_numa_node ()
int
gomp_get_numa_distance (int node1, int node2)
{
- if (node1 < 0 || node2 < 0 || num_numa_nodes < 0)
- return -1;
+ /* num_numa_nodes: 0 = uninit, -1 = being initialized, -2 = error,
+ > 0 = the number of numa nodes.
+ numa_distances: a 2D array of #nodes * #nodes with the distances.
+ remap: map the numa number to the index in numa_distances;
+ as numa numbers might not be contiguous.
+ num_remap: size of the remap array. */
+ static int num_numa_nodes = 0;
+ static int *numa_distances = NULL;
+ static unsigned num_remap = 0;
+ static unsigned *remap = NULL;
- if (numa_distances == NULL)
+retry:
+ int num = __atomic_load_n (&num_numa_nodes, MEMMODEL_RELAXED);
+ if (node1 < 0 || node2 < 0 || num <= -2)
+ return -1;
+ if (num > 0)
{
- num_numa_nodes = -1;
- DIR *dir = opendir ("/sys/devices/system/node");
- if (!dir)
+ if (node1 > num_remap || node2 > num_remap)
return -1;
- struct dirent *dp;
- int cnt = 0;
- errno = 0;
- while ((dp = readdir(dir)) != NULL)
- if (strncmp ("node", dp->d_name, 4 /* strlen ("node") */) == 0)
- cnt++;
- else if (errno)
- {
- closedir (dir);
- return -1;
- }
- closedir (dir);
- numa_distances = (int *) gomp_malloc (sizeof (int) * cnt * cnt);
+ node1 = remap[node1];
+ node2 = remap[node2];
+ if (node1 >= num || node2 >= num)
+ return -1;
+ return numa_distances[node1 * num_numa_nodes + node2];
+ }
+
+ /* Obtain distance data - or busy wait until another thread did so. */
+ int val = 0;
+ if (!__atomic_compare_exchange_n (&num_numa_nodes, &val, -1, false,
+ MEMMODEL_ACQUIRE, MEMMODEL_ACQUIRE))
+ {
+ while (-1 == __atomic_load_n (&num_numa_nodes, MEMMODEL_ACQUIRE));
+ __asm volatile ("" : : : "memory");
+ goto retry;
+ }
+
+ /* Obtain numa nodes. */
- constexpr int len = sizeof ("/sys/devices/system/node/node12345/"
- "distance");
- char filename[len];
+ num = 0;
+ FILE *f = fopen ("/sys/devices/system/node/online", "r");
+ if (f == NULL)
+ goto fail;
+ char *lineptr = NULL;
+ size_t nline;
+ if (getline (&lineptr, &nline, f) <= 0)
+ {
+ free (lineptr);
+ fclose (f);
+ goto fail;
+ }
+ fclose (f);
+
+ /* Create mapping array between the node number and contiguous
+ n-th online node count. */
+ unsigned constexpr UNSET_REMAP = (unsigned) -1;
+ num_remap = 8;
+ remap = malloc (sizeof (*remap) * num_remap);
+ if (!remap)
+ goto fail;
+ for (int i = 0; i < num_remap; i++)
+ remap[i] = UNSET_REMAP;
+ char *q = lineptr;
+ while (*q && *q != '\n')
+ {
+ unsigned nfirst, nlast;
+ char *end;
- for (int i = 0; i < cnt; i++)
+ errno = 0;
+ nfirst = strtoul (q, &end, 10);
+ if (errno || end == q)
+ goto fail;
+ q = end;
+ nlast = nfirst;
+ if (*q == '-')
+ {
+ errno = 0;
+ nlast = strtoul (q + 1, &end, 10);
+ if (errno || end == q + 1 || nlast < nfirst)
+ goto fail;
+ q = end;
+ }
+ if (num_remap <= nlast)
{
- if (len < snprintf (filename, sizeof (filename),
- "/sys/devices/system/node/node%d/distance", i))
- return -1;
- int distance = -1;
- FILE *in = fopen (filename, "r");
- if (!in)
- {
- free (numa_distances);
- return -1;
- }
- for (int j = 0; j < cnt; j++)
- {
- fscanf (in, "%d", &distance);
- if (distance == -1)
- {
- fclose (in);
- free (numa_distances);
- return -1;
- }
- numa_distances[i * cnt + j] = distance;
- }
- fclose (in);
+ int n = nlast > 2 * num_remap ? nlast + 1 : num_remap * 2;
+ remap = realloc (remap, sizeof (*remap) * n);
+ if (!remap || n <= nlast)
+ goto fail;
+ for (int i = num_remap; i < n; i++)
+ remap[i] = UNSET_REMAP;
+ num_remap = n;
}
- num_numa_nodes = cnt;
+ for (; nfirst <= nlast; nfirst++)
+ remap[nfirst] = num++;
+ if (*q == ',')
+ ++q;
}
- return numa_distances[node1 * num_numa_nodes + node2];
+ free (lineptr);
+ lineptr = NULL;
+
+ /* Fill distance data. */
+ numa_distances = (int *) gomp_malloc (sizeof (*numa_distances) * num * num);
+
+ for (int i = 0; i < num_remap; i++)
+ if (remap[i] == UNSET_REMAP)
+ continue;
+ else
+ {
+ constexpr int len = sizeof ("/sys/devices/system/node/node12345/"
+ "distance");
+ char filename[len];
+ int node = remap[i];
+ if (node < 0)
+ goto fail;
+ if (len < snprintf (filename, sizeof (filename),
+ "/sys/devices/system/node/node%d/distance", i))
+ goto fail;
+ FILE *in = fopen (filename, "r");
+ if (!in)
+ goto fail;
+ for (int j = 0; j < num; j++)
+ {
+ /* Distance -1 might happen on virtualized systems or when the
+ kernel reports "No NUMA configuration found". The code assumes
+ that only distance values show up for online nodes. */
+ int distance;
+ if (fscanf (in, "%d", &distance) != 1)
+ {
+ fclose (in);
+ goto fail;
+ }
+ numa_distances[node * num + j] = distance;
+ }
+ fclose (in);
+ }
+ __asm volatile ("" : : : "memory");
+ __atomic_store_n (&num_numa_nodes, num, MEMMODEL_RELAXED);
+ goto retry;
+
+fail:
+ free (lineptr);
+ free (remap);
+ free (numa_distances);
+ __atomic_store_n (&num_numa_nodes, -2, MEMMODEL_RELAXED);
+ return -1;
}
