RFR: 8263476: NMT: Stack guard pages should not be marked as committed [v2]
Johan Sjölen
jsjolen at openjdk.org
Mon Aug 4 13:55:59 UTC 2025
On Mon, 4 Aug 2025 13:19:14 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> Use reserved (instead of committed) memory for stack-guard-pages on linux like systems.
>>
>> `os::must_commit_stack_guard_pages` uses `commit` in its name, but `commit` usually has specific meanings in OS memory context. The actual question the caller is asking is whether the caller needs to do some preparation work before marking the guard-pages as inaccessible. To avoid confusion, I changed it to "allocate". Other suggestions are welcome.
>>
>> Test: tier1-3
>
> Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>
> - review
> - Merge branch 'master' into no-commit-stack-guard-page
> - no-commit-stack-guard-page
I think that the gist of the patch is fine, and I think it's an improvement. Looks essentially like what I wanted to do, but a bit better?
I have a couple of questions, though, as I don't understand why certain things were done (see comments)
src/hotspot/os/bsd/os_bsd.cpp line 1686:
> 1684: // munmap()ping them. If not, just call uncommit_memory().
> 1685: bool os::remove_stack_guard_pages(char* addr, size_t size) {
> 1686: ShouldNotReachHere();
The comment no longer makes any sense.
src/hotspot/os/bsd/os_bsd.inline.hpp line 44:
> 42: assert(uses_stack_guard_pages(), "sanity check");
> 43: return false;
> 44: }
I'm missing why it's OK to remove this.
-------------
PR Review: https://git.openjdk.org/jdk/pull/26571#pullrequestreview-3084305117
PR Review Comment: https://git.openjdk.org/jdk/pull/26571#discussion_r2251559222
PR Review Comment: https://git.openjdk.org/jdk/pull/26571#discussion_r2251569227
More information about the hotspot-runtime-dev
mailing list