UCS/TOPO: Replace numa_distance with sysfs based implementation by ofirfarjun7 · Pull Request #8698 · openucx/ucx

ofirfarjun7 · 2022-11-13T17:03:38Z

What

Implement numa_distance function to calculate the distance between IB devices and CPU's NUMA node.

Why ?

Remove UCX libnuma dependency

How ?

Extend Topo providers API to include method for calculating NUMA distance between IB devices and CPU's NUMA node.
Remove/Replace libnuma dependent code.
Remove libnuma from UCX

src/ucs/memory/numa.c

ofirfarjun7 · 2022-12-13T09:50:36Z

test/apps/uct_info/Makefile.in


-UCX_STATIC_LDFLAGS = -static $(shell pkg-config --libs --static $(EXTRA_MODULES) ucx) -lnuma
-UCT_STATIC_LDFLAGS = -static $(shell pkg-config --libs --static $(EXTRA_MODULES) ucx-uct) -lnuma
+UCX_STATIC_LDFLAGS = -static $(shell pkg-config --libs --static $(EXTRA_MODULES) ucx) $(NUMA_LIBS)


Need to revert in the following PR and remove linking with libnuma from all makefiles

gleon99 · 2023-01-10T10:01:45Z

PR description:

"numa distance" -> numa_distance
numa -> NUMA
libnuma -> libnuma

src/ucs/memory/numa.c

src/ucs/sys/topo/base/topo.c

src/ucs/sys/topo/base/topo.h

test/gtest/ucs/test_topo.cc

src/ucs/memory/numa.c

src/ucs/sys/init.c

gleon99 · 2023-02-12T11:26:10Z

src/ucs/sys/topo/base/topo.c

+        return ucs_topo_get_avg_distance_default(device, distance);
+    }
+
+    ret = ucs_sys_getaffinity(&process_affinity);


Need to use thread affinity instead (pthread_getaffinity_np).
Please add a func in ucs_sys, reuse in uct_ib_md_handle_mr_list_multithreaded.

@ofirfarjun7 unresolved?

src/ucs/sys/topo/base/topo.h

src/ucs/sys/sys.h

src/ucs/memory/numa.c

src/ucs/memory/numa_defs.h

src/ucs/memory/numa.c

yosefe · 2023-03-25T15:32:29Z

src/ucs/memory/numa_defs.h

+
+#include <stdint.h>
+
+#define UCS_NUMA_MIN_DISTANCE 10


why does it need to be in H file?

We have references to this macro in other files (topo.c)
Need to remove from numa.h (leftovers...)

I would expect topo.c to use ucs_topo_default_distance

src/ucs/memory/numa_defs.h

src/ucs/memory/numa.c

src/ucs/sys/topo/base/topo.c

test/apps/uct_info/Makefile.in

src/ucs/memory/numa.c

src/ucs/memory/numa.h

src/ucs/sys/sys.c

src/ucs/sys/topo/base/topo.c

test/apps/uct_info/Makefile.in

src/ucs/memory/numa.c

src/ucs/sys/topo/base/topo.c

yosefe · 2023-04-03T07:19:57Z

src/ucs/sys/sys.c


 static ucs_status_t
-ucs_sys_enum_threads_cb(const struct dirent *entry, void *_ctx)
+ucs_sys_enum_threads_cb(const struct dirent *entry, void *arg)


rename it also in H file

src/ucs/sys/topo/base/topo.c

test/apps/uct_info/Makefile.in

src/ucs/sys/sys.c

src/ucs/sys/topo/base/topo.c

test/gtest/ucs/test_topo.cc

gleon99

If already committing some minor fixes, please go over the docstrings - I think there are several places with "excessive spacing" (args, return, etc).

yosefe · 2023-04-03T15:22:38Z

src/uct/ib/base/ib_md.c

    if (ret != 0) {
-        ucs_error("pthread_getaffinity_np() failed: %m");
-        return UCS_ERR_INVALID_PARAM;
+        return ret;


"int ret" -> ucs_status_t status
!= 0 -> != UCS_OK

gleon99 · 2023-04-04T11:15:31Z

@ofirfarjun7 please squash.

ofirfarjun7 added the WIP-DNM Work in progress / Do not review label Nov 13, 2022

ofirfarjun7 force-pushed the topic/topo-numa-distance branch from 80a3b9a to 7bb6d56 Compare November 13, 2022 17:07

ofirfarjun7 force-pushed the topic/topo-numa-distance branch 8 times, most recently from f5b41df to 479b0d4 Compare November 29, 2022 08:51

gleon99 reviewed Nov 29, 2022

View reviewed changes

src/ucs/memory/numa.c Show resolved Hide resolved

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

ofirfarjun7 commented Dec 4, 2022

View reviewed changes

src/ucs/memory/numa.c Show resolved Hide resolved

ofirfarjun7 commented Dec 4, 2022

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

gleon99 reviewed Dec 5, 2022

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

ofirfarjun7 force-pushed the topic/topo-numa-distance branch 3 times, most recently from 9ff60ba to cd5254a Compare December 13, 2022 09:19

ofirfarjun7 marked this pull request as ready for review December 13, 2022 09:46

ofirfarjun7 removed the WIP-DNM Work in progress / Do not review label Dec 13, 2022

ofirfarjun7 commented Dec 13, 2022

View reviewed changes

gleon99 reviewed Jan 23, 2023

View reviewed changes

gleon99 reviewed Jan 24, 2023

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

gleon99 reviewed Feb 5, 2023

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

gleon99 reviewed Feb 6, 2023

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

gleon99 reviewed Feb 6, 2023

View reviewed changes

gleon99 reviewed Feb 12, 2023

View reviewed changes

src/ucs/sys/topo/base/topo.h Outdated Show resolved Hide resolved

src/ucs/sys/topo/base/topo.h Outdated Show resolved Hide resolved

ofirfarjun7 requested a review from gleon99 February 19, 2023 11:20

gleon99 reviewed Mar 12, 2023

View reviewed changes

src/ucs/sys/topo/base/topo.h Outdated Show resolved Hide resolved

gleon99 reviewed Mar 22, 2023

View reviewed changes

gleon99 reviewed Mar 23, 2023

View reviewed changes

src/ucs/memory/numa_defs.h Outdated Show resolved Hide resolved

src/ucs/memory/numa_defs.h Outdated Show resolved Hide resolved

src/ucs/memory/numa_defs.h Outdated Show resolved Hide resolved

src/ucs/memory/numa_defs.h Outdated Show resolved Hide resolved

gleon99 reviewed Mar 23, 2023

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

gleon99 reviewed Mar 23, 2023

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

gleon99 reviewed Mar 23, 2023

View reviewed changes

src/ucs/memory/numa.c Outdated Show resolved Hide resolved

src/ucs/memory/numa.c Show resolved Hide resolved

yosefe reviewed Mar 25, 2023

View reviewed changes

ofirfarjun7 requested a review from gleon99 March 26, 2023 14:59

yosefe reviewed Apr 2, 2023

View reviewed changes

yosefe reviewed Apr 3, 2023

View reviewed changes

yosefe previously approved these changes Apr 3, 2023

View reviewed changes

gleon99 reviewed Apr 3, 2023

View reviewed changes

src/ucs/sys/sys.c Outdated Show resolved Hide resolved

gleon99 reviewed Apr 3, 2023

View reviewed changes

src/ucs/sys/topo/base/topo.c Outdated Show resolved Hide resolved

gleon99 reviewed Apr 3, 2023

View reviewed changes

test/gtest/ucs/test_topo.cc Show resolved Hide resolved

gleon99 reviewed Apr 3, 2023

View reviewed changes

ofirfarjun7 dismissed yosefe’s stale review via f1ff0a2 April 3, 2023 15:17

yosefe reviewed Apr 3, 2023

View reviewed changes

gleon99 previously approved these changes Apr 4, 2023

View reviewed changes

ofirfarjun7 dismissed gleon99’s stale review via ef439b0 April 4, 2023 12:59

ofirfarjun7 force-pushed the topic/topo-numa-distance branch from 66098b2 to ef439b0 Compare April 4, 2023 12:59

ofirfarjun7 added the WIP-DNM Work in progress / Do not review label Apr 4, 2023

UCS/TOPO: Replace numa_distance with sysfs based implementation

c21b5bb

ofirfarjun7 force-pushed the topic/topo-numa-distance branch from ef439b0 to c21b5bb Compare April 4, 2023 13:15

ofirfarjun7 removed the WIP-DNM Work in progress / Do not review label Apr 4, 2023

ofirfarjun7 requested a review from gleon99 April 4, 2023 13:16

yosefe approved these changes Apr 5, 2023

View reviewed changes

yosefe enabled auto-merge April 5, 2023 08:19

yosefe merged commit 80c0edb into openucx:master Apr 5, 2023

pentschev mentioned this pull request Sep 25, 2024

Add UCC support conda-forge/openmpi-feedstock#175

Merged

5 tasks


		#include <stdint.h>

		#define UCS_NUMA_MIN_DISTANCE 10

Conversation

ofirfarjun7 commented Nov 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why ?

How ?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ofirfarjun7 Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

gleon99 commented Jan 10, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gleon99 Feb 12, 2023

Choose a reason for hiding this comment

Uh oh!

gleon99 Mar 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yosefe Mar 25, 2023

Choose a reason for hiding this comment

Uh oh!

ofirfarjun7 Mar 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yosefe Apr 2, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ofirfarjun7 commented Nov 13, 2022 •

edited

Loading

gleon99 Mar 21, 2023 •

edited

Loading

ofirfarjun7 Mar 25, 2023 •

edited

Loading