RFR(xxs): 8187230: [aix] Leave OS guard page size at default for non-java threads instead of explicitly setting it

[Thomas Stüfe]

On Wed, Oct 18, 2017 at 9:12 AM, David Holmes <david.holmes at oracle.com>
wrote:

> Looks fine to me.
>
> Cheers,
> David
>
>
Thanks David!


> On 18/10/2017 5:10 PM, Thomas Stüfe wrote:
>
>> Hi all,
>>
>> I am cleaning up my backlog of old issues which did not make it into the
>> repo before the consolidation.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8187230 <
>> https://bugs.openjdk.java.net/browse/JDK-8187230>
>>
>> Last Webrev (just rebased to the new repo structure, no changes):
>> http://cr.openjdk.java.net/~stuefe/webrevs/8187230-aix-leave
>> -os-guard-page-size-at-default-for-non-java-threads/webrev.02/webrev/
>>
>> For your convenience, here the original message:
>>
>> <<<
>> The change is very subtle.
>>
>> Before, we would set the OS guard page size for every thread - for java
>> threads disable them, for non-java threads we'd set them to 4K.
>>
>> Now, we still disable them for java threads but leave them at the OS
>> default size for non-java threads.
>>
>> The really important part is the disabling of OS guard pages for java
>> threads, where we have a VM guard pages in place and do not want to spend
>> more memory on OS guards. We do not really care for the exact size of the
>> OS guard pages for non-java threads, and therefore should not set it - we
>> should leave the size in place the OS deems sufficient. That also spares us
>> the complexity of handling the thread stack page size, which on AIX may be
>> different from os::vm_page_size().
>>  >>>
>>
>> @Chris: you did ask whether this would make sense for Linux too. I think
>> you are right, but as Goetz pointed out matters are more complicated as
>> glibc pthread_create does not substract OS guard size from the user
>> specified stack size, so it requires us to know the OS guard size and add
>> it to the specified stack size (funny, the same issue we have with VM
>> guards and -Xss). So, for now, I'd prefer this to keep AIX only.
>>
>> I think I need a second reviewer beside Goetz.
>>
>> Thanks!
>>
>> Thomas
>>
>>
>>
>> On Fri, Sep 8, 2017 at 10:48 AM, Thomas Stüfe <thomas.stuefe at gmail.com
>> <mailto:thomas.stuefe at gmail.com>> wrote:
>>
>>     Hi Guys,
>>
>>     On Fri, Sep 8, 2017 at 9:51 AM, Lindenmaier, Goetz
>>     <goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com>> wrote:
>>
>>         Hi Chris,
>>
>>         on linux the pthread implementation is a bit strange, or buggy.
>>         It takes the OS guard pages out of the stack size specified.
>>         We need to set it so we can predict the additional space
>>         that must be allocated for the stack.
>>
>>         See also the comment in os_linux.cpp, create_thread().
>>
>>
>>     Goetz, I know we talked about this off list yesterday, but now I am
>>     not sure this is actually needed. Yes, to correctly calculate the
>>     stack size, we need to know the OS guard page size, but we do not
>>     need to set it, we just need to know it. So, for non-java threads
>>     (java threads get the OS guard set to zero), it would probably be
>>     sufficient to:
>>
>>     - pthread_attr_init() (sets default thread attribute values to the
>>     attribute structure) and then
>>     - pthread_attr_getguardsize() to read the guard size from that
>>     structure.
>>
>>     That way we leave the OS guard page at the size glibc deems best. I
>>     think that is a better option. Consider a situation where the glibc
>>     changes the size of the OS guard pages, for whatever reason - we
>>     probably should follow suit.
>>
>>     See e.g. this security issue - admittedly only loosely related,
>>     since the fix for this issue seemed to be a fix to stack banging,
>>     not changing the OS guard size:
>>     https://access.redhat.com/security/vulnerabilities/stackguard
>>     <https://access.redhat.com/security/vulnerabilities/stackguard>
>>
>>     So, in short, I think we could change this for Linux too. If you
>>     guys agree, I'll add this to the patch. Since I am on vacation and
>>     the depot is closed, it may take some time.
>>
>>     Kind Regards, Thomas
>>
>>
>>
>>
>>         Best regards,
>>            Goetz.
>>
>>         > -----Original Message-----
>>         > From: ppc-aix-port-dev [mailto:ppc-aix-port-dev-bounc
>> es at openjdk.java.net
>>         <mailto:ppc-aix-port-dev-bounces at openjdk.java.net>]
>>         > On Behalf Of Chris Plummer
>>         > Sent: Thursday, September 07, 2017 11:07 PM
>>         > To: Thomas Stüfe <thomas.stuefe at gmail.com <mailto:
>> thomas.stuefe at gmail.com>>;
>>         ppc-aix-port-
>>         > dev at openjdk.java.net <mailto:dev at openjdk.java.net>
>>         > Cc: HotSpot Open Source Developers <
>> hotspot-dev at openjdk.java.net <mailto:hotspot-dev at openjdk.java.net>>
>>         > Subject: Re: RFR(xxs): 8187230: [aix] Leave OS guard page size
>> at default for
>>         > non-java threads instead of explicitly setting it
>>         >
>>          > Hi Thomas,
>>          >
>>          > Is there a reason this shouldn't also be done for linux?
>>          >
>>          > thanks,
>>          >
>>          > Chris
>>          >
>>          > On 9/7/17 3:02 AM, Thomas Stüfe wrote:
>>          > > Hi all,
>>          > >
>>          > > may I please have a review for this small change:
>>          > >
>>          > > Bug:
>>          > > https://bugs.openjdk.java.net/browse/JDK-8187230
>>         <https://bugs.openjdk.java.net/browse/JDK-8187230>
>>          > >
>>          > > Webrev:
>>          > > http://cr.openjdk.java.net/~stuefe/webrevs/8187230-aix-
>>         <http://cr.openjdk.java.net/~stuefe/webrevs/8187230-aix->
>>          > > leave-os-guard-page-size-at-default-for-non-java-
>>          > threads/webrev.00/webrev/
>>          > >
>>          > > The change is very subtle.
>>          > >
>>          > > Before, we would set the OS guard page size for every
>>         thread - for java
>>          > > threads disable them, for non-java threads we'd set them to
>> 4K.
>>          > >
>>          > > Now, we still disable them for java threads but leave them
>>         at the OS
>>          > > default size for non-java threads.
>>          > >
>>          > > The really important part is the disabling of OS guard
>>         pages for java
>>          > > threads, where we have a VM guard pages in place and do not
>>         want to
>>          > spend
>>          > > more memory on OS guards. We do not really care for the
>>         exact size of the
>>          > > OS guard pages for non-java threads, and therefore should
>>         not set it - we
>>          > > should leave the size in place the OS deems sufficient.
>>         That also spares us
>>          > > the complexity of handling the thread stack page size,
>>         which on AIX may be
>>          > > different from os::vm_page_size().
>>          > >
>>          > > Thank you and Kind Regards, Thomas
>>          >
>>          >
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20171018/82b8655e/attachment-0001.html>