RFR(M): 8170655: [posix] Fix minimum stack size computations

Daniel D. Daugherty daniel.daugherty at oracle.com
Mon Dec 5 23:43:49 UTC 2016


On 12/5/16 4:27 PM, Daniel D. Daugherty wrote:
> On 12/3/16 11:17 AM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>>
>>
>> I would like to fix two issues of minimum stack size computation:
>>
>> http://cr.openjdk.java.net/~goetz/wr16/8170655-compilerGuardFix/webrev.01/ 
>>
>>
>> Please review and sponsor.
>
> I'm sponsoring the related bug:
>
> 8169373 Work around linux NPTL stack guard error
> https://bugs.openjdk.java.net/browse/JDK-8169373
>
> so I guess I should sponsor this one also... For obvious reasons, this
> fix will also need a "full RBT" run...
>
> > 
> http://cr.openjdk.java.net/~goetz/wr16/8170655-compilerGuardFix/webrev.01/
>
> src/cpu/ppc/vm/globals_ppc.hpp
>     No comments.
>
> src/cpu/x86/vm/globals_x86.hpp
>     No comments.
>
> src/os/posix/vm/os_posix.cpp
>     My brain read right past where you took out "MAX2" and changed the
>     math to "cur = cur + guard_size + shadow_size". I think I've been
>     staring at that particular line of code for way too long (in a couple
>     of different bug fixes)...
>
>     I think with your fix that Solaris specific block on
>     L1132 - L1148 can go away (see JDK-8161093). Hopefully
>     Coleen will chime in on just this part.
>
>     L1154:   _compiler_thread_min_stack_allowed = 
> _compiler_thread_min_stack_allowed +
>         Please add a comment above this line:
>
>         // Reminder: a compiler thread is-a Java thread.

And this comment above set_minimum_stack_sizes also needs work:

1108 // Check minimum allowable stack sizes for thread creation and to 
initialize
1109 // the java system classes, including StackOverflowError - depends 
on page
1110 // size.  Add two 4K pages for compiler2 recursion in main thread.
1111 // Add in 4*BytesPerWord 4K pages to account for VM stack during
1112 // class initialization depending on 32 or 64 bit VM.
1113 jint os::Posix::set_minimum_stack_sizes() {

since the code that did a lot of that comment is now gone...

Dan


>
> src/os/posix/vm/os_posix.hpp
>     L48:   // Set_minimum_stack_sizes() ...
>         Please change 'S' -> 's' since it is the function's name.
>
> src/os_cpu/aix_ppc/vm/os_aix_ppc.cpp
>     L539: // HotSpotguard pages is added later.
>         Typo: space before 'guard'
>
>     L542: size_t os::Posix::_vm_internal_thread_min_stack_allowed = 64 
> * K;
>         VM internal thread size lowered from 128K to 64K without any
>         changes to how _vm_internal_thread_min_stack_allowed is set.
>         Any particular reason for this change?
>
> src/os_cpu/bsd_x86/vm/os_bsd_x86.cpp
>     No comments.
>
> src/os_cpu/linux_aarch64/vm/os_linux_aarch64.cpp
>     No comments.
>
> src/os_cpu/linux_ppc/vm/os_linux_ppc.cpp
>     L542: size_t os::Posix::_vm_internal_thread_min_stack_allowed = 64 
> * K;
>         VM internal thread size lowered from 128K to 64K without any
>         changes to how _vm_internal_thread_min_stack_allowed is set.
>         Any particular reason for this change?
>
> src/os_cpu/linux_s390/vm/os_linux_s390.cpp
>     L478: size_t os::Posix::_compiler_thread_min_stack_allowed = (52 
> DEBUG_ONLY(+32)) * K;
>     L479: size_t os::Posix::_java_thread_min_stack_allowed = (32 
> DEBUG_ONLY(+8)) * K;
>         consistency - trying to put space around operators so...
>
>         '+32' -> '+ 32'
>         '+8' -> '+ 8'
>
>     L480: size_t os::Posix::_vm_internal_thread_min_stack_allowed = 32 
> * K;
>         VM internal thread size lowered from 128K to 32K without any
>         changes to how _vm_internal_thread_min_stack_allowed is set.
>         Any particular reason for this change?
>
> src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>     No comments.
>
> src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>     No comments.
>
> src/os_cpu/solaris_sparc/vm/os_solaris_sparc.cpp
>     No comments.
>
> src/os_cpu/solaris_x86/vm/os_solaris_x86.cpp
>     No comments.
>
> src/share/vm/runtime/os.hpp
>     L439:     java_thread,       // Java, CocdeCacheSweeper, 
> JVMTIAgent and Service threads.
>         Typo: 'CocdeCacheSweeper' -> 'CodeCacheSweeper'
>
>
> Dan
>
>>
>>
>>
>> This are the issues I excluded from the "8169373: Work around linux 
>> NPTL stack guard error."
>>
>>
>>
>> I wrote a lengthy explanation in the bug, trying to comment on what was
>>
>> said in the other thread.  I'll repeat it here, I think that's better 
>> for discussion.
>>
>> Dan, thanks for improving the text, I use the improved variant here:
>>
>>
>>
>> HotSpot has three cmd line options to set stack sizes (besides -Xss):
>>
>>   -XX:ThreadStackSize for threads executing Java code.
>>
>>   -XX:CompilerThreadStackSize for threads used by the JIT compilers.
>>
>>   -XX:VMThreadStackSize for threads executing VM internal tasks as gc.
>>
>>
>>
>> All these flags should not be set to a value that leads to a stack 
>> overflow before user code can be executed. As the VM executes a lot 
>> of code for initialization and also the JIT already compiles methods, 
>> considerable amounts of stack can be used during startup. We must try 
>> to avoid stack overflows before startup is complete as error handling 
>> might not be properly in place yet.
>>
>>
>>
>> Required minimum stack sizes depend on frame sizes and program 
>> execution paths. Frame sizes again depend on the C compiler used, the 
>> platform compiled for, and design decisions in interpreter, C1 and C2.
>>
>>
>>
>> Required stack sizes also depend on option settings, e.g. with JVM/TI 
>> enabled, frames can get bigger. With inlining increased JIT compilers 
>> might do more optimizations leading to deeper call chains, etc.
>>
>>
>>
>> While the minimum stack sizes should reflect differences in Platform 
>> and compiler, they must not, and cannot, cover all possible option 
>> settings.
>>
>>
>>
>> This change addresses two issues:
>>
>>
>>
>> 1. Fixed minimum stack size configuration
>>
>>
>>
>> Currently, the minimum Java thread size is given in a constant per 
>> os/cpu platform for each of the three stack kinds. This number 
>> includes the size required for guard pages.
>>
>>
>>
>> The guard pages are used for stack overflow detection. They make up 4 
>> zones on the stack: Red, Yellow, Reserved and Shadow pages. The Red, 
>> Yellow and Reserved pages are protected to detect stack overflows. 
>> The Shadow pages are just some extra space to allow methods that 
>> don't do a stack bang to execute.
>>
>>
>>
>> Unfortunately, the size required for guard pages is not fixed at 
>> compile time. It depends on the concrete system the VM is started on. 
>> Thus the minimum sizes given can be too small to hold the guard 
>> pages. This lead to errors in the past that were solved by 
>> introducing code that overruled the per-platform minimum stack size. 
>> This code nowadays is the MAX2() in os_posix.cpp:1114 and the SOLARIS 
>> special case further down. It uses the value (4 * BytesPerWord 
>> COMPILER2_PRESENT(+ 2)) * 4 * K) (os_posix.cpp:1117) as minimum 
>> required space for frames. Thereby it effectively overrules the 
>> minimum stack size settings given in the os/cpu constants, and there 
>> is currently no way to specify this size per platform.
>>
>>
>>
>> This change proposes to fix this issue by specifying the space needed 
>> for stack frames in the os/cpu constants. During startup, this number 
>> is increased by the space required for the guard pages. Thus, this 
>> takes into account the page size of the concrete system the VM runs 
>> on, and also eventual changes to the guard pages by the flags 
>> StackRed/Yellow/Reserved/Shadow/Pages. This gives the opportunity to 
>> reduce the minimum stack sizes on systems with small pages.
>>
>>
>>
>> Minimum stack size configuration is more simple with this change and 
>> valid for systems with any page size.
>>
>>
>>
>> 2. Stack guard pages not considered for compiler thread stacks
>>
>>
>>
>> Compiler threads are Java threads. The C++ class CompilerThread is a 
>> subclass of JavaThread. When a compiler thread is started, 
>> JavaThread::run() is executed which protects the red, yellow and 
>> reserved pages.
>>
>>
>>
>> Since 8140520 the minimum stack sizes for Compiler and VM internal 
>> threads no longer include the space for the guard pages. This is 
>> correct for the VM internal threads, but not for the Compiler thread.
>>
>>
>>
>> For the HotSpot C1 and C2 compilers it would be fine to reserve space 
>> for the Red/Yellow/Reserved pages, as they don't stack bang with the 
>> shadow page size. But since introducing JVMCI, compilers written in 
>> Java can be running on the compiler threads. Therefore the shadow 
>> pages are needed, too.
>>
>>
>>
>> As for the Java thread, this change uses a os/cpu constant for the 
>> required minimum space for compiler frames and then adds the zone 
>> sizes to the minimum stack sizes during startup.
>>
>>
>>
>> New sizing:
>>
>>
>>
>> The constants of the os/cpu minimum thread sizes are reduced by the 
>> default guard page sizes and then verified by starting the VM to 
>> assure the stack still suffices to get through startup. Hotspot jtreg 
>> tests are passing. The overall sizes required (after adding guard 
>> pages) on the systems I have available get a bit smaller. In most 
>> cases the sizes even suffice to run simple programs as SpecJvm98. The 
>> table below gives the systems I tested and the required sizes 
>> reported when started with too small stacks.
>>
>>
>>
>> <pre>
>>
>>   Thread kind:       Java      Compiler  VM
>>
>>                      old new   old new   old new
>>
>>
>>
>> bsd x86_64    dbg: 240 232    64  64    64  64
>>
>>                 opt: 240 232    64  64    64  64
>>
>>
>>
>> linux x86_64  dbg: 240 144    64 152    64  64
>>
>>                 opt: 232 136    64 144    64  64
>>
>>
>>
>> solaris sparc dbg:
>>
>>                 opt: 240 192   128 128   128 128
>>
>>
>>
>> aix ppc64     dbg: 512 512   384 512   128 128
>>
>>                 opt: 512 512   384 512   128 128
>>
>>   linux ppc64   dbg: 512 384   128 384   128  64
>>
>>                 opt: 512 384   128 384   128  64
>>
>>   linux ppc64le dbg: 512 384   128 384   128  64
>>
>>                 opt: 512 384   128 384   128  64
>>
>>
>>
>> linux s390    dbg: 236 140   128 184   128  32
>>
>>                 opt: 236 124   128 144   128  32
>>
>>   </pre>
>>
>>
>>
>>
>>
>>
>
>



More information about the hotspot-runtime-dev mailing list