RFR: 8049536: os::commit_memory on Solaris uses aligment_hint as page size

[Erik Helin]

Hi Thomas,

thanks for having a look!

On 2014-09-11 17:23, Thomas Schatzl wrote:
>
> Hi,
>
> On Thu, 2014-09-11 at 16:15 +0200, Erik Helin wrote:
>> Hi all,
>>
>> this small patch fixes a problem with the arguments to the syscall
>> memcntl on Solaris.
>>
>> When memcntl is used with MHA_MAPSIZE_VA to tell the system the
>> preferred page size for a memory area, the mha_pagesize member of the
>> struct memcntl_mha argument must be a valid page size (a valid page size
>> if one of the sizes returned by the syscall getpagesizes).
>>
>> One variant of the function os::commit_memory has an alignment_hint
>> parameter and the code in os::Solaris::commit_memory_impl sometimes
>> passes this alignment_hint as the mha_pagesize to memcntl. The issue is
>> that there is no check that this alignment_hint actually is a valid page
>> size.
>>
>
> Where does this wrong alignment come from? Only from the user via
> -XX:LargePageAlignmentInBytes?

No, the alignment hint comes from 
CollectorPolicy::generation_alignment(), which is passed to the parallel 
scavenge code as the generation alignment when setting up the heap.

On 2014-09-11 17:23, Thomas Schatzl wrote:
> If so, wouldn't it be better to make this adjustment during argument
> processing? Then the VM could inform the user about the change to some
> degree, either per message, or by overriding the given value
> (using FLAG_SET_xxx).
>
> The alignment_hint should still be checked for validity when it is
> used, but I think if the method gets a wrong value for the hint from
> anywhere other than the user, it imo should be an error. Not silently
> adjusted which potentially leads to hard to find performance problems.

As we discussed on IM, it seems like the alignment_hint is meant to be 
used like a hint, and then the OS specific code can decide the page size 
that is most suitable. In os_linux.cpp for example, the alignment_hint 
is only used for checking if the caller might want to use large pages 
(not the actual page size). BSD and Windows ignores the hint.

In the Solaris code today, the page size is adjusted if the 
alignment_hint is larger than the largest page size, otherwise the 
alignment_hint is treated an exact page size.

I've added a comment to the code explaining that memcntl requires an 
exact page.

Please see new webrevs:
- full: http://cr.openjdk.java.net/~ehelin/8049536/webrev.01/
- incremental: http://cr.openjdk.java.net/~ehelin/8049536/webrev.00-01/

On 2014-09-11 17:23, Thomas Schatzl wrote:
>> Testing:
>> - JPRT
>> - Tested with the patch for
>>     https://bugs.openjdk.java.net/browse/JDK-8027915
>>     which immediately provokes the bug on Solaris SPARC when just running
>>     java -version. Now it works fine.
>
> Can you add a test case that checks there is no "MPSS failed" message with
> various LargePageAlignmentInBytes values?

Sure, although I no longer managed to reproduce the bug with only
java -Xms32m -Xmx128m -XX:LargePageSizeInBytes=256m -version

Let me see if I can come up with some flags that provokes the bug even 
without my fix for JDK-8027915.

Thanks,
Erik

> Thanks,
>    Thomas
>
>