Re: RFR: 8341491: Reserve and commit memory operations should be protected by NMT lock

[Stefan Karlsson]

On Mon, 31 Mar 2025 14:09:22 GMT, Robert Toyonaga <d...@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