[8u] RFR: 8217766: Container Support doesn't work for some Join Controllers combinations

[Severin Gehwolf]

On Mon, 2020-11-30 at 05:13 +0000, Andrew Hughes wrote:
> On 15:32 Tue 27 Oct     , Severin Gehwolf wrote:
> > Hi!
> > 
> > Could I please get a review for this 8u backport of 8217766. I see
> > Oracle backported it too to 8u281. The JDK 11 patch does not apply
> > cleanly, because a) no unified logging in hotspot in 8u b) split of jdk
> > and hotspot repositories in 8u.
> > 
> > I've manually backported hotspot changes. The JDK changes actually
> > applied cleanly. While this fixes part of the problem. JDK-8254854 will
> > be fixed separately and as a follow-up.
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8217766
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217766/01/
> > 
> > Testing: Manually on affected system with join controllers[1].
> > Container tests on Linux x86_64: no regressions.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin
> > 
> > [1] https://bugs.openjdk.java.net/browse/JDK-8217766?focusedCommentId=14373516&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14373516
> > 
> 
> A blank line (before the mountinfo comment) gets lost in the backport
> from 11u to 8u, causing the diffs of osContainer_linux.cpp to not line
> up. You can see this after applying your patch and comparing the two
> files:
> 
> @@ -262,13 +324,14 @@
>      char tmpcgroups[MAXPATHLEN+1];
>      char *cptr = tmpcgroups;
>      char *token;
> +
>      // mountinfo format is documented at https://www.kernel.org/doc/Documentation/filesystems/proc.txt
>      if (sscanf(p, "%*d %*d %*d:%*d %s %s %*[^-]- cgroup %*s %s", tmproot, tmpmount, tmpcgroups) != 3) {
>        continue;
>      }
>      while ((token = strsep(&cptr, ",")) != NULL) {
>        if (strcmp(token, "memory") == 0) {
> -        memory = new CgroupSubsystem(tmproot, tmpmount);
> +        memory = new CgroupMemorySubsystem(tmproot, tmpmount);
>        } else if (strcmp(token, "cpuset") == 0) {
>          cpuset = new CgroupSubsystem(tmproot, tmpmount);
>        } else if (strcmp(token, "cpu") == 0) {
> 
> Not only does this make it hard to compare your backport with the
> original to see what changed, it will cause unnecessary context
> differences for future backports. Please rectify this.

Thanks for the review!

Done:
https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8217766/jdk8/hotspot/webrev/

Thanks,
Severin