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

[Zhengyu Gu]

Thanks, Thomas.

-Zhengyu

On 7/7/20 9:34 AM, Thomas Stüfe wrote:
> 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 
> <mailto: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>>>
>      >      >      >     <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 <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
>      >      >      >
>      >      >
>      >
>