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