RFR: 8328944: NMT reports "unknown" memory [v2]

[Gerard Ziemski]

On Mon, 18 Nov 2024 10:42:59 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Why should we supplement `mtNone` with `mtAllocated`? Is one indicative of a failure mode?

That is the general idea, if we see `mtNone` in a report we know we missed something. `mtAllocated` would be the one indicating that we are "in transition".

But I put it back into draft mode to experiment and get a better picture on how to proceed.

> src/hotspot/share/nmt/virtualMemoryTracker.hpp line 389:
> 
>> 387:   static bool remove_released_region    (address base_addr, size_t size);
>> 388:   static bool remove_released_region    (ReservedMemoryRegion* rgn);
>> 389:   static void set_reserved_region_type  (address addr, MemTag mem_tag);
> 
> I don't think that we should remove this API. It may very well need to be used, even if its usage should be minimized.

If there is currently no-one using it, then we should remove it. We can add it back, once we need it.

> src/hotspot/share/runtime/os.hpp line 460:
> 
>> 458:   // Attempts to reserve the virtual memory at [addr, addr + bytes).
>> 459:   // Does not overwrite existing mappings.
>> 460:   static char*  attempt_reserve_memory_at(char* addr, size_t bytes, bool executable, MemTag mem_tag);
> 
> Is it worth adding an overload: `reserve_memory(bytes, mem_tag)` where `executable` is implicitly `false`?

I would want that myself too, but that requires that we put it at the end, which I did in fact had at one point, but didn't like the parameters call order - the mem_tag is usually the very last one.

> test/hotspot/gtest/memory/test_virtualspace.cpp line 374:
> 
>> 372:                      alignment,       // alignment
>> 373:                      page_size,       // page size
>> 374:                      (char *)nullptr, // requested_address
> 
> Style fix: Remove space between `char` and `*`

Fixed.

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

PR Comment: https://git.openjdk.org/jdk/pull/21843#issuecomment-2483404110
PR Review Comment: https://git.openjdk.org/jdk/pull/21843#discussion_r1847064817
PR Review Comment: https://git.openjdk.org/jdk/pull/21843#discussion_r1846811525
PR Review Comment: https://git.openjdk.org/jdk/pull/21843#discussion_r1847065123