RFR(s): 8077276: allocating heap with UseLargePages and HugeTLBFS may trash existing memory mappings (linux)

[Coleen Phillimore]

On 5/5/15, 6:51 AM, Thomas Stüfe wrote:
> Hi Coleen,
>
> thanks a lot for reviewing! Attached find my hg export. I retested 
> with x86, x64, ppc64, hence the delay.

Checkin going now.
>
> On Mon, May 4, 2015 at 10:05 PM, Coleen Phillimore 
> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>> 
> wrote:
>
>
>     Hi Thomas,
>
>     I reviewed the latest version.  Thanks to Stefan who reviewed the
>     earlier versions and suggested changes.
>
>     This looks great.  I am not an expert at large pages (or even that
>     knowledgeable) but your change addresses the safety concern that
>     you had and the code appears to do what you said.
>
>
> :-)
>
>
>     Is there a follow-up RFE to clean out the last use of and remove
>     the requested address parameter to os::reserve_memory()?
>
>
> Yes, I plan to do this as one of my next changes.

Excellent, thanks!
>
>        This os::reserve_memory() calling os::pd_reserve_memory
>     layering is really annoying and seems not helpful.
>
>
> I think the original intend was to define entry points to be tracked 
> by NMT - so, os::reserve_memory would be os::pd_reserve_memory + NMT 
> tracking. But it did not get used this way, so in the end, I also do 
> not find it very helpful.

It would be better not to have these thin layers on pd_reserve_memory 
and put the NMT code in os_<os>.cpp instead.  I think.

Thanks!
Coleen
>
>
>     Thanks,
>     Coleen
>
>     (also, I will sponsor it for you if you send me an hg exported
>     version of your commit)
>
>
> Again thanks, and thanks to Stefan too. I am happy this change is 
> finally approved.
>
> Thomas
>
>     On 4/30/15, 3:33 AM, Thomas Stüfe wrote:
>
>         Hi all,
>
>         I realize that this patch may look more complicated than it is.
>
>         Basically, the problem is that under certain conditions, memory is
>         allocated using mmap(addr, MAP_FIXED) for an initial
>         reservation (e.g. java
>         heap), which may trash existing mappings.
>
>         The fix is basically just to remove the MAP_FIXED flag for the
>         initial
>         allocation.
>
>         Fix looks more complicated than this because the test
>         functions were
>         expanded to add regression tests for this fix.
>
>         Please tell me if I should dumb down this fix or add explanations.
>
>         Thanks & Kind regards, Thomas
>
>
>
>         On Thu, Apr 30, 2015 at 8:39 AM, David Holmes
>         <david.holmes at oracle.com <mailto:david.holmes at oracle.com>>
>         wrote:
>
>             On 30/04/2015 4:59 AM, Thomas Stüfe wrote:
>
>                 Could I have another reviewer, please, and a sponsor?
>
>             Latest webrev is:
>
>             http://cr.openjdk.java.net/~stuefe/webrevs/8077276/webrev.05/webrev
>             <http://cr.openjdk.java.net/%7Estuefe/webrevs/8077276/webrev.05/webrev>
>
>             Sorry not something I can review in depth.
>
>             David
>
>
>               Thanks!
>
>                 Thomas
>
>
>                 On Thursday, April 9, 2015, Thomas Stüfe
>                 <thomas.stuefe at gmail.com
>                 <mailto:thomas.stuefe at gmail.com>> wrote:
>
>                   Hi all,
>
>                     please review this fix to huge page allocation on
>                     Linux.
>
>                     bug: https://bugs.openjdk.java.net/browse/JDK-8077276
>                     webrev:
>                     http://cr.openjdk.java.net/~stuefe/webrevs/8077276/webrev.00/webrev/
>                     <http://cr.openjdk.java.net/%7Estuefe/webrevs/8077276/webrev.00/webrev/>
>
>                     os::Linux::reserve_memory_special_huge_tlbfs_mixed()
>                     first establishes a
>                     mapping with small pages over the whole requested
>                     range, then exchanges
>                     the
>                     parts of the mapping which are aligned to large
>                     page size with large
>                     pages.
>
>                     Now, when establishing the first mapping, it uses
>                     os::reserve_memory()
>                     with a potentially non-null req_addr. In that
>                     case, os::reserve_memory()
>                     will do a mmap(MAP_FIXED).
>
>                     This will trash any pre-existing mappings.
>
>                     Note that I could not willingly reproduce the
>                     error with an unmodified
>                     VM.
>                     But I added a reproduction case which demonstrated
>                     that if one were to
>                     call
>                     os::Linux::reserve_memory_special_huge_tlbfs_mixed()
>                     with a non-NULL
>                     req_addr, existing mappings would be trashed.
>                     Depending on where we are
>                     in
>                     address space, we also would overwrite libc
>                     structures and crash
>                     immediately.
>
>                     The repro case is part of the change, see changes in
>                     test_reserve_memory_special_huge_tlbfs_mixed(),
>                     and can be executed with:
>
>                     ./images/jdk/bin/java -XX:+UseLargePages
>                     -XX:+UseHugeTLBFS -XX:-UseSHM
>                     -XX:+ExecuteInternalVMTests
>                     -XX:+VerboseInternalVMTests
>
>                     The fix: instead of using os::reserve_memory(),
>                     os::Linux::reserve_memory_special_huge_tlbfs_mixed()
>                     now calls mmap()
>                     directly with the non-NULL req_addr, but without
>                     MAP_FIXED. This means
>                     the
>                     OS may do its best to allocate at req_addr, but
>                     will not trash any
>                     mappings. This also follows the pattern in
>                     os::Linux::reserve_memory_special_huge_tlbfs_only(),
>                     which is a sister
>                     function of
>                     os::Linux::reserve_memory_special_huge_tlbfs_mixed().
>
>                     Note also discussion at:
>                     http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-April/017823.html
>                     .
>
>                     Thanks for reviewing!
>
>                     Kind Regards, Thomas
>
>
>
>
>