RFR: 8341491: Reserve and commit memory operations should be protected by NMT lock
Stefan Karlsson
stefank at openjdk.org
Tue Apr 1 15:32:17 UTC 2025
On Mon, 31 Mar 2025 14:09:22 GMT, Robert Toyonaga <duke at openjdk.org> wrote:
> OK should I update this PR to do the following things:
>
> * Add comments explaining the asymmetrical locking and warning against patterns that lead to races
Sounds like a good idea.
>
> * swapping the order of `NmtVirtualMemoryLocker` and release/uncommit
I wonder if this should be done as new RFE after the change below. It might need a bit of investigation to make sure that the reasoning around this is correct.
>
> * Fail fatally if release/uncommit does not complete.
I think this would be a good, separate RFE to be done before we try to swap the order.
>
>
> Or does it make more sense to do that in a different issue/PR?
>
> Also, do we want to keep the new tests and the refactorings (see below)?
>
> ```
> if (MemTracker::enabled()) {
> MemTracker::NmtVirtualMemoryLocker nvml;
> result = pd_some_operation(addr, bytes);
> if (result != nullptr) {
> MemTracker::record_some_operation(addr, bytes);
> }
> } else {
> result = pd_unmap_memory(addr, bytes);
> }
> ```
>
> To:
>
> ```
> MemTracker::NmtVirtualMemoryLocker nvml;
> result = pd_unmap_memory(addr, bytes);
> MemTracker::record_some_operation(addr, bytes);
> ```
My thinking is that after you done (2) above, then you will not need to expose the NMT lock to this level. The code would be:
MemTracker::record_some_operation(addr, bytes); // Lock confined inside this
pd_unmap_memory(addr, bytes);
So, I would wait with this cleanup until we know more about (2).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/24084#issuecomment-2769766908
More information about the serviceability-dev
mailing list