RFR: 8349988: Change cgroup version detection logic to not depend on /proc/cgroups [v4]
Thomas Fitzsimmons
duke at openjdk.org
Tue Apr 1 18:27:28 UTC 2025
On Tue, 1 Apr 2025 17:48:43 GMT, Ashutosh Mehra <asmehra at openjdk.org> wrote:
>> This doesn't make the code any better to read, IMO. The previous version seems better considering the non-mock code path isn't tested any more/less.
>
> @jerboaa @fitzsim I agree this doesn't look good.
> But I have another suggestion. Given that we now determine if we are in v2 or v1 by using `statfs` before calling `determine_type`, we should IMO rename `determine_type` to be something like `validate_and_populate_cgroup`.
> In addition to that, I also feel it would be better if the logic in `determine_type` can be broken down into two functions - one for v1 and another for v2. That would simplify the code and make it more readable as well.
> The tests can also be updated to call the function that corresponds to the cgroup version being tested.
> And I understand this is quite a bit of refactoring so I don't mind if this is done in a subsequent PR.
> What do you think?
> Apart from this I don't have any more comments. Rest looks good.
I agree these would be good subsequent changes. I tried some refactoring of `determine_type` into several functions while working on this patch but abandoned the effort to focus on the logic. I think after this patch, splitting `determine_type` will be a little easier (but still tricky -- the `cgroups v1`-versus-no-relevant-`cgroups` logic "wants" to be one long function, I suspect).
I will leave the subsequent PR up to @jerboaa if he wants to file a bug and assign it to me.
Now I will check if `master` needs re-merging, and push the test case change.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23811#discussion_r2023471782
More information about the hotspot-dev
mailing list