Hi Jie,
On 16/04/2020 11:23 pm, jiefu(傅杰) wrote:
Hi Severin,
Thanks for your review and very nice suggestions.
Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.01/
test/hotspot/jtreg/containers/docker/TestGetFreeSwapSpaceSize.java is added to
reproduce the bug.
Can you please use the standard OpenJDK file header after your Tencent
copyright line:
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License
version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
I don't think the "classpath exception" is relevant to tests - certainly
other tests I checked do not have it.
Thanks,
David
-----
Thanks a lot.
Best regards,
Jie
On 2020/4/16, 4:40 PM, "Severin Gehwolf" <sgehw...@redhat.com> wrote:
Hi Jie,
On Fri, 2020-04-10 at 01:49 +0000, jiefu(傅杰) wrote:
> Hi all,
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8242480
> Webrev: http://cr.openjdk.java.net/~jiefu/8242480/webrev.00/
>
> Negative values were returned by getFreeSwapSpaceSize() in our docker
testing.
> The reason is that current implementation doesn't consider the situation
when memory.limit_in_bytes == memory.memsw.limit_in_bytes, which means do not use
the swap space at all.
>
> In
src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java,
let's see
> ------------------------------------------------
> 71 public long getFreeSwapSpaceSize() {
> 72 if (containerMetrics != null) {
> 73 long memSwapLimit =
containerMetrics.getMemoryAndSwapLimit();
> 74 long memLimit = containerMetrics.getMemoryLimit();
> 75 if (memSwapLimit >= 0 && memLimit >= 0) {
> 76 for (int attempt = 0; attempt < MAX_ATTEMPTS_NUMBER;
attempt++) {
> 77 long memSwapUsage =
containerMetrics.getMemoryAndSwapUsage();
> 78 long memUsage =
containerMetrics.getMemoryUsage();
> 79 if (memSwapUsage > 0 && memUsage > 0) {
> 80 // We read "memory usage" and "memory and swap
usage" not atomically,
> 81 // and it's possible to get the negative
value when subtracting these two.
> 82 // If this happens just retry the loop for a
few iterations.
> 83 if ((memSwapUsage - memUsage) >= 0) {
> 84 return memSwapLimit - memLimit -
(memSwapUsage - memUsage);
> 85 }
> 86 }
> 87 }
> 88 }
> 89 }
> 90 return getFreeSwapSpaceSize0();
> 91 }
> ------------------------------------------------
> If memSwapLimit (@line 73) equals memLimit (@line 74), then
getFreeSwapSpaceSize() may return a negative value @line 84.
>
> It would be better to fix it.
> Could you please review it and give me some advice?
Would this be reproducible via a test? There is
test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java which
contains testOperatingSystemMXBeanAwareness() tests.
It would be good to capture this in a test somehow.
Thanks,
Severin
