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