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

Coleen Phillimore coleen.phillimore at oracle.com
Fri Nov 18 14:40:23 UTC 2016

On 11/18/16 1:34 AM, David Holmes wrote:
> 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.
By duplication do you mean this bit of code?


+ // Work around NPTL stack guard error.
+ size_t guard_size = 0;
+ pthread_attr_getguardsize(&attr, &guard_size);
+ *bottom += guard_size;
+ *size -= guard_size;

This should be cleaned up with this patch and not an RFE for 10.

I don't know the NPTL issues well enough to be a reviewer for this, but 
I don't like something I don't know about distributed in many places.

>> 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.

I think the additional changes should also wait for 10.  I like the 
change to not have shadow pages for the compiler threads (not including 
AOT compiler threads, right?), but they also collide with work that I 
think Dan is going to do for windows.   We don't want the os::Posix 
functions in these files.


Also, the work that Dan and Ron did in this area didn't change the sizes 
and I'd want Dan to have time to comment on this.


> 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());
>>>> }

More information about the hotspot-runtime-dev mailing list