RFR: 8262953: [TESTBUG] Test /runtime/containers/cgroup/PlainRead.java fails if CPU quoata is not enabled

[David Holmes]

On 19/04/2021 7:05 pm, Severin Gehwolf wrote:
> On Thu, 25 Mar 2021 20:11:30 GMT, Anton Ostrouhhov <github.com+22177208+aostrouhhov at openjdk.org> wrote:
> 
>> Hello!
>> Both _CPU Quota_ and _CPU Period_ are missing from output if kernel was built without `CONFIG_CFS_BANDWIDTH`.
>>
>> This is my first contribution so please tell me if i am wrong somewhere.
> 
>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-runtime-dev](mailto:hotspot-runtime-dev at mail.openjdk.java.net):_
>>
>> On 16/04/2021 10:26 pm, Severin Gehwolf wrote:
>>
>>> On Fri, 16 Apr 2021 12:18:05 GMT, David Holmes <dholmes at openjdk.org> wrote:
>>>> I'm not sure my, simple, point is getting through. Here's the cgroupv1 code:
>>>> ```
>>>> /* cpu_quota
>>>> *
>>>> * Return the number of microseconds per period
>>>> * process is guaranteed to run.
>>>> *
>>>> * return:
>>>> *    quota time in microseconds
>>>> *    -1 for no quota
>>>> *    OSCONTAINER_ERROR for not supported
>>>> */
>>>> int CgroupV1Subsystem::cpu_quota() {
>>>> GET_CONTAINER_INFO(int, _cpu->controller(), "/cpu.cfs_quota_us",
>>>> "CPU Quota is: %d", "%d", quota);
>>>> return quota;
>>>> }
>>>> ```
>>>> But if GET_CONTAINER_INFO doesn't find /cpu.cfs_quota_us the macro does "return OSCONTAINER_ERROR" which is "return -2". Yet the spec states that if there is no quota then -1 is returned, so why are we not returning -1?
>>>
>>>
>>> @dholmes-ora Because they're not equivalent. `-1` means, the system supports it in general, but no cgroups quota is in place. I.e. unlimited. For example A) `docker run --memory 200M <somejavacontainer>` on a cgroup capable system with all the kernel support should report the memory limit. B) `docker run <somejavacontainer>`. Case B) would return -1. `-2` is usually something unexpected and means the system doesn't support it (be it by kernel config, the file not being present where a file is expected to be present, etc.). We want to distinguish those two cases.
>>
>> I don't see why the user cares about this distinction.
> 
> That's how it's presented to the user (no distinction).
> 
> This mapping is being done in function `CgroupSubsystem::active_processor_count` here:
> https://github.com/openjdk/jdk/blob/master/src/hotspot/os/linux/cgroupSubsystem_linux.cpp#L442
> 
> I might have done a poor job at describing why we **internally** want to distinguish those two cases. Note that, we are discussing this in the context of a test failure. The test seems to assume that if cgroups is available (hotspot requires `memory`, `cpu`, `cpuset`, `cpuacct` and all seem present), the assumption is that `CONFIG_CFS_BANDWIDTH` is on too.
> 
>> We might care for
>> logging purposes but that is different.
> 
> It's this exact issue, is it not? The test uses container trace logging to assert quotas are being recognized (even for the plain cgroup case). It doesn't account for the `CONFIG_CFS_BANDWIDTH=no` case apparently.

Sorry I wasn't clear. The logging would distinguish the exact case 
not-supported versus not-set, but would still report something.

David

>> There is no difference to the
>> end user between cgroups defines no quota and cgroups quotas are not
>> compiled into the kernel.
> 
> Agreed.
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/3206
>