RFR: 8350567: NMT: update VMATree::register_mapping to copy the existing tag of the region [v2]

[Johan Sjölen]

On Thu, 27 Feb 2025 09:38:12 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> When committing a sub-region (SR) in the middle of a reserved region (RR), we need to decide on the MemTag. To find the correct tag, we had to find the RR base and take the tag and use it for SR.
>> With this PR, there will be no need to find the RR base and the tag of the previous region of SR can be copied to the SR.
>> 
>> Tests:
>> linux-x64-debug, gtest:NMT*, runtime/NMT
>
> Afshin Zafari has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - removed extra whitespace.
>  - unit test added.

It seems like the actual bug is in the summary diff accounting code and *not*  in the actual reservations, see comments. Is that correct?

src/hotspot/share/nmt/vmatree.cpp line 80:

> 78:       MemTag tag = leqA_n->val().out.mem_tag();
> 79:       stA.out.set_tag(tag);
> 80:       LEQ_A.state.out.set_tag(tag);

Shouldn't `LEQ_A.state.out`'s tag already be `tag`? Is this line really necessary?

src/hotspot/share/nmt/vmatree.cpp line 210:

> 208: 
> 209:   // Finally, we can register the new region [A, B)'s summary data.
> 210:   MemTag tag_to_change = use_tag_inplace ? stA.out.mem_tag() : metadata.mem_tag;

Do we ever change the `stA.out.mem_tag()` ? Can't it always be `stA.out.mem_tag()`?

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

PR Review: https://git.openjdk.org/jdk/pull/23771#pullrequestreview-2652806668
PR Review Comment: https://git.openjdk.org/jdk/pull/23771#discussion_r1976706349
PR Review Comment: https://git.openjdk.org/jdk/pull/23771#discussion_r1976706641