Getting rid of the requested_addr parameter in os::reserve_memory?

[Stefan Karlsson]

Hi Thomas,

On 2015-04-02 12:06, Thomas Stüfe wrote:
> Hi all,
>
> Proposal: I would like to remove the "requested_addr" parameter from
> os::reserve_memory(), because it is dangerous and should not be used.
>
> Here is why:
>
> when reserving memory, there are two reasons to specify a wish address.
>
> 1) we want to reserve a memory range at a given address, because we find
> that address numerically pleasing for some reason (heap optimized for
> compressed oops etc). In this case, we only want to probe; we can live with
> the address being different and certainly do not want to trash any
> pre-existing mappings.
>
> 2) we want to change properties of a pre-existing memory range, e.g. commit
> the memory which was only reserved before.
>
> For both cases, os::reserve_memory() is the wrong choice. For (1), we have
> os::attempt_reserve_memory_at(). For (2), we have os::commit_memory() etc.
>
> In the current implementation of os::reserve_memory(), when requested_addr
> != NULL, we may trash existing mappings, this depends on the implementation.
> - On windows, we use VirtualAlloc(), which will not trash.
> - On mmap-based implementations (linux,bsd,solaris), specifying
> requested_addr on os::reserve_memory() will cause mmap() to be called with
> MAP_FIXED, which at least on Linux and Solaris (not sure about BSD) trashes
> pre-existing mappings.
> - On AIX, we use SystemV shm for most allocations; besides, MAP_FIXED won't
> normally trash existing mappings.
>
> Normally, os::reserve_memory() is called with requested_addr=NULL; I found
> only two places where we actually hand a non-NULL requested_addr to
> os::reserve_memory(), and I think both could be considered errors:
>
> A) on Windows, we use it in os::pd_split_reserved_memory(). Here, we want
> to split a preallocated range into two ranges, in order to be able to
> release them independentally. This code wants to work around the fact that
> VirtualAlloc() allocations are one entity and can only be released as such,
> as opposed to mmap(), where you can unmap page-wise.
> It does this by deallocating the whole range and allocating twice two
> separate ranges in the same address range. This coding is racy: some other
> allocation may come between the deallocation and the subsequent
> allocations. Neither is there any error checking, so we will not notice if
> this fails.
>
> B) on Linux, we call os::reserve_memory() with a non-NULL address in
> os::Linux::reserve_memory_special_huge_tlbfs_mixed(). This code gets called
> via some code paths when allocating large paged heap, and I think this is
> simply wrong and may trash existing mappings. The larger the heap is and
> the more refined the
> "lets-try-to-allocate-heap-at-compressed-oops-friendly-address" logic, the
> more probable this becomes.
>
> ----
>
> Therefore I would like to get rid of the requested_addr parameter in
> os::reserve_memory() and also change code that uses it (A) and (B) to
> either attempt_reserve_memory_at() or something different altogether. (A)
> could be just rewritten using VirtualAlloc() calls directly, though that
> does not solve the bad design. (B), I don't know - would have to look at
> this more closely.
>
> Note that this is not a theoretical problem; from time to time we have
> problems because programmers are unaware of the MAP_FIXED problem and used
> os::reserve_memory() instead of os::attempt_reserve_memory_at(). The nasty
> thing about this is that you normally will not notice, not in a normal java
> launcher scenario, where memory allocation at process startup is somewhat
> deterministic. But our (SAP) java VM gets used a lot with other launchers,
> which usually initialize the java vm last, after doing a lot of other
> stuff; here, the address space may be already populated with lots of
> non-java mappings.
>
> What do you think?

Sounds good to me.

StefanK

>
> Also, happy easter :)
>
> ...Thomas Stuefe