Thanks Severin and David for your review.
Will push it tomorrow.
Best regards,
Jie
On 2020/4/17, 8:56 PM, "David Holmes" <david.hol...@oracle.com> 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" <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
> >
> >
> >
> >
>
>
>
