fs2: Fix statHugeTlb error when rsvd usage is present#24
fs2: Fix statHugeTlb error when rsvd usage is present#24kolyshkin merged 1 commit intoopencontainers:mainfrom
Conversation
e3d9c0a to
b502476
Compare
|
@AkihiroSuda @thaJeztah could you please take a look when you have a chance? Tests are passing locally. |
You are right, I've checked in my machine. |
|
hi @kolyshkin, @AkihiroSuda, @haircommander |
b502476 to
5e787da
Compare
kolyshkin
left a comment
There was a problem hiding this comment.
Overall looks good, left a single nit.
Also, please change libct/cg/fs2: to fs2: in commit subject, as this is where it lives now (used to be part of runc's libcontainer/cgroups but now it's a separate repo).
I'm against these tests though, they are very non-real and do not actually test anything. I'd rather have something like a simple bats tests which checks that hugetlb is available, when runs runc events --stats $CTID and checks that hugetlb is not empty.
5e787da to
c1ab7c5
Compare
|
Thank you, @kolyshkin. I've updated the code and reworded the commit message accordingly. Regarding the tests, they are ported from fs/hugetlb_test.go, and other fs2 modules follow a similar testing pattern. I agree they could be more realistic, and what you described sounds like a solid direction. Would you prefer that be addressed in this PR, or do you see it as a broader improvement for the library? Since there are currently no bats tests in the repo, introducing them would involve a fair amount of work on its own. I'm also open to removing the tests and limiting this PR to the bug fix, given that this module didn't have any tests to begin with. What are your thoughts? |
|
Yes, let's remove the test, and then I'll add one to runc repo. |
Signed-off-by: Gavin Lam <gavin.oss@tutamail.com>
c1ab7c5 to
94067f2
Compare
|
Thanks @kolyshkin, I've removed the tests. Please take another look. |
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
@kolyshkin @lifubang PTAL
|
@kolyshkin does it look good to you? shall we merge this please? |
As promised in opencontainers/cgroups#24 (review) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Better late than never, see opencontainers/runc#4898 |
As promised in opencontainers/cgroups#24 (review) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As promised in opencontainers/cgroups#24 (review) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
HugeTLB stats for cgroup v2 is currently not working when rsvd usage is present. When the
.rsvd.currentfile exists, the code attempts to read a.rsvd.eventsfile to obtain failcnt stat. However, this file does not exist, resulting in the following error.open /sys/fs/cgroup/pod_123.slice/pod_123-456.slice/hugetlb.2MB.rsvd.events: no such file or directoryI’ve verified in kernel source code that the cgroup v2 HugeTLB controller does not create a
.rsvd.eventsfile.https://github.com/torvalds/linux/blob/v6.15/mm/hugetlb_cgroup.c#L711-L756
P.S. This is blocking cri-o/cri-o#9257