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

[David Holmes]

Looks good Chris!

Thanks.

David

On 23/03/2017 7:38 AM, Chris Plummer wrote:
> Hello,
>
> Sorry to beat a dead horse here, but in addition to Dan's recommended
> fixes, I also reworked how I check for overflow when aligning up thanks
> to a suggestion from David:
>
> http://cr.openjdk.java.net/~cjplummer/8176768/webrev.03/webrev.hotspot/
>
> src/os/linux/vm/os_linux.cpp:
>
>  726   // behaviour. However, be careful not to end up with a size
>  727   // of zero due to overflow. Don't add the guard page in that case.
>  728   size_t guard_size = os::Linux::default_guard_size(thr_type);
>  729   if (stack_size <= SIZE_MAX - guard_size) {
>  730     stack_size += guard_size;
>  731   }
>  732   assert(is_size_aligned(stack_size, os::vm_page_size()),
> "stack_size not aligned");
>
> src/os/posix/vm/os_posix.cpp:
>
> 1260   // pthread_attr_setstacksize() may require that the size be
> rounded up to the OS page size.
> 1261   // Be careful not to round up to 0. Align down in that case.
> 1262   if (stack_size <= SIZE_MAX - vm_page_size()) {
> 1263     stack_size = align_size_up(stack_size, vm_page_size());
> 1264   } else {
> 1265     stack_size = align_size_down(stack_size, vm_page_size());
> 1266   }
>
> Dan's comments below for reference:
>> src/os/aix/vm/os_aix.cpp
>>       No comments.
>>
>> src/os/bsd/vm/os_bsd.cpp
>>       No comments.
>>
>> src/os/linux/vm/os_linux.cpp
>>       L729:   // In that cast just subract the page size to get the
>> maximum possible stack size.
>>           Typo: 'cast' -> 'case'
>>           Typo: 'subract' -> 'subtract' (Thomas also commented on it)
>>
>> src/os/posix/vm/os_posix.cpp
>>       L263:   // aligning up could have resulted in the size being 0. In
>> that case just subract the
>>           Nit: 'aligning' -> 'Aligning' (since it's a sentence)
>>           Typo: 'subract' -> 'subtract'
>>
>> src/os/solaris/vm/os_solaris.cpp
>>       No comments.
>>
>> src/share/vm/prims/jvm.cpp
>>       L2812:       // -Avoid truncating on 32-bit platforms if size is
>> greater than UINT_MAX
>>           Nit: needs a period at the end like L2813.
>>
>> test/runtime/Thread/TooSmallStackSize.java
>>       No comments.
>>
>> test/runtime/Thread/TestThreadStackSizes.java
>>       L26:  * @summary Test user threads with various stacks sizes.
>>           Typo?: "stacks sizes" -> "stack sizes"
>>
>>       L36:         super(null, null, "TestThreadStackSizes"+stackSize,
>> stackSize);
>>           Nit: spaces around the "+".
>>
>>       L46:             TestThreadStackSizes testThreadStackSize  = new
>> TestThreadStackSizes(stackSize);
>>           Nit: extra space before '='.
> thanks
>
> Chris