RFR: 8354115: NMT: VMATree should not accept `uncommit` a `released` region [v2]

Gerard Ziemski gziemski at openjdk.org
Thu Apr 10 15:24:37 UTC 2025


On Thu, 10 Apr 2025 12:52:52 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> At `uncommit` we change the state of the regions to `Reserved`. This is not acceptable to uncommit a  `Released` region and change its state to `Reserved`.
>> This case is detected and an invalid `SummaryDiff` (all its contents are -1) is returned. 
>> 
>> Two test-cases added and run.
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   returns invalid SummaryDiff, instead of assert.

Changes requested by gziemski (Reviewer).

Looks good, just a couple minor feedback issues.

src/hotspot/share/nmt/vmatree.hpp line 183:

> 181:       }
> 182:     }
> 183:     bool is_valid() const {

How about more descriptive name, such as `is_non_free()` or `was_allocated()`, `was_touched()` something like that to mark that this was used, `valid` is too generic?

test/hotspot/gtest/nmt/test_vmatree.cpp line 745:

> 743: 
> 744: TEST_VM_F(NMTVMATreeTest, UncommmitReleasedRegion) {
> 745:   {

I would also add this case (which is the one I used originally to find this issue):


    Tree tree;
    // 0..............................
    // 0.......40----60........
    VMATree::SummaryDiff diff = tree.uncommit_mapping(40, 20, rd2);
    EXPECT_FALSE(diff.is_valid())z

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

PR Review: https://git.openjdk.org/jdk/pull/24572#pullrequestreview-2757241041
PR Comment: https://git.openjdk.org/jdk/pull/24572#issuecomment-2794214667
PR Review Comment: https://git.openjdk.org/jdk/pull/24572#discussion_r2037671164
PR Review Comment: https://git.openjdk.org/jdk/pull/24572#discussion_r2037659755


More information about the hotspot-runtime-dev mailing list