linux: teach uv_get_constrained_memory() cgroupsv2 #3744
Conversation
maleadt
left a comment
maleadt
left a comment
There was a problem hiding this comment.
The simplification here is that you assume cgroupv2 to be mounted at /sys/fs/cgroup, and avoid parsing mountinfo. That's not technically correct, but then again I haven't seen cgroupv2 be mounted anywhere else, so maybe it's fine to go with the simpler solution until somebody complains. The only exception is the hybrid cgroupv1+cgroupv2 set-up, which mounted the v2 hierarchy at /sys/fs/cgroup/unified, but that was a transitional solution, and the fallback still works.
|
So do we go ahead with this PR, @bnoordhuis? |
|
I'm waiting for a review from @vtjnash or another maintainer. |
|
Great, thanks @bnoordhuis! In addition to |
| sscanf(buf, "%" PRIu64, &rc); | ||
| if (1 != sscanf(buf, "%" PRIu64, &rc)) | ||
| if (0 == strcmp(buf, "max\n")) | ||
| rc = ~0ull; |
There was a problem hiding this comment.
@bnoordhuis I just noticed that this returns -1 when no limit is set. The documentation currently states that 0 is returned for both no and unknown limits. Do you want to keep it like this (being able to discern between both cases) and update the documentation, or return 0 from here as well?
There was a problem hiding this comment.
The documentation is ambiguously worded.
"If there is no such constraint, or the constraint is unknown" should say "if there is no constraining mechanism, or the constraint is unknown" but there is a mechanism in effect and the constraint is known, it's set to the max.
I will say that Node.js currently gets it wrong. It seems to assume that if uv_get_constrained_memory() returns non-zero, that implicitly means it's less than uv_get_free_memory() or uv_get_total_memory(). Not strictly always true.
There was a problem hiding this comment.
Is it OK to mention the return value for no limit/max in the documentation? I've updated #3753.
3 participants
Fixes: #2315
cc @maleadt - I figured it'd be easier to type it out than having you jump through more hoops.