Add support for Linux memory policy #1282
Conversation
askervin
requested review from
AkihiroSuda,
crosbymichael,
cyphar,
dqminh,
giuseppe,
hqhq,
kolyshkin,
mrunalp,
thaJeztah,
tianon and
utam0k
as code owners
|
Small nit, I'd suggest to use |
|
LGTM after the changes suggested above |
askervin
force-pushed
the
5aD-oci-mempolicy
branch
from
be4b9f4 to
ee377f1
Compare
Thanks @kad, fixed. Definitely better. |
askervin
force-pushed
the
5aD-oci-mempolicy
branch
from
ee377f1 to
68936b6
Compare
There was a problem hiding this comment.
It seems set_mempolicy(2) is only effective agains for called threads. I'm not sure how to do it for processes created with exec. Any ideas?
set_mempolicy() sets the NUMA memory policy of the calling thread,
which consists of a policy mode and zero or more nodes, to the
values specified by the mode, nodemask, and maxnode arguments.
The behavior of several other system calls is the same, so it might be a good idea to define their behavior as well.
I'm not sure but we may need to implement it in nsexec.c of runc because of the thread limitation. I recommend to implement PoC in runc. |
the man page says: so I don't think it is a problem for the spec |
|
@giuseppe Oh, I missed it. Looks good. |
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod when OCI spec is merged Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod when OCI spec is merged Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod when OCI spec is merged Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod when OCI spec is merged Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod when OCI spec is merged Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod when OCI spec is merged Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod Signed-off-by: Antti Kervinen <[email protected]>
askervin
force-pushed
the
5aD-oci-mempolicy
branch
from
a722a43 to
fb37d43
Compare
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod Signed-off-by: Antti Kervinen <[email protected]>
Enable setting a NUMA memory policy for the container. New linux.memoryPolicy object contains inputs to the set_mempolicy(2) syscall. Signed-off-by: Antti Kervinen <[email protected]>
askervin
force-pushed
the
5aD-oci-mempolicy
branch
from
fb37d43 to
57c9495
Compare
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 TODO: - remove the replace from go.mod Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 Signed-off-by: Antti Kervinen <[email protected]>
| * `MPOL_PREFERRED_MANY` | ||
| * `MPOL_LOCAL` | ||
|
|
||
| * **`nodes`** *(string, REQUIRED)* - list of memory nodes from which nodemask is constructed to set_mempolicy(2). This is a comma-separated list, with dashes to represent ranges. For example, `0-3,7` represents memory nodes 0,1,2,3, and 7. |
There was a problem hiding this comment.
I think this might not be 100% correct. nodes is not required for MPOL_DEFAULT or MPOL_LOCAL, in fact set_mempolicy(2) explicitly mention to set nodes to NULL, 0 (even if some variants can work as well as long as nodes is empty).
There is also a question about parsing nodes strings. libnuma supports more modes than the ones described here. Including "all" (all nodes), "+" for relative and "!" for negation. Wouldn´t it be better to align the implementations to configure mempolicy to a common format/method and provide a better user interface?
There was a problem hiding this comment.
Thanks for pointing out the issue in documentation, @fabbione! Fixed in PR #1294.
Regarding the convenience syntax for nodes, I was arguing against it in opencontainers/runc#4726 (comment). Let's continue the discussion there, if you disagree.
There was a problem hiding this comment.
no problem at all, thanks for being so fast in fixing the docs etc.
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 Signed-off-by: Antti Kervinen <[email protected]>
|
|
||
| // Nodes representing the nodemask for the set_mempolicy syscall in comma separated ranges format. | ||
| // Format: "<node0>-<node1>,<node2>,<node3>-<node4>,..." | ||
| Nodes string `json:"nodes"` |
There was a problem hiding this comment.
Should this have been omitempty too?
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 Signed-off-by: Antti Kervinen <[email protected]>
Implement support for Linux memory policy in OCI spec PR: opencontainers/runtime-spec#1282 Signed-off-by: Antti Kervinen <[email protected]>
Enable setting a NUMA memory policy for the container. New linux.mempolicy object contains inputs to the set_mempolicy(2) syscall.