On Wed, 23 Jun 2021 13:37:59 GMT, Matthias Baesken <mbaes...@openjdk.org> wrote:

>> Hello, please review this PR; it extend the OSContainer API in order to also 
>> support the pids controller of cgroups.
>> 
>> I noticed that unlike the other controllers "cpu", "cpuset", "cpuacct", 
>> "memory"  on some older Linux distros (SLES 12.1, RHEL 7.1) the pids 
>> controller might not be there (or not fully supported) so it was added as 
>> optional  , see the coding
>> 
>> 
>>   if (!cg_infos[PIDS_IDX]._data_complete) {
>>     log_debug(os, container)("Optional cgroup v1 pids subsystem not found");
>>     // keep the other controller info, pids is optional
>>   }
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Adjustments following Severins comments

Hi Severin , thanks for all the comments.   I prepared a second version with 
those changes
added a couple of log_is_enabled checks like you suggested
moved limit_from_str to CgroupSubsystem
added helpers pids_max_val()  and swicthed to GET_CONTAINER_INFO_CPTR
pids_max() now returns -1 for unlimited/max , and the -3 is gone
moved limitFromString java coding to 
src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystem.java
added a better comment to 
test/hotspot/jtreg/containers/cgroup/CgroupSubsystemFactory.java about pids 
hiearchy values

Regarding your questions about tests, I run the exisiting docker/cgroup related 
tests; and also checked 
the hs_err output (on SLES/Ubuntu)  for new added "maximum number of tasks"  
(this is present because systemd cgroup usage).
But I think that the testing needs to be enhanced (e.g. with some added docker 
tests?).  Do you have some good suggestions
where I could look at existing (docker?) tests and  adjust those for the new 
pids.max ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/4518

Reply via email to