RFR: 8335091: NMT: VMATree reserve_mapping and commit_mapping APIs need MEMFLAGS while un/-committing API has no MEMFLAGS arg [v3]

[Thomas Stuefe]

On Thu, 25 Jul 2024 14:51:50 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> In committing a region, it is not mandatory to provide a MEMFLAGS flag where the committed region inherits the flag from the main region it resides in.
>> In un-committing there is no need to a  MEMFLAGS at all. 
>> The `register_mapping` API of the VMATree *requires* a MEMFLAGS (via metadata arg) in both of these two operations. To do the flag inheriting, it is possible to copy the flag of the left node in the tree to the newly inserted ones. 
>> 
>> An optional bool arg (default is false) is added to VMATree API to copy the existing flag of the left node to the new nodes.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   reserve always use the flag in metadata

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

> 35: 
> 36: VMATree::SummaryDiff VMATree::register_mapping(position A, position B, StateType state,
> 37:                                                const RegionData& metadata, bool copy_flag) {

We need to think about what the behavior should be if the caller passes `copy_flag=true` but vmatree disagrees and thinks the caller should specify a flag. At the moment we quietly take over the flag from the `metadata` argument, but I think we should assert. Something like "reserving without pre-existing range requires a valid NMT flag".

Tied into that is the question of what the flag in the `metadata` argument should be for `copy_flag=true`. I would argue that the flag must be `mtNone` - any other flag would contradict the "copy_flag" argument.

In fact, we could argue that `mtNone` means `copy_flag==true` and then we don't need the new `copy_flag` argument. In that case, reserving with `mtNone` means "no clue about the flag, you figure it out" and then it requires a pre-existing mapping. Are there valid use cases for reserving ranges with mtNone as a valid NMT flag?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20330#discussion_r1694639430