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

[Thomas Stuefe]

On Sun, 28 Jul 2024 19:42:54 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> Alright, I think this is OK.
>> 
>> 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.
>
>> 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.

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

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