Review request 6983240: guarantee((Solaris::min_stack_allowed >= (StackYellowPages+StackRedPages...) wrong

[Vladimir Kozlov]

Looks good for me.

Vladimir

Coleen Phillimore wrote:
> 
> Can I get one more review?
> 
> webrev at http://cr.openjdk.java.net/~coleenp/6983240_2/
> 
> Thanks,
> Coleen
> 
> 
> 
> On 10/05/10 11:26, Coleen Phillimore wrote:
>>
>> Thanks David - some comments inline...
>>
>> On 10/04/10 20:41, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> In os_solaris.cpp this comment should now refer to 2*BytesPerWord
>> Okay.
>>>
>>>   // Add in BytesPerWord times page size to account for VM stack during
>>>   // class initialization depending on 32 or 64 bit VM.
>>>
>>> That said this formulation (not your I know) is confusing. What does 
>>> bytesPerWord have to do with the amount of stack needed? Is this just 
>>> some weird way of getting twice the amount of stack on 64-bit as 
>>> 32-bit? Are the pages sizes even the same in that case?
>> Yeah, I believe it's just to account for 64 vs. 32 bit.  The page 
>> sizes are the same if 32 or 64 bit.  I have
>>>
>>> Also in os_solaris.cpp isn't this guarantee always false:
>>>
>>> !   guarantee(os::Solaris::min_stack_allowed >= thr_min_stack(),
>>> !             "need to increase min_stack_allowed");
>> No, it doesn't assert, because in your next mail you found that 
>> thr_min_stack() is smaller than our min_stack_allowed.  I could revert 
>> that change but min_stack_allowed() is for the VM to get initialized 
>> rather than this thr_min_stack() which is for the OS to initialize the 
>> thread.  I would assume the latter is always smaller than the former 
>> or something is really wrong.  Changing it back probably doesn't hurt 
>> anything.
>>
>> Thanks,
>> Coleen
>>>
>>> AFAICS min_stack_allowed is set to one of these values:
>>>
>>> ./os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp:size_t 
>>> os::Solaris::min_stack_allowed = 128*K;
>>> ./os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp:size_t 
>>> os::Solaris::min_stack_allowed = 96*K;
>>> ./os_cpu/solaris_x86/vm/os_solaris_x86.cpp:size_t 
>>> os::Solaris::min_stack_allowed = 224*K;
>>> ./os_cpu/solaris_x86/vm/os_solaris_x86.cpp:size_t 
>>> os::Solaris::min_stack_allowed = 64*K;
>>>
>>> but the manpage for thr_min_stack states:
>>>
>>>     thr_min_stack() will return the unsigned int  THR_MIN_STACK,
>>>     which is the minimum-allowable size for a thread's stack.
>>>
>>>     In this implementation the default size for a  user-thread's
>>>     stack   is   one   mega-byte.
>>>
>>> 1MB > all the above. ???
>>>
>>> Cheers,
>>> David
>>>
>>> Coleen Phillimore said the following on 10/05/10 08:45:
>>>> Summary: min_stack_allowed is a compile time constant and 
>>>> Stack*Pages are settable
>>>>
>>>> Also stress tested various combinations of -Xss and 
>>>> -XX:StackShadowPages=n.  This also fixes a bug
>>>> 6346701: stack overflow in native method results in segfault, not a 
>>>> StackOverflowError
>>>> or rather it fixes the observed SEGVs with different settings of 
>>>> -Xss and -XX:StackShadowPages in the evaluation.  We were getting 
>>>> stack overflow before the stack overflow exception was initialized, 
>>>> causing infinite recusion trying to initialize it.
>>>>
>>>> Also fixed a random g++ compilation error.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/6983240/
>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=6983240
>>>>
>>>> Thanks,
>>>> Coleen
>>
>