RFR: 8170888: [linux] Experimental support for cgroup memory limits in container (ie Docker) environments

[David Holmes]

Updated webrev at:

http://cr.openjdk.java.net/~dholmes/8170888/webrev.v2/

Now checks fscanf ret code. Plus fixed 'else' formatting.

Thanks,
David
-----

+       int ret = fscanf(fp, JULONG_FORMAT, &cgroup_max);
+       if (ret == 1 && cgroup_max > 0) {
+         // If unlimited, cgroup_max will be a very large, but unspecified
+         // value, so use initial phys_mem as a limit
+         log_info(gc, heap)("Setting phys_mem to the min of cgroup limit ("
+                            JULONG_FORMAT "MB) and initial phys_mem ("
+                            JULONG_FORMAT "MB)", cgroup_max/M, phys_mem/M);
+         phys_mem = MIN2(cgroup_max, phys_mem);
+       } else {
+         warning("Unable to read/parse cgroup memory limit from %s: %s",
+                 lim_file, errno != 0 ? strerror(errno) : "unknown error");
+       }



On 10/12/2016 3:00 PM, Kim Barrett wrote:
>> On Dec 9, 2016, at 8:24 PM, David Holmes <david.holmes at oracle.com> wrote:
>>
>> Hi Kim,
>>
>> Thanks for looking at this.
>>
>> On 10/12/2016 10:22 AM, Kim Barrett wrote:
>>>> On Dec 8, 2016, at 9:59 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170888
>>>>
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8170888/webrev/
>>>
>>> ------------------------------------------------------------------------------
>>> src/share/vm/runtime/arguments.cpp
>>> 2050       fscanf(fp, JULONG_FORMAT, &cgroup_max);
>>> 2051       if (cgroup_max > 0) {
>>>
>>> Missing check of fscanf result.  I don't see anything in the doc for
>>> fscanf that promises an argument won't be clobbered if there are match
>>> failures, errors, or fewer inputs than specified in the format string.
>>> Even if that no such clobbering happens, as written we have a really
>>> rare failure mode -- in cgroup context, but inopportune EINTR -- that
>>> gets reported as a parse failure warning and ignoring the cgroup limit.
>>
>> Hmmm okay I was trying to keep it simple. I will check rc==0 as well, else give the warning, including reporting errno if non-zero.
>
> Okay.
>
>>> ------------------------------------------------------------------------------
>>> src/share/vm/runtime/arguments.cpp
>>> 2054         log_info(gc, heap)("Setting phys_mem to the min of cgroup limit ("
>>>
>>> log_info seems a little noisy, especially considering that we
>>> ultimately announce the final ergonomic (Min|Max)HeapSize with
>>> log_trace.
>>
>> I figure if you want to use the cgroup limit then you want to clearly see the result of doing so. I don't think hiding that info at the lowest tracing level would be helpful to the user.
>
> That’s a good rationale.  Leave the logging as is.
>
>