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

[Kim Barrett]

> 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.