add Linux numa memory policy support #1852
Conversation
Signed-off-by: Fabio M. Di Nitto <[email protected]>
Reviewer's GuideIntroduce optional NUMA support by adding a configure flag with header/library checks and updating build dependencies across documentation and CI Dockerfiles. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Ephemeral COPR build failed. @containers/packit-build please check. |
fabbione
force-pushed
the
linux-memory-policy
branch
from
9300607 to
bf06b93
Compare
Signed-off-by: Fabio M. Di Nitto <[email protected]>
Signed-off-by: Fabio M. Di Nitto <[email protected]>
fabbione
force-pushed
the
linux-memory-policy
branch
from
bf06b93 to
b94b5b1
Compare
Signed-off-by: Fabio M. Di Nitto <[email protected]>
fabbione
force-pushed
the
linux-memory-policy
branch
from
b94b5b1 to
87179df
Compare
Signed-off-by: Fabio M. Di Nitto <[email protected]>
fabbione
force-pushed
the
linux-memory-policy
branch
from
bc7f8a5 to
9a21680
Compare
|
I don't think we need another dependency for this feature. We can call directly the |
@giuseppe let´s talk when you are back from vacation :) that was one of the many questions I had in the list. |
|
Sure, but if there's anything I can answer now, feel free to ask about it :) |
not going to bother you while you are on vacation. Get off the computer :P |
fabbione
force-pushed
the
linux-memory-policy
branch
from
6ccae1d to
6f7e65c
Compare
Signed-off-by: Fabio M. Di Nitto <[email protected]>
Signed-off-by: Fabio M. Di Nitto <[email protected]>
fabbione
force-pushed
the
linux-memory-policy
branch
from
6f7e65c to
ebfeb36
Compare
|
There are 2 commits related to tests: that will be moved on a separate PR. At the moment I am just playing around with different ideas. |
fabbione
force-pushed
the
linux-memory-policy
branch
8 times, most recently
from
498679f to
4c16364
Compare
fabbione
force-pushed
the
linux-memory-policy
branch
2 times, most recently
from
dd89d67 to
13109fa
Compare
|
TMT tests failed. @containers/packit-build please check. |
fabbione
force-pushed
the
linux-memory-policy
branch
4 times, most recently
from
961fc8d to
0855812
Compare
|
Ephemeral COPR build failed. @containers/packit-build please check. |
mempolicy.c: - new file - add CPP info to match userland vs kernel interface versioning to avoid manual tracking of numaif interface - provides libcrun_set_mempolicy as wrapper to set_mempolicy(2) with all possible error checking and report that can be done before calling set_mempolicy(2). - use libnuma nodemask parser that is actively maintained by numa kernel maintainers to match kernel features and provides accurate error reports based on hw running the code (see also changes to error.c) mempolicy.h: - new file - define libcrun_set_mempolicy mempolicy_internal.h: - new file - define memory policy mode and flags maps to be shared and updated in one single place - numa python bindings are not available on most distros. This makes numa features detection challenging for crun test suite without causing false positives. This header provides shared common definitions with tests_mempolicy_helper.c that is used by the test suite and avoid duplicate code around container.c: - add call to libcrun_set_mempolicy in libcrun_container_run_internal error.c: - override numa_warn WEAK symbol from libnuma to capture numa parser errors and translate them into crun warnings tests/tests_mempolicy_helper.c: - new file - print a list of numa features detected during the build - shares info from mempolicy_internal.h tests/test_mempolicy.py: - new file - add both negative and positive tests for mempolicy.c - tests will run if hw supports numa or skip Makefile.am: - update - changes verified also with make distcheck Signed-off-by: Fabio M. Di Nitto <[email protected]>
fabbione
force-pushed
the
linux-memory-policy
branch
from
0855812 to
3b55373
Compare
fabbione
changed the title
|
@giuseppe this is for when you are back from vacation. I am not marking this PR ready to merge because there are some inconsistencies between the specs opencontainers/runtime-spec#1282 and set_mempolicy(2) documentation and I want to make sure everything is aligned. The specs marks "nodes" as (string, REQUIRED) but that is not 100% accurate. MPOL_DEFAULT and MPOL_LOCAL in fact requires to pass NULL, 0 as nodemask and nodemask_size, making nodes an unnecessary requirement. The implementation in opencontainers/runc#4726 in facts does not pass nodes to MPOL_DEFAULT and lacks a test for MPOL_LOCAL. The specs also define how to pass the nodemask string, that while effective, is a bit less flexible of the implementation in libnuma. libnuma allows for the keyword "all", negations with "!" and relative allocation with "+". My 2c would be to align around libnuma implementation here as it is more flexible and probably more visible when looking for "what options can I pass to NUMA nodemask?" Same feature, same implementations for all users? It could potentially improve the end user experience. The next bit is related on the general philosophy of crun coding, that is for me to understand. NUMA is one of the evolving kernel interfaces, as you might have noticed I did add several barriers to check for that, but then it boils down to how you want it supported. Should the crun implementation "not care" of the OS headers and redefine internally the values (duplicating code etc), to allow people to use any random kernel regardless of the OS provided headers, or should use the OS provided headers to match build / features and OS kernel? Once built with redefined values, some of the error checking become moot and we only get set_mempolicy errors with no idea what is wrong, that would make debugging configurations more challenging. NUMA being a rather advance setting, could potentially limit the execution of the container. In an homogeneous cluster where all nodes have same setup, this is a non-issue, but let´s take the example of real life cluster that expands over time and nodes have different NUMA properties. Would it make sense to have an extra flag in the spec for numa_strict or numa_relaxed? In a similar fashion, specially for ARM cpus, that have very different NUMA implementations if any at all. do we delegate the scheduling responsibility to upper levels or do we want to provide help to get stuff running? again goes back to the layering philosophy of containers here, where I am very much green. Thanks. PS: the last failures in CI are unrelated to those changes. some ssl certs expired in testing farms. |
Closes: #1844
Summary by Sourcery
Enable optional NUMA support in the build system and ensure libnuma is installed across documentation and test environments.
New Features:
Enhancements: