RFR: 8234930: Use MAP_JIT when allocating pages for code cache on macOS [v8]
Anton Kozlov
akozlov at openjdk.java.net
Tue Dec 15 15:36:57 UTC 2020
On Sat, 12 Dec 2020 07:52:45 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> src/hotspot/os/bsd/os_bsd.cpp line 1690:
>>
>>> 1688: if (::mprotect(addr, size, prot) == 0) {
>>> 1689: return true;
>>> 1690: }
>>
>> You need to handle mprotect failure here too. Probably just by returning false. There is no point in doing the mmap below as fallback. The same applies for the OpenBSD path too.
>>
>> mprotect may, at least on Linux, fail if the new mapping introduced by changing the protection would bring the process above the system limit for numbrer of mappings. I strongly believe there must be a similar error scenario on Mac. At least on BSD there is (https://man.openbsd.org/mprotect.2), see ENOMEM.
>
> Also, this is asymetric to uncommit now for the !exec case. There, we mmap(MAP_NORESERVE, PROT_NONE). We have established that MAP_NORESERVE is a noop, so this would be probably fine. Still, I'd do the mmap(PROT_RW) for commit instead for !exec:
>
> if (exec)
> // Do not replace MAP_JIT mappings, see JDK-8234930
> return mprotect() == 0;
> } else {
> mmap ...
> }
> If not, I would remove MAP_NORESERVE from this code.
> You need to handle mprotect failure here too.
They are handled later https://github.com/openjdk/jdk/pull/294/files#diff-1f93205c2e57bee432f8fb7a0725ba1dfdbe5b901ac63010ea0b43922e34ac12R1708
> Also, this is asymetric to uncommit now for the !exec case. Still, I'd do the mmap(PROT_RW) for commit instead for !exec:
Thanks, this looks good, I've applied the suggestion.
-------------
PR: https://git.openjdk.java.net/jdk/pull/294
More information about the hotspot-dev
mailing list