RFR(M): 8169373: Work around linux NPTL stack guard error.

[David Holmes]

Hi Goetz,

On 17/11/2016 1:52 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I addressed the additional topics, but in an extra change.
>
> First, I rounded up the NPTL bug workaround:
> http://cr.openjdk.java.net/~goetz/wr16/8169373-ppc-stackFix/webrev.02
>
> I fixed os::Posix::describe_pthread_attr() and current_stack_region()
> In the os_cpu files.
> Interestingly, in zero,  current_stack_region() is already fixed.

So on Linux, pthread_attr_getstack is also broken - in that it returns 
the address at the end of the guard section, instead of the usable stack?

Can you add error checking on the pthread_attr_getguardsize call please.

The duplication of the current_stack_region static method in the os-cpu 
files is horrible. This should be placed as a private method in the os 
class and moved to os_linux.cpp. RFE for 10.


> Then I edited an extra webrev to fix
>   - unnecessary OS guard page on Compiler threads
>   - unpredictable minimal stack sizes because of varying (guard) page sizes:
> http://cr.openjdk.java.net/~goetz/wr16/8169373-ppc-stackFix/webrev.minStSz/
>
> Compiler threads now don't get OS guard pages.  Saves one page per compiler thread.
>
> Also, the constants in the os_<os>_<cpu>.cpp files now list minimum space
> required for frames.  I basically reduced them by the size of the guard zones.
> Where minimum stack sizes are computed, the page size is known and the
> minimal value is extended by the required space for the guard pages. As compiler threads
> don't bang, no space for the shadow zone is needed.
>
> Should I add this additional change to 8169373?  As this only deals with minimal
> stack sizes, and reduces these sizes, I don't see a relevant risk.
> Only on systems with large pages the minimal sizes now might be bigger, but
> then the old sizes would have led to immediate stack overflows.

Sorry this is getting a bit too divergent and disruptive to me for this 
stage of 9. You are redefining what the "minimum stack" values mean as 
well as changing their actual value. (BTW I'm sure there is a test of 
these values that would need updating.)

I can't tell if it is right/wrong, necessary/unnecessary - but given the 
recent work done in this area I'm somewhat cautious about changing it 
again - in 9.

David

>
> Best regards,
>   Goetz.
>
>
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Dienstag, 15. November 2016 00:54
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-runtime-
>> dev at openjdk.java.net
>> Subject: Re: RFR(M): 8169373: Work around linux NPTL stack guard error.
>>
>> Hi Goetz,
>>
>> On 15/11/2016 3:12 AM, Lindenmaier, Goetz wrote:
>>> Hi David,
>>>
>>>> Did you really intend to increase the real minimum stack from 128K to
>>>> 256K ? (on a 64K page system)
>>> I finally figured why I have to set such high minimum values.
>>>
>>> A CompilerThread is a JavaThread. It's getting yellow, red etc
>>> pages at the top. As these are 64K on 64K systems, the compiler
>>> thread stack has to be this big. Thus you actually need similar
>>> values once you run a linuxx86_64 VM on a system with 64K pages.
>>> Does this exist?
>>
>> No idea, sorry.
>>
>>> But, this means that there is no use of the OS guard pages
>>> configured in default_guard_size().  This is called with 'compiler_thread'
>>> for CompilerThreads.
>>>
>>> We should probably also return '0' in that case.
>>>
>>> What do you think?
>>
>> I think I agree. As you note a CompilerThread is-a JavaThread even
>> though the ThreadType enum treats them as completely distinct:
>>
>>    enum ThreadType {
>>      vm_thread,
>>      cgc_thread,        // Concurrent GC thread
>>      pgc_thread,        // Parallel GC thread
>>      java_thread,
>>      compiler_thread,
>>      watcher_thread,
>>      os_thread
>>    };
>>
>> so anything with VM supplied guard pages doesn't need the glibc ones as
>> well.
>>
>> I'm struggling to understand why os::Linux::default_guard_size is
>> defined per CPU type? I would not expect this to need to vary based on
>> CPU at all. ??
>>
>> Thanks,
>> David
>>
>>> Best regards,
>>>   Goetz.
>>>
>>> size_t os::Linux::default_guard_size(os::ThreadType thr_type) {
>>>   // Creating guard page is very expensive. Java thread has HotSpot
>>>   // guard page, only enable glibc guard page for non-Java threads.
>>>   return (thr_type == java_thread ? 0 : page_size());
>>> }
>>>