[16] RFR(T) 8248426: NMT: VirtualMemoryTracker::split_reserved_region() does not properly update summary counting

[Thomas Stüfe]

Hi Zhengyu,

okay, I get it now. Thank you. Reviewed from my side.

Cheers, Thomas

On Tue, Jul 7, 2020 at 1:53 PM Zhengyu Gu <zgu at redhat.com> wrote:

> Hi Thomas,
>
> On 7/4/20 1:35 AM, Thomas Stüfe wrote:
> > Hi Zhengyu,
> >
> > sorry for the wait.
>
> No problem.
>
> >
> >
> > This looks good, but why did you remove the "!same region" condition? I
> > believe that is needed for the case when CDS' first mapping encounters
> > errors, so before it rebuilds CDS at another location it removes (a) all
> > mappings and then (b) the enclosing reservation. (a) should be ignored
> > by NMT but (b) should not. I may be wrong, I have had no coffee yet.
> > Cheers, Thomas
>
> In new version, same region is handled in line #470
>
>   470   if (reserved_rgn->same_region(addr, size)) {
>   471     return remove_released_region(reserved_rgn);
>   472   }
>
> so, !same region is always true.
>
> Thanks,
>
> -Zhengyu
>
> >
> >     Thanks,
> >
> >     -Zhengyu
> >
> >      >
> >      > But if you are still unconvinced, I won't hold you up. The change
> >      > certainly works as it is now and is okay for me, it just would
> >     not be my
> >      > preferred solution.
> >      >
> >      > Cheers, Thomas
> >      >
> >      >     Thanks,
> >      >
> >      >     -Zhengyu
> >      >
> >      >
> >      >      >
> >      >      >     Thanks, Thomas
> >      >      >
> >      >      >
> >      >      >
> >      >      >
> >      >      >
> >      >      >     On Fri, Jun 26, 2020 at 10:54 PM Zhengyu Gu
> >     <zgu at redhat.com <mailto:zgu at redhat.com>
> >      >     <mailto:zgu at redhat.com <mailto:zgu at redhat.com>>
> >      >      >     <mailto:zgu at redhat.com <mailto:zgu at redhat.com>
> >     <mailto:zgu at redhat.com <mailto:zgu at redhat.com>>>> wrote:
> >      >      >
> >      >      >         Hi,
> >      >      >
> >      >      >         Please review this trivial patch that fixes summary
> >      >     counting in
> >      >      >         VirtualMemoryTracker::split_reserved_region().
> >      >      >
> >      >      >         The method uses internal method to remove a
> >     reserved region,
> >      >      >         which does
> >      >      >         not update counting information. It should use
> >     high level
> >      >     tracking
> >      >      >         method
> VirtualMemoryTracker::remove_released_region()
> >      >     instead.
> >      >      >
> >      >      >         Without patch, NMT summary reports uncategorized
> >     memory, e.g.
> >      >      >
> >      >      >         -                   Unknown (reserved=1060416KB,
> >      >     committed=0KB)
> >      >      >                                       (mmap:
> >     reserved=1060416KB,
> >      >      >         committed=0KB)
> >      >      >
> >      >      >
> >      >      >
> >      >      >
> >      >      >         Bug:
> https://bugs.openjdk.java.net/browse/JDK-8248426
> >      >      >         Webrev:
> >      > http://cr.openjdk.java.net/~zgu/JDK-8248426/webrev.00/
> >      >      >
> >      >      >         Test:
> >      >      >             hotspot_nmt
> >      >      >             Submit test in progress
> >      >      >
> >      >      >
> >      >      >         Thanks,
> >      >      >
> >      >      >         -Zhengyu
> >      >      >
> >      >
> >
>
>