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

[Thomas Stüfe]

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?

Also, happy easter :)

...Thomas Stuefe