Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
Thanks Severin and David for your review.
Will push it tomorrow.
Best regards,
Jie
On 2020/4/17, 8:56 PM, "David Holmes" wrote:
On 17/04/2020 5:00 pm, jiefu(傅杰) wrote:
> Hi David,
>
> Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.02/
>
> The file header had been fixed. Please review it.
File header update looks good.
Thanks,
David
> Thanks a lot.
> Best regards,
> Jie
>
> On 2020/4/17, 11:59 AM, "David Holmes" wrote:
>
> 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"
wrote:
> >
> > Hi Jie,
> >
> > On Fri, 2020-04-10 at 01:49 +, 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 }
> > > 8
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
On 17/04/2020 5:00 pm, jiefu(傅杰) wrote:
Hi David,
Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.02/
The file header had been fixed. Please review it.
File header update looks good.
Thanks,
David
Thanks a lot.
Best regards,
Jie
On 2020/4/17, 11:59 AM, "David Holmes" wrote:
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" wrote:
>
> Hi Jie,
>
> On Fri, 2020-04-10 at 01:49 +, 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.
>
> I
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
On Fri, 2020-04-17 at 06:58 +, jiefu(傅杰) wrote: > Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.02/ Looks good. Thanks, Severin
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
Hi David,
Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.02/
The file header had been fixed. Please review it.
Thanks a lot.
Best regards,
Jie
On 2020/4/17, 11:59 AM, "David Holmes" wrote:
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" wrote:
>
> Hi Jie,
>
> On Fri, 2020-04-10 at 01:49 +, 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
>
>
>
>
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
Hi Severin, Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.02/ Please review it. Thanks a lot. Best regards, Jie On 2020/4/16, 11:40 PM, "Severin Gehwolf" wrote: Since you've added a new test, please move them to the jdk docker tests in: test/jdk/jdk/internal/platform/docker/ Fixed. test/hotspot/jtreg/containers/docker/TestGetFreeSwapSpaceSize.java + * @build sun.hotspot.WhiteBox GetFreeSwapSpaceSize + * @run driver ClassFileInstaller -jar whitebox.jar sun.hotspot.WhiteBox sun.hotspot.WhiteBox$WhiteBoxPermission I don't see any reason why WhiteBox would be needed for this test. Is that an oversight or am I missing something? It's my oversight. Thanks for correcting me.
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
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" wrote:
Hi Jie,
On Fri, 2020-04-10 at 01:49 +, 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
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
Hi Jie,
On Thu, 2020-04-16 at 13:23 +, jiefu(傅杰) wrote:
> Hi Severin,
>
> Thanks for your review and very nice suggestions.
Thanks for adding a test!
> Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.01/
>
> test/hotspot/jtreg/containers/docker/TestGetFreeSwapSpaceSize.java is
> added to reproduce the bug.
Since you've added a new test, please move them to the jdk docker tests
in:
test/jdk/jdk/internal/platform/docker/
This is because the OperatingSystemMXBean uses the Metrics.java
internal API (not hotspot cgroup support) and the tests really belong
there alongside other Metrics docker tests.
test/hotspot/jtreg/containers/docker/TestGetFreeSwapSpaceSize.java
+ * @build sun.hotspot.WhiteBox GetFreeSwapSpaceSize
+ * @run driver ClassFileInstaller -jar whitebox.jar sun.hotspot.WhiteBox
sun.hotspot.WhiteBox$WhiteBoxPermission
I don't see any reason why WhiteBox would be needed for this test. Is
that an oversight or am I missing something?
Thanks,
Severin
> Thanks a lot.
> Best regards,
> Jie
>
>
> On 2020/4/16, 4:40 PM, "Severin Gehwolf"
> wrote:
>
> Hi Jie,
>
> On Fri, 2020-04-10 at 01:49 +, 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/Operating
> SystemImpl.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
>
>
>
>
Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)
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.
Thanks a lot.
Best regards,
Jie
On 2020/4/16, 4:40 PM, "Severin Gehwolf" wrote:
Hi Jie,
On Fri, 2020-04-10 at 01:49 +, 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
