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" <david.hol...@oracle.com> 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" <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
>
>
>
>
