(RFR)(S)(10): 8176768: hotspot ignores PTHREAD_STACK_MIN when creating new threads (original) (raw)

Thomas Stüfe thomas.stuefe at gmail.com
Fri Mar 17 06:48:49 UTC 2017

Hi Chris,

please find my answers inline.

On Fri, Mar 17, 2017 at 5:27 AM, Chris Plummer <chris.plummer at oracle.com> wrote:

Ok, time for a new webrev:

http://cr.openjdk.java.net/~cjplummer/8176768/webrev.01/webrev.hotspot The changes look good.

The only thing that has changed since the first webrev are the asserts added to oslinux.cpp and osbsd.cpp. And to summarize what we discuss already:

- The assert should never happen due to the stack size being too small since it will be at least PTHREADSTACKMIN. - The assert should never happen due to an unaligned stack size because we always align it to the page size.

As a side note, on AIX we have a complicated page scheme where thread stacks may have a different page size from the java heap or from the primordial thread. But using os::vm_page_size() should be ok, we ensure that os::vm_page_size is always the same or a multiple of the thread stack page size.

Would you mind adding the assert to os_aix.cpp too?

- Any assert would therefore be a VM bug and not due to user error. - No fixing the java launcher. If the user specifies a stack that is too small, hotspot will already detect this. If the user specifies a stack size that is large enough but not page aligned, then we just ignore any error (if the platform doth protest) and the user gets a main thread with a stack size set to whatever the OS default is.

I still need to retest (I only ran TooSmallStackSize.java), but figure getting agreement on the changes first would be best before I bog down our testing resources. thanks, Chris I am all fine with the changes.

Kind Regards, Thomas

On 3/15/17 10:03 PM, Chris Plummer wrote:

Hello,

Please review the following: https://bugs.openjdk.java.net/browse/JDK-8176768 http://cr.openjdk.java.net/~cjplummer/8176768/webrev.00/webrev.hotspot While working on 8175342 I noticed our stack size on xgene was 8mb even though I was specifying -Xss72k. It turns out the following code was failing: pthreadattrsetstacksize(&attr, stacksize); Although we computed a minimum stack size of 72k, so -Xss72k should be fine, pthreads on this platform requires the stack be at least 128k, so it failed the pthreadattrsetstacksize() call. The end result is pthreadattrsetstacksize() had no impact on the thread's stack size, and we ended up with the platform default of 8mb. The fix is to round up the following variables to PTHREADSTACKMIN after computing their new values: javathreadminstackallowed compilerthreadminstackallowed vminternalthreadminstackallowed For solaris, there was an issue using PTHREADSTACKMIN. You need to #define POSIXCSOURCE >= 199506L in order to get PTHREADSTACKMIN #defined, and this needs to be done before including OS header files. I noticed that on solaris we were using thrminstack() elsewhere instead of PTHREADSTACKMIN, so I decided to do the same with this fix. Either way is ugly (the #define or using thrminstack()). And speaking of the existing use of thrminstack(), I deleted it. It was being applied before any adjustments to the stack sizes had been made (rounding and adding red, yellow, and shadow zones). This mean the stack ended up being larger than necessary. With the above fix in place, we are now applying thrminstack() after recomputing the minimum stack sizes. If for any reason one of those stack sizes is now too small, the correct fix is to adjust the initial stack sizes, not apply thrminstack() to the initial stack sizes. However, it looks like no adjustment is needed. I did something close to our nightly testing on all affect platforms, and no new problems turned up. thanks, Chris

More information about the hotspot-dev mailing list