On Fri, 13 Mar 2026 14:46:14 GMT, Robert Toyonaga <[email protected]> wrote:
>> ## Summary >> Now that we [fail fatally if `os::` memory functions encounter >> errors](https://bugs.openjdk.org/browse/JDK-8353564), it's possible to >> tighten NMT locking scope by swapping the order of `pd_release_memory` and >> `record_virtual_memory_release`. >> See original discussion: >> https://github.com/openjdk/jdk/pull/24084#issuecomment-2752240832 >> >> ### The new ordering in os::release_memory: >> >> **Thread A:** >> _lock_ >> (A1) update NMT >> _unlock_ >> (A2) pd_release >> >> **Thread B:** >> (B1) pd_reserve >> _lock_ >> (B2) update NMT >> _unlock_ >> >> The race having ordering A1, B1, B2, A2 is not possible. As long as >> Thread_A has not yet released the reservation, the OS prevents Thread_B from >> racing to re-reserve the same range. Meaning Thread_A does not need to call >> pd_release under the NMT lock protection. >> >> >> ## Further work >> It's possible that the same lock scope tightening can be done for >> `os::unmap_memory` and `os::uncommit_memory`, but there would be no hard >> guarantee provided by the OS against races. We would need to rely on >> external synchronization in the caller to prevent concurrent modification of >> regions. In practice, it doesn't seem like any existing Hotspot code can >> lead to the races described **_below_**, but I'd like to collect more >> opinions here before changing `os::unmap_memory` and `os::uncommit_memory`. >> >> ### Possible uncommit Race >> **Thread_A:** >> _lock_ >> (A1) update NMT with uncommit >> _unlock_ >> (A2) pd_uncommit >> >> **Thread_B:** >> (B1) pd_commit >> _lock_ >> (B2) update NMT with commit >> _unlock_ >> >> Problematic ordering: A1, B1, B2, A2 >> (The region is uncommitted but NMT thinks it's committed. The OS doesn't >> stop us from doing B1 before A2) >> >> ### Possible unmap race >> **Thread_A:** >> _lock_ >> (A1) update NMT with release >> _unlock_ >> (A2) pd_unmap_memory >> >> **Thread_B:** >> (B1) pd_map_memory >> _lock_ >> (B2) update NMT with reserve + commit >> _unlock_ >> >> Problematic ordering: A1, B1, B2, A2 >> (The OS allows remapping over an existing mapping using MAP_FIXED, so B1 can >> happen before A2. This results in Thread_B's mapping being destroyed.) > > Robert Toyonaga has updated the pull request incrementally with one > additional commit since the last revision: > > update copyright dates > ### > > **Thread A:** _lock_ (A1) update NMT _unlock_ (A2) pd_release > > **Thread B:** (B1) pd_reserve _lock_ (B2) update NMT _unlock_ > > The race having ordering A1, B1, B2, A2 is not possible. As long as Thread_A > has not yet released the reservation, the OS prevents Thread_B from racing to > re-reserve the same range. Meaning Thread_A does not need to call pd_release > under the NMT lock protection. Interesting. Yes, I see it. If T1 releases the NMT range first, then T2 re-allocates, we are living in a temporary state where NMT thinks a range is free, but it's actually not (yet) free. But that is benign, and actually can happen all the time, since NMT does not track all process mappings. Good. This is a good simplification. Thanks! ------------- PR Comment: https://git.openjdk.org/jdk/pull/29962#issuecomment-4056077440
