RFR: 8255973: Add more logging to debug JDK-8255917 [v3]

[Thomas Stuefe]

On Thu, 12 Nov 2020 05:15:09 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Hi, Please review
>> 
>>   JDK-8255917 failed on not finding the committed region from the _reserved_regions list when try to add the region to the committed list. It looks like possible the region was released without logged. 
>>   Added a log tag for tracing Native Memory Tracking (NMT) add/remove recorded info. This could help us to identify the problem for NMT or the code behind NMT. Function pd_attempt_reserve_memory_at could fail, but no message logged, so added a logging for its failure. 
>> 
>> Tests: tier1-4, local jtreg.
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make nmt log at debug level, and attempt_reserve_memory_at failed log to os+metaspace debug level

Hi Yumin,

thinking this through more closely, and looking at your work with JDK-8255917, I am not sure that you really need logging inside NMT. NMT just shows an error we should have detected way earlier when reserving. Knowing when reserve or commit failed would be very helpful.

Currently I think Iois theory about the split_memory() messing up is the most probable. Even though I am not sure how that could have happened. But could you please also add logging to os::split_memory() on windows? Especially I'd like to know if the two re-reservation calls failed.

To stop blocking this, I relent and leave all decisions about tags or log level to you. Lets get this logging out of the door, once we found the problem we can brush up the logging in a separate step.

src/hotspot/share/runtime/os.cpp line 1676:

> 1674:       lt.print("Attempt to reserve memory at " INTPTR_FORMAT " for "
> 1675:                          SIZE_FORMAT " bytes failed\n", p2i(addr), bytes);
> 1676:     }

Can you please add GetLastError? (returns a 32bit unsigned, so use %u and maybe cast to unsigned explicitly: (.. failed (%u)", ..., (unsigned) ::GetLastError())

Is there any reason not to just use log_info(os)("..") instead of LogTarget?

-------------

PR: https://git.openjdk.java.net/jdk/pull/1150