8246648: issue with OperatingSystemImpl getFreeSwapSpaceSize in docker after 8242480

[Severin Gehwolf]

On Fri, 2020-06-05 at 08:14 +0000, Baesken, Matthias wrote:
> Bug/webrev :
> 
> https://bugs.openjdk.java.net/browse/JDK-8246648
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8246648.0/

Prior this patch OperatingSystemMXBean.getFreeSwapSpaceSize() would
return 0 (-1 - memLimit < 0), on a system with a kernel which doesn't
support swap limits since -1 will be returned for
Metrics.getMemoryAndSwapLimit(). After this patch it would return the
system value.

Similarly, for a system with explicit --memory-swap=-1 (unlimited) it
would return 0 prior patch, and the system value post-patch.

In both cases, pre/post-patch not all combinations are correct. 

However, it seems reasonable to account for the -1 retval of
memSwapLimit explicitly and be well-behaved for the explicit unlimited
swap value configuration and only be ill-behaved for a kernel not
supporting swap limits (rare?). In which case, a warning is being
printed when running the container with a swap limit.

In summary: Patch looks OK to me.

Aside:

  55     public long getTotalSwapSpaceSize() {
  56         if (containerMetrics != null) {
  57             long limit = containerMetrics.getMemoryAndSwapLimit();
  58             // The memory limit metrics is not available if JVM runs on Linux host (not in a docker container)
  59             // or if a docker container was started without specifying a memory limit (without '--memory='
  60             // Docker option). In latter case there is no limit on how much memory the container can use and
  61             // it can use as much memory as the host's OS allows.
  62             long memLimit = containerMetrics.getMemoryLimit();
  63             if (limit >= 0 && memLimit >= 0) {
  64                 // we see a limit == 0 on some machines where "kernel does not support swap limit capabilities"

This comment --^ is wrong. Should be "we see a limit == -1 ..."

  65                 return (limit < memLimit) ? 0 : limit - memLimit;
  66             }

Not sure if you want to change that as part of this patch.

Thanks,
Severin