[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8219414: SA: jhsdb jsnap throws UnmappedAddressException with core generated by gcore
From:       Yasumasa Suenaga <yasuenag () gmail ! com>
Date:       2019-02-21 12:01:28
Message-ID: 50facbe2-262b-b20d-8de6-24ec2d629b06 () gmail ! com
[Download RAW message or body]

Thanks Jini!


Yasumasa


On 2019/02/21 19:19, Jini George wrote:
> Looks good to me, Yasumasa. Thanks!
> 
> -Jini.
> 
> On 2/21/2019 11:45 AM, Yasumasa Suenaga wrote:
>> Hi Jini,
>>
>> Thank you for your comment.
>>
>> I uploaded new webrev.
>> Could you review again?
>>
>>      http://cr.openjdk.java.net/~ysuenaga/JDK-8219414/webrev.01/
>>
>> diff from webrev.00 is here:
>>      http://hg.openjdk.java.net/jdk/submit/rev/bb58c9d41381
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> 2019年2月21日(木) 12:30 Jini George <jini.george@oracle.com>:
>>>
>>> Your changes look good to me overall, Yasumasa. A couple of points though.
>>>
>>> * Since you are making modifications to have DumpPrivateMappingsInCore
>>> independent of CDS, in os::abort(), you would also need change the
>>> following lines:
>>> 1361 #if INCLUDE_CDS
>>> 1362         if (UseSharedSpaces && DumpPrivateMappingsInCore) {
>>> 1363             ClassLoader::close_jrt_image();
>>> 1364         }
>>> 1365 #endif
>>>
>>> to
>>>
>>> 1361
>>> 1362         if (DumpPrivateMappingsInCore) {
>>> 1363             ClassLoader::close_jrt_image();
>>> 1364         }
>>>
>>> But this might cause the zero build to fail due to the call to
>>> ClassLoader::close_jrt_image()
>>> (https://bugs.openjdk.java.net/browse/JDK-8215342). So, it might be
>>> better to guard the above lines with #ifndef ZERO to have:
>>>
>>> 1361 #ifndef ZERO
>>> 1362         if (DumpPrivateMappingsInCore) {
>>> 1363             ClassLoader::close_jrt_image();
>>> 1364         }
>>> 1365 #endif
>>>
>>>
>>> * Also, one nit:
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8219414/webrev.00/src/hotspot/os/linux/os_linux.cpp.html
>>>
>>> - Could you please remove or modify the comment at line 3436 since
>>> set_coredump_filter() is not restricted to include only largepages now?
>>>
>>> Thank you!
>>> Jini.
>>>
>>>
>>> On 2/21/2019 7:43 AM, Yasumasa Suenaga wrote:
>>>> Thanks Chris!
>>>>
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> 2019年2月21日(木) 11:08 Chris Plummer <chris.plummer@oracle.com
>>>> <mailto:chris.plummer@oracle.com>>:
>>>>
>>>>          Ok. The changes look fine to me then.
>>>>
>>>>          thanks,
>>>>
>>>>          Chris
>>>>
>>>>          On 2/20/19 6:02 PM, Yasumasa Suenaga wrote:
>>>>            > Hi Chris,
>>>>            >
>>>>            > I grep'ed with "MAP_SHARED" on jdk/src.
>>>>            > Shared memory is used in ZGC (ZBackingFile and
>>>>          ZPhysicalMemoryBacking)
>>>>            > and in FileChannel::map at least.
>>>>            >
>>>>            > I think we have memory footprint concern about them, but shared
>>>>          memory
>>>>            > should be dumped.
>>>>            > If we did not get them, we cannot analyze ZGC related behavior and
>>>>            > other code which uses shared memory.
>>>>            > (We can pass FD to os::reserve_memory - it will use shared memory if
>>>>            > FD is passed.)
>>>>            >
>>>>            > Thus I want to introduce `DumpSharedMappingsInCore` for dumping
>>>>          shared
>>>>            > memory and set it to true by default.
>>>>            >
>>>>            >
>>>>            > Thanks,
>>>>            >
>>>>            > Yasumasa
>>>>            >
>>>>            >
>>>>            > 2019年2月21日(木) 3:10 Chris Plummer <chris.plummer@oracle.com
>>>>          <mailto:chris.plummer@oracle.com>>:
>>>>            >> [adding runtime]
>>>>            >>
>>>>            >> Hi Yasumasa,
>>>>            >>
>>>>            >> Overall looks good. Just a couple of questions.
>>>>            >>
>>>>            >> Do we have the same footprint concerns with the shared mappings
>>>>          as we
>>>>            >> did with the private mappings? If not, possibly it doesn't need an
>>>>            >> option and should always be enabled.
>>>>            >>
>>>>            >> thanks,
>>>>            >>
>>>>            >> Chris
>>>>            >>
>>>>            >> On 2/20/19 12:03 AM, Yasumasa Suenaga wrote:
>>>>            >>> Hi all,
>>>>            >>>
>>>>            >>> Please review this webrev:
>>>>            >>>
>>>>            >>>         JBS: https://bugs.openjdk.java.net/browse/JDK-8219414
>>>>            >>>         webrev:
>>>>          http://cr.openjdk.java.net/~ysuenaga/JDK-8219414/webrev.00/
>>>>            >>>
>>>>            >>> I tried to get PerfCounter values via `jhsdb jsnap` from core image
>>>>            >>> which is generated by `gcore` (provided by GDB), but I encountered
>>>>            >>> UnmappedAddressException.
>>>>            >>>
>>>>            >>> It is caused by `generate-core-file` on GDB regards
>>>>          `coredump_filter` on procfs.
>>>>            >>>
>>>>            >>> https://sourceware.org/gdb/onlinedocs/gdb/Core-File-Generation.html
>>>>            >>>
>>>>            >>> JDK-8200613 introduced `DumpPrivateMappingsInCore` for CDS. I
>>>>          want to
>>>>            >>> introduce `DumpSharedMappingsInCore` for shared memory mapping.
>>>>            >>>
>>>>            >>> Currently `DumpPrivateMappingsInCore` affects when
>>>>          `UseSharedSpaces` is enabled.
>>>>            >>> I want `DumpPrivateMappingsInCore` to affect independently in this
>>>>            >>> change because file-backed private memory which is not CDS might be
>>>>            >>> useful in the future.
>>>>            >>>
>>>>            >>>
>>>>            >>> Thanks,
>>>>            >>>
>>>>            >>> Yasumasa
>>>>            >>
>>>>
>>>>
[prev in list] [next in list] [prev in thread] [next in thread]