On Fri, 13 Mar 2026 15:49:59 GMT, Thomas Stuefe <[email protected]> wrote:
>> Robert Toyonaga has updated the pull request incrementally with one >> additional commit since the last revision: >> >> update copyright dates > > src/hotspot/share/runtime/os.cpp line 1965: > >> 1963: // There is no lock protection keeping pd_reserve_memory and >> record_virtual_memory_reserve atomic. >> 1964: // We assume that there is some external synchronization that prevents >> a region from being released >> 1965: // before it is finished being reserved in this function. > > How would a concurrent thread even get access to the pointer to be able to > release it? Not sure if this is worth commenting, unless I misunderstand you. It would only happen in an unusual scenario where a wish address is known ahead of time by both threads. After reconsidering it, I think I agree with you. This comment is unnecessarily verbose and may lead to confusion. > src/hotspot/share/runtime/os.cpp line 2401: > >> 2399: // about-to-be-unmapped region will be overwritten. That race is >> technically possible, but we assume >> 2400: // there is external synchronization to prevent a caller from >> re-mapping this specific address at least >> 2401: // until this function returns. > > IIUC, you are worrying about someone relying on the synchronisation provided > before via the NMT locker? But that would only be active if NMT were active. > Otherwise, remapping memory that is about to be unmapped would fail today, > too. > > Hence I am not sure this is even worth commenting. It sounds a bit like if > "you hit yourself with a hammer, it will hurt, so we assume something is > preventing you from doing so". hmm good point. Yes, today, even with NMT enabled, nothing prevents a caller from remapping and overwritting an existing mapping. The NMT locker does nothing at all to prevent that. So even if there is a valid reason for remapping a range, there needs to be external synchronization for the map/unmap calls regardless. This comment is unnecessary and I will remove it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29962#discussion_r2933634292 PR Review Comment: https://git.openjdk.org/jdk/pull/29962#discussion_r2933631688
