RFR(S): 8186286: [BSD] Primary thread's stack size is reported incorrectly

[Thomas Stüfe]

Hi David,

On Thu, Aug 17, 2017 at 11:54 PM, David Holmes <david.holmes at oracle.com>
wrote:

> Hi Gunter,
>
> On 18/08/2017 1:01 AM, Haug, Gunter wrote:
>
>> Thanks for the review, Aleksey and Thomas
>>
>> You’re right, it is much nicer to use the macros. I’ve updated the change
>> accordingly:
>> http://cr.openjdk.java.net/~ghaug/webrevs/8186286.v1
>>
>
> I'm unclear about the logic change here. If you round down then there is a
> chance you will enter the following if-block where otherwise you would not.
>
>  915     *size = align_down(*size, getpagesize());
>  916
>  917     if ((*size) < (DEFAULT_MAIN_THREAD_STACK_PAGES *
> (size_t)getpagesize())) {
>
>
I don't see how this could happen? Gunters fix causes the size to snap to
the lower page boundary, but never cross it. The code below ensures a lower
cap at a page boundary.

If size was larger than that cap, after the align_down it will be at most
right at that cap. If it was lower than the cap, it stays lower. In both
cases behavior  is unchanged.

Cheers, Thomas

Have you verified with a range of aligned and unaligned stack sizes around
> that threshhold that everything works okay?
>
> Some typos in the comment block:
>
> alligned -> aligned
> boundries -> boundaries
> I round -> We round
>
> @Thomas: os::vm_page_size() is not used in current_stack_region(), I think
>> because of initialization dependencies.
>>
>
> I don't see the dependency. It requires that it only be used after
> os::init() has been called. AFAICS the first time we will use this logic is
> when we attach the main thread, which happens after os::init().

Thanks,
> David
>
> Best regards,
>> Gunter
>>
>> From: Thomas Stüfe <thomas.stuefe at gmail.com>
>> Date: Thursday, 17. August 2017 at 15:06
>> To: "Haug, Gunter" <gunter.haug at sap.com>
>> Cc: "hotspot-dev at openjdk.java.net" <hotspot-dev at openjdk.java.net>
>> Subject: Re: RFR(S): 8186286: [BSD] Primary thread's stack size is
>> reported incorrectly
>>
>> Hi Gunter,
>>
>>
>> On Thu, Aug 17, 2017 at 2:24 PM, Haug, Gunter <gunter.haug at sap.com
>> <mailto:gunter.haug at sap.com>> wrote:
>> Hi,
>>
>> can I please have reviews and a sponsor fort the following small bug fix:
>> http://cr.openjdk.java.net/~ghaug/webrevs/8186286/
>> https://bugs.openjdk.java.net/browse/JDK-8186286
>>
>> At least on Mac OS 10.12 we have observed stack sizes of the primary
>> thread not aligned to pages boundries. This can be provoked by e.g.
>> setrlimit() (ulimit -s xxxx in the shell).This voids the computation of the
>> addresses of the guard pages.
>>
>> Fix:
>> Apparently Mac OS actually rounds upwards to next multiple of page size
>> however, it is conservative to round downwards here to be on the safe side.
>>
>> Thanks and best regards,
>> Gunter
>>
>> Thanks for the patch!
>>
>> Very minor nits: what Alexey wrote (we also have "is_aligned"). Plus, any
>> reason not to use os::vm_page_size()? Initialization dependencies?
>>
>> Kind Regards, Thomas
>>
>>
>>
>>