RFR: 8230305: Cgroups v2: Container awareness

Severin Gehwolf sgehwolf at redhat.com
Wed Nov 6 09:47:58 UTC 2019


Hi Bob,

On Tue, 2019-11-05 at 16:54 -0500, Bob Vandette wrote:
> Severin,
> 
> Thanks for taking on this cgroup v2 improvement.  
> 
> In general I like the implementation and the refactoring.  The CachedMetric class is nice.
> We can add any metric we want to cache in a more general way.
> 
> Is this the latest version of the webrev?
> 
> http://cr.openjdk.java.net/~sgehwolf/webrevs/cgroupsv2-hotspot/03/webrev/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp.html
> 
> It looks like you need to add the caching support for active_processor_count (JDK-8227006).

No, my latest version is v04, but it's not been properly rebased on top
of JDK-8227006, yet, as it hasn't been pushed at the time. Anyway, 04,
already has caching for active_processor_count and avoids calling
os::Linux::active_processor_count() unconditionally as in v03 at the
expense of making CgroupSubsystem a friend class of Linux:

http://cr.openjdk.java.net/~sgehwolf/webrevs/cgroupsv2-hotspot/04/webrev/

I'll do a proper rebase ASAP.

> I’m not sure it’s worth providing different strings for Unlimited versus Max or Scaled shares.
> I’d just try to be compatible with the cgroupv2 output so you don’t have to change the test.

OK. Will do.

> I wonder if it’s worth trying to synthesize memory_max_usage_in_bytes() by keeping the highest
> value ever returned by the API.

Interesting idea. I'll ponder this a bit and get back to you.

> Are you planning on adding cgroupv2 support for jdk.internal.platform.Metrics?

Yes, I am. I wanted to get feedback on the hotspot parts first, though,
so as to avoid needing to change both impls. In general the Java impls
will be a mirrors of the hotspot versions of the API.

> jdk/open/src//java.base/linux/classes/jdk/internal/platform/cgroupv2/Metrics.java
> 
> This is needed for -XshowSettings:system and will eventually be needed for 
> a new Container MXBean.

Yes, understood.

Thanks for the review!

Cheers,
Severin

> Bob.
> 
> 
> > On Oct 18, 2019, at 12:24 PM, Severin Gehwolf <sgehwolf at redhat.com> wrote:
> > 
> > On Tue, 2019-10-15 at 11:19 +0200, Severin Gehwolf wrote:
> > > Hi,
> > > 
> > > Please review this update to the container detection code which adds
> > > cgroup version 2 support. Initial review of this started in [1]. Bob
> > > preferred one big patch and didn't like the other refactoring in
> > > os_linux so this has been dropped.
> > > 
> > > This new patch includes both, the refactoring to move cgroup v1
> > > specific implementation out of osContainer_linux.{h,c}pp files[2] as
> > > well as updated detection code and the implementation for cgroups
> > > v2[3]. After this patch, osContainer_linux{h,c}pp files are cgroups
> > > version agnostic. Implementations for specific versions are in
> > > cgroupV{1,2}Subsystem.{c,h}pp files. Some shared, cgroup version
> > > agnostic code is in cgroupSubsystem.{c,h}pp.
> > > 
> > > Updated detection logic looks in /proc/cgroups first for hierarchy ids
> > > of cpu/cpuset/cpuacct/memory controllers. If the hierarchy id for all
> > > of them is 0 and they're all enabled, cgroups v2, unified hierarchy is
> > > assumed. Otherwise it uses cgroups v1 controllers (also known as hybrid
> > > or legacy hierarchy). Note that controllers can be only be mounted via
> > > one or the other hierarchy, legacy (v1) or unified (v2) - exclusive[4].
> > > 
> > > Note that for the cgroups v2 cpu_shares() implementation a reverse
> > > mapping is needed for the plain value exposed via cpu.weight.
> > > Additionally, there doesn't seem to be an API exposed to use for an
> > > implementation of memory_max_usage_in_bytes() in cgroups v2. Hence, it
> > > returns OSCONTAINER_ERROR which is mapped to "not supported" elsewhere
> > > in hotspot code.
> > > 
> > > Once reviewed, I intend to push this in two changesets/bugs. One for
> > > the refactoring work (no-op) as JDK-8230848. Another for the cgroupv2
> > > impl with JDK-8230305.
> > > 
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8230305
> > 
> > Rebased webrev on top of JDK-8232207:
> > http://cr.openjdk.java.net/~sgehwolf/webrevs/cgroupsv2-hotspot/03/webrev/
> > 
> > Thanks,
> > Severin
> > 
> > > Testing: tier1 tests on Linux x86_64. Docker/podman hotspot tests on
> > > F30 with hybrid cgroup v1 hierarchy. Hotspot container tests on F31
> > > with unified hierarchy on Linux x86_64. jdk/submit. All pass.
> > > 
> > > Thoughts?
> > > 
> > > Thanks,
> > > Severin
> > > 
> > > [1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2019-September/039605.html
> > >    http://mail.openjdk.java.net/pipermail/hotspot-dev/2019-October/039708.html
> > > [2] http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8230848/04/webrev/
> > > [3] http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8230305/06/webrev/
> > > [4] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#mounting
> > >    "All controllers which support v2 and are not bound to a v1 hierarchy are
> > >    automatically bound to the v2 hierarchy and show up at the root.
> > >    Controllers which are not in active use in the v2 hierarchy can be
> > >    bound to other hierarchies."



More information about the hotspot-dev mailing list