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

David Holmes david.holmes at oracle.com
Sat Dec 10 01:24:03 UTC 2016


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.

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

> ------------------------------------------------------------------------------
> src/share/vm/runtime/arguments.cpp
>
> Trivial style nit:
>
> 2058       }
> 2059       else {
>
> 2063     }
> 2064     else {
>
> should be "  } else {"
>
> ------------------------------------------------------------------------------

Ok.

I'll do the updates and try to retest over the weekend - then post 
updated webrev.

Thanks,
David



More information about the hotspot-dev mailing list