RFR: 8344883: Force clients to explicitly pass mem_tag value, even if it is mtNone [v8]

[Stefan Karlsson]

On Wed, 2 Apr 2025 18:37:36 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> This is a follow-up to #21843. Here we are focusing on removing the mem tag paremeter with default value of mtNone, to force everyone to provide mem tag, if known.
>> 
>> I tried to fill in tag, when I was pretty certain that I had the right type.
>> 
>> At least one more follow-up will be needed after this, to change the remaining mtNone to valid values.
>
> Gerard Ziemski has updated the pull request incrementally with one additional commit since the last revision:
> 
>   The real feedback from StefanK

Looks mostly good, there are a few things that I'd like to get cleaned up, but we can take care of that in separate RFEs.

src/hotspot/share/runtime/os.hpp line 527:

> 525: 
> 526:   static char*  map_memory(int fd, const char* file_name, size_t file_offset,
> 527:                            char *addr, size_t bytes, MemTag mem_tag, bool read_only = false,

AFAICT, there's no need to have a default value for read_only. I think we should remove this default value and move the MemTag parameter so that it comes after read_only and before allow_exec. This would make the parameter order more consistent with the other functions that accept a mem_tag and an executable.

Given that you have tested the current patch, I'm fine with doing this as a small follow-up patch.

test/hotspot/gtest/gc/z/test_zForwarding.cpp line 58:

> 56: 
> 57:     for (uintptr_t start = 0; start + ZGranuleSize <= ZAddressOffsetMax; start += increment) {
> 58:       char* const reserved = os::attempt_reserve_memory_at((char*)ZAddressHeapBase + start, ZGranuleSize, mtNone);

Suggestion:

      char* const reserved = os::attempt_reserve_memory_at((char*)ZAddressHeapBase + start, ZGranuleSize, mtTest);


This snuck in with one of your latest changes.

test/hotspot/gtest/runtime/test_os.cpp line 733:

> 731:   // Reserve a small range and fill it with a marker string, should show up
> 732:   // on implementations displaying range snippets
> 733:   char* p = os::reserve_memory(1 * M, mtInternal);

Suggestion:

  char* p = os::reserve_memory(1 * M, mtTest);

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24282#pullrequestreview-2739478381
PR Review Comment: https://git.openjdk.org/jdk/pull/24282#discussion_r2026779415
PR Review Comment: https://git.openjdk.org/jdk/pull/24282#discussion_r2026781507
PR Review Comment: https://git.openjdk.org/jdk/pull/24282#discussion_r2026782706