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

[Afshin Zafari]

On Mon, 29 Jul 2024 06:17:27 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> One thing that worries me a bit is the fact that we take the `leq` value without checking if it's equal to the `A` address. This means that if we only have a range 0-100 with mtTest and I reserve with `copy_flag` at 1000-1100, then that range will receive `mtTest`. That's a bit spooky. Perhaps the `copy_flag` should only be used if there's a range starting at exactly that address? That seems less surprising to me.
>> 
>> You are right with this point, but it happens now at call to `uncommit_mapping` where the `copy_flag` is true. `reserve_mapping` always uses `false` for the `copy_flag`. Is this correct if we `assert` that the `leq` is `Committed` or `Reserved`? Is this also correct for commit and `copy_flag`?
>
> @afshin-zafari thank you for implementing this
> 
>> > One thing that worries me a bit is the fact that we take the `leq` value without checking if it's equal to the `A` address. This means that if we only have a range 0-100 with mtTest and I reserve with `copy_flag` at 1000-1100, then that range will receive `mtTest`. That's a bit spooky. Perhaps the `copy_flag` should only be used if there's a range starting at exactly that address? That seems less surprising to me.
>> 
>> You are right with this point, but it happens now at call to `uncommit_mapping` where the `copy_flag` is true. `reserve_mapping` always uses `false` for the `copy_flag`. Is this correct if we `assert` that the `leq` is `Committed` or `Reserved`? Is this also correct for commit and `copy_flag`?
> 
> We should not test the address value of A. However, we should check the outgoing state of A and *only copy the flag if outgoing node state is either `Committed` or `Reserved`*. We should require a flag from the caller if outgoing state is `Released`: it means we are reserving a new mapping inside what was before a hole in the address space with no pre-existing mapping. So there is no mapping to take over the flag from.
> 
> Some examples, lets say we have: 
> 
> `reserve(0, 100, mtNMT);`
> 
> Now we do:
> 
> A) `reserve(50, 60)` 	(inside) -> should take mtNMT
> B) `reserve(0, 20)` 	(same starting address) -> should take mtNMT
> C) `reserve(90, 100)` 	(same ending address) -> should take mtNMT
> 
> But
> 
> D) `reserve(100, 200)`   should require flag from caller: even though its adjacent to the old area, its a new area
> E) `reserve(1000, 1100)` should require flag from caller
> 
> 
> Side note, a theoretical question is what to do with partly overlapping areas: `reserve(90, 110)`
> 
> The current code would take over the flag from the original mapping between 10 and 100. And I think that is fine. We don't expect this to happen in hotspot anyway.

Thank you @tstuefe, @jdksjolen and @gerard-ziemski for your reviews.
It passed all tiers1-5, so I integrate it.

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

PR Comment: https://git.openjdk.org/jdk/pull/20330#issuecomment-2410846826