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

[Chris Plummer]

Ok, time for a new webrev:

http://cr.openjdk.java.net/~cjplummer/8176768/webrev.01/webrev.hotspot

The only thing that has changed since the first webrev are the asserts 
added to os_linux.cpp and os_bsd.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 PTHREAD_STACK_MIN.
  - The assert should never happen due to an unaligned stack size 
because we always align it to the page size.
  - 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

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:
>
>       pthread_attr_setstacksize(&attr, stack_size);
>
> 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 pthread_attr_setstacksize() call. The end result is 
> pthread_attr_setstacksize() 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 PTHREAD_STACK_MIN after computing their 
> new values:
>
>       _java_thread_min_stack_allowed
>       _compiler_thread_min_stack_allowed
>       _vm_internal_thread_min_stack_allowed
>
> For solaris, there was an issue using PTHREAD_STACK_MIN. You need to 
> #define _POSIX_C_SOURCE >= 199506L in order to get PTHREAD_STACK_MIN 
> #defined, and this needs to be done before including OS header files. 
> I noticed that on solaris we were using thr_min_stack() elsewhere 
> instead of PTHREAD_STACK_MIN, so I decided to do the same with this 
> fix. Either way is ugly (the #define or using thr_min_stack()).
>
> And speaking of the existing use of thr_min_stack(), 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 thr_min_stack() 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 thr_min_stack() 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