RFR(M): 8170655: [posix] Fix minimum stack size computations
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Dec 14 15:04:56 UTC 2016
Thanks for the confirmation.
Dan
On 12/14/16 12:09 AM, Lindenmaier, Goetz wrote:
> Hi Dan,
>
> Yes, that's the final versions including the errno->error fix.
>
> Best regards,
> Goetz.
>
>
>
>> -----Original Message-----
>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>> Sent: Dienstag, 13. Dezember 2016 17:49
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; David Holmes
>> <david.holmes at oracle.com>; Coleen Phillimore
>> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size computations
>>
>> Goetz,
>>
>> Does this latest webrev include the last of the code review changes that
>> you were holding onto?
>>
>> Dan
>>
>>
>> On 12/13/16 9:22 AM, Lindenmaier, Goetz wrote:
>>> I messed up the link, the rebased webrev of 8170655 is
>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>> compilerGuardFix/webrev.03/
>>> Best regards,
>>> Goetz.
>>>
>>> (I'll resend the mail on the other mail thread anyways ...)
>>>
>>>> -----Original Message-----
>>>> From: Lindenmaier, Goetz
>>>> Sent: Dienstag, 13. Dezember 2016 17:16
>>>> To: 'daniel.daugherty at oracle.com' <daniel.daugherty at oracle.com>; David
>>>> Holmes <david.holmes at oracle.com>; Coleen Phillimore
>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
>>>> Subject: RE: RFR(M): 8170655: [posix] Fix minimum stack size computations
>>>>
>>>> Hi,
>>>>
>>>> I rebased 8170655 to the recent changes in hotspot:
>>>> http://cr.openjdk.java.net/~goetz/wr16/8169373-ppc-stackFix/webrev.09/
>>>>
>>>> The patch is still in our test patch queue for the nightly tests, and there
>>>> were no failures I can attribute to this change.
>>>>
>>>> Best regards,
>>>> Goetz.
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>>>> Sent: Dienstag, 13. Dezember 2016 16:45
>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; David Holmes
>>>>> <david.holmes at oracle.com>; Coleen Phillimore
>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-dev at openjdk.java.net
>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size computations
>>>>>
>>>>> The current set of RBT test results looks bad. I suspect a mismatch
>>>>> between current test suite version(s) and the baseline that I used.
>>>>> I created the repo for 8169373 back on 2016.12.02 and the last
>>>>> changeset in hotspot at that time was dated 2016.11.30... so the
>>>>> baseline on which I applied your changes is almost 2 weeks out of
>>>>> date (pre-dates the latest Jigsaw refresh)...
>>>>>
>>>>> > Should I rebase the changes to the latest repo version?
>>>>>
>>>>> If you don't mind rebasing both patches (8169373 and 8170655) to the
>>>>> current JDK9-hs, that would be appreciated. Thanks!
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>>
>>>>> On 12/13/16 8:17 AM, Daniel D. Daugherty wrote:
>>>>>> Both 8169373 and 8170655 are bug fixes so they do not have to be
>>>>>> in by the extended Feature Complete deadline. If they did, I would
>>>>>> be going through more Tums than I already am... :-)
>>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/13/16 8:02 AM, Lindenmaier, Goetz wrote:
>>>>>>> Hi Dan,
>>>>>>>
>>>>>>> that sounds bad ... thanks for your update! Yes, I also have the
>>>>>>> Feature
>>>>>>> Complete deadline in mind, that's why I would like to see the changes
>>>>>>> committed :) But obviously I currently can't help.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Goetz.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>>>>>>> Sent: Dienstag, 13. Dezember 2016 15:56
>>>>>>>> To: David Holmes <david.holmes at oracle.com>; Lindenmaier, Goetz
>>>>>>>> <goetz.lindenmaier at sap.com>; Coleen Phillimore
>>>>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-
>> dev at openjdk.java.net
>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>> computations
>>>>>>>>
>>>>>>>> Hi Goetz,
>>>>>>>>
>>>>>>>> JDK9-hs is still closed for the run up to extended Feature Complete.
>>>>>>>> Should be open again sometime soon.
>>>>>>>>
>>>>>>>> I kicked off a "full RBT" run on 8169373 last Monday. It ran for four
>>>>>>>> days before it collided with a maintenance window on the Aurora DB.
>>>>>>>> I kicked off a "full RBT" run on 8170655 last Tuesday. It ran for three
>>>>>>>> days before it collided with the same brick wall.
>>>>>>>>
>>>>>>>> Neither RBT run is complete; in fact there are entire platforms with
>>>>>>>> zero results so they are pretty much useless. Normally RBT runs don't
>>>>>>>> take more than a couple of days, but our internal test systems are a
>>>>>>>> bit swamped due to the upcoming extended Feature Complete
>> deadline.
>>>>>>>> I did start another "full RBT" run on 8169373 on Sunday, but it is
>>>>>>>> not complete either. I was hoping to have all the testing done by
>>>>>>>> the time JDK9-hs reopened, but it looks like that is not going to
>>>>>>>> happen.
>>>>>>>>
>>>>>>>> Dan
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/13/16 1:41 AM, David Holmes wrote:
>>>>>>>>> On 13/12/2016 6:16 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>> Hi Dan,
>>>>>>>>>>
>>>>>>>>>> what's the status of my changes? I think hs is open again, at least
>>>>>>>>>> I see changes popping up ... just a gentle reminder :)
>>>>>>>>> Actually it is closed (again?) other than for selective changes as we
>>>>>>>>> had to pull down the big jigsaw update from dev, and merge in with
>>>> AOT
>>>>>>>>> push, and a couple of other approved changes, and need things to
>>>>>>>>> stabilise so we can push up to dev.
>>>>>>>>>
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> Should I rebase the changes to the latest repo version?
>>>>>>>>>>
>>>>>>>>>> Best regards,
>>>>>>>>>> Goetz.
>>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com]
>>>>>>>>>>> Sent: Dienstag, 6. Dezember 2016 22:15
>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Coleen
>>>>>>>>>>> Phillimore
>>>>>>>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-
>>>>> dev at openjdk.java.net
>>>>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>>>>> computations
>>>>>>>>>>>
>>>>>>>>>>> On 12/6/16 1:31 PM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>> Hi Dan,
>>>>>>>>>>>>
>>>>>>>>>>>> I fixed the comment. I will post a new webrev once the other
>>>>>>>>>>>> change is pushed so that I can make a real change.
>>>>>>>>>>> I'll resync my repo to the webrev.02 version and restart my
>>>>>>>>>>> testing on this fix. I'm hoping we have only comment changes
>>>>>>>>>>> after the webrev.02 version...
>>>>>>>>>>>
>>>>>>>>>>> Also gotta check the test results on 8169373.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>> Meta question: Now _java_thread_min_stack_allowed is
>>>>>>>>>>>>> specific to an
>>>>>>>>>>>>> OS/CPU platform (as are the other two). That OS/CPU
>>>>>>>>>>>>> platform
>>>>>>>>>>>>> enumeration
>>>>>>>>>>>>> doesn't take into account whether C1 or C2 is in play. We
>>>>>>>>>>>>> used to add
>>>>>>>>>>>>> just a little more space via the COMPILER2_PRESENT()
>> macro,
>>>>>>>>>>>>> but now
>>>>>>>>>>> we
>>>>>>>>>>>>> don't. Yes, I know that you've done a bunch of testing and
>>>>>>>>>>>>> I'm running
>>>>>>>>>>>>> our usual batch of tests also, but this subtle change is
>>>>>>>>>>>>> bothering
>>>>>>>>>>>> Yep, I could have made sizing with C1, then C2, then
>>>>>>>>>>>> TieredCompilation,
>>>>>>>>>>>> but I don't think that would make a change.
>>>>>>>>>>>> This extra size was meant for the compiler thread. 83251780
>> moved
>>>>>>>>>>>> it to the
>>>>>>>>>>>> Java threads. I think it was there because C2 used to have
>>>>>>>>>>>> recursive
>>>>>>>>>>> algorithms.
>>>>>>>>>>>> But it only would have had an effect in rare situations because the
>>>>>>>>>>>> other MAX
>>>>>>>>>>>> should mostly be used on systems with large pages, where the
>>>>>>>>>>>> alignement
>>>>>>>>>>>> would bring more than these 4K. The compiler gained the
>>>> alignment
>>>>>>>>>>>> of the
>>>>>>>>>>>> vm_default_page_size() region plus the space gained by bigger
>>>>>>>>>>> StackShadowPages
>>>>>>>>>>>> which were huge at some point (20+ pages * 64K ...). Remember
>> the
>>>>>>>>>>> compiler does
>>>>>>>>>>>> no stack banging and thus could use the shadow region.
>>>>>>>>>>>>
>>>>>>>>>>>> The original code looked like this:
>>>>>>>>>>>>
>>>>>>>>>>>>
>> http://hg.openjdk.java.net/jdk8/jdk8/hotspot/file/87ee5ee27509/src/os/linux/v
>>>>>>>>>>> m/os_linux.cpp
>>>>>>>>>>>> os::Linux::min_stack_allowed =
>>>>>>>>>>>> MAX2(os::Linux::min_stack_allowed,
>>>>>>>>>>>> (size_t)(StackYellowPages+StackRedPages+StackShadowPages) *
>>>>>>>>>>> Linux::page_size() +
>>>>>>>>>>>> (2*BytesPerWord COMPILER2_PRESENT(+1)) *
>>>>>>>>>>> Linux::vm_default_page_size());
>>>>>>>>>>>
>>>>>>>>>>> I'm good with this explanation. Coleen?
>>>>>>>>>>>
>>>>>>>>>>> Dan
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> Best regards,
>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>
>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>> From: Daniel D. Daugherty
>> [mailto:daniel.daugherty at oracle.com]
>>>>>>>>>>>>> Sent: Tuesday, December 06, 2016 8:32 PM
>>>>>>>>>>>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Coleen
>>>>>>>> Phillimore
>>>>>>>>>>>>> <coleen.phillimore at oracle.com>; hotspot-runtime-
>>>>>>>> dev at openjdk.java.net
>>>>>>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>>>>>>> computations
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 12/6/16 2:35 AM, Lindenmaier, Goetz wrote:
>>>>>>>>>>>>>> Hi Coleen,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> thanks for looking at this change!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> New webrev:
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>> compilerGuardFix/webrev.02/
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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
>>>>>>>>>>>>> L1112: // It dependes on word size, platform calling
>>>>>>>>>>>>> conventions, C
>>>>>>>>>>>>> frame layout and
>>>>>>>>>>>>> Typo: 'dependes' -> 'depends'
>>>>>>>>>>>>>
>>>>>>>>>>>>> If you put "It" at the end of the sentence on the
>>>>>>>>>>>>> previous
>>>>>>>>>>>>> line, then the formatting will look better. :-)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Meta question: Now _java_thread_min_stack_allowed is
>>>>>>>>>>>>> specific to an
>>>>>>>>>>>>> OS/CPU platform (as are the other two). That OS/CPU
>>>>>>>>>>>>> platform
>>>>>>>>>>>>> enumeration
>>>>>>>>>>>>> doesn't take into account whether C1 or C2 is in play. We
>>>>>>>>>>>>> used to add
>>>>>>>>>>>>> just a little more space via the COMPILER2_PRESENT()
>> macro,
>>>>>>>>>>>>> but now
>>>>>>>>>>> we
>>>>>>>>>>>>> don't. Yes, I know that you've done a bunch of testing and
>>>>>>>>>>>>> I'm running
>>>>>>>>>>>>> our usual batch of tests also, but this subtle change is
>>>>>>>>>>>>> bothering
>>>>>>>>>>>>> me...
>>>>>>>>>>>>>
>>>>>>>>>>>>> src/os/posix/vm/os_posix.hpp
>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>
>>>>>>>>>>>>> src/os_cpu/aix_ppc/vm/os_aix_ppc.cpp
>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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
>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>
>>>>>>>>>>>>> src/os_cpu/linux_s390/vm/os_linux_s390.cpp
>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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
>>>>>>>>>>>>> No comments.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Dan
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> See my comments inline.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>>>>>>>> bounces at openjdk.java.net] On Behalf Of Coleen Phillimore
>>>>>>>>>>>>>>> Sent: Dienstag, 6. Dezember 2016 01:00
>>>>>>>>>>>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>>>>>>>>>>>> Subject: Re: RFR(M): 8170655: [posix] Fix minimum stack size
>>>>>>>>>>>>> computations
>>>>>>>>>>>>>>> On 12/5/16 6: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...
>>>>>>>>>>>>>>> Thank you, Dan for sponsoring this.
>>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>> Yes, I think with refactoring, this is dead code. I think you
>>>>>>>>>>>>>>> should
>>>>>>>>>>>>>>> remove it, and make 8161093 a duplicate so it can be closed
>>>> with
>>>>>>>>>>>>>>> this fix.
>>>>>>>>>>>>>> I removed the code and closed the bug as duplicate.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The changes in this file (adding the guard_size and
>> shadow_size)
>>>>>>>>>>>>>>> are a
>>>>>>>>>>>>>>> lot cleaner than the max of some random number of pages
>>>>> someone
>>>>>>>>>>>>>>> calculated a long time ago.
>>>>>>>>>>>>>> Thanks!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> There's a comment above this function that should be
>>>>>>>>>>>>>>> rewritten also
>>>>>>>>>>>>> though:
>>>>>>>>>>>>>>> // Check minimum allowable stack sizes for thread creation
>>>>>>>>>>>>>>> and to
>>>>>>>>>>>>> initialize
>>>>>>>>>>>>>>> // the java system classes, including StackOverflowError -
>>>>>>>>>>>>>>> depends on
>>>>>>>>>>>>> page
>>>>>>>>>>>>>>> // size. Add two 4K pages for compiler2 recursion in main
>>>>>>>>>>>>>>> thread.
>>>>>>>>>>>>>>> // Add in 4*BytesPerWord 4K pages to account for VM stack
>>>>> during
>>>>>>>>>>>>>>> // class initialization depending on 32 or 64 bit VM.
>>>>>>>>>>>>>> Dan saw this too. I wrote a summary of the text in the bug.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>>>>
>> compilerGuardFix/webrev.01/src/os_cpu/linux_sparc/vm/os_linux_sparc.cpp
>>>>>>>>>>>>> .u
>>>>>>>>>>>>>>> diff.html
>>>>>>>>>>>>>>> This seems small. I don't think it should be reduced because I
>>>>>>>>>>>>>>> don't
>>>>>>>>>>>>>>> think we have a lot of testing for this platform.
>>>>>>>>>>>>>> I changed it to 64K. I can't test on this platform, so this
>>>>>>>>>>>>>> probably really is
>>>>>>>>>>>>> better.
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>>>>
>> compilerGuardFix/webrev.01/src/os_cpu/linux_x86/vm/os_linux_x86.cpp.ud
>>>>>>>>>>>>> iff.
>>>>>>>>>>>>>>> html
>>>>>>>>>>>>>>> Why is DEBUG_ONLY( + 4) removed? We do run native JVM
>>>> code
>>>>>>>> with
>>>>>>>>>>>>>>> debugging on in Java threads also, although the +4 seems
>>>>>>>>>>>>>>> arbitrary.
>>>>>>>>>>>>>> I checked startup and ran jvm98 and did not find any
>>>>>>>>>>>>>> differences in
>>>>>>>>>>>>> slowdebug/opt
>>>>>>>>>>>>>> variants. So I thought I should get rid of this. Also, this kind
>>>>>>>>>>>>>> of accounts for
>>>>>>>>>>>>> the
>>>>>>>>>>>>>> extra space of shadow pages in dbg builds, which now is no
>> more
>>>>>>>>>>>>>> included
>>>>>>>>>>>>> in that
>>>>>>>>>>>>>> number.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> If you're removing the concept of DEBUG_ONLY to these sizes,
>> it
>>>>>>>>>>>>>>> still
>>>>>>>>>>>>>>> remains in this change:
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/wr16/8170655-
>>>>>>>>>>>>>>>
>> compilerGuardFix/webrev.01/src/os_cpu/linux_s390/vm/os_linux_s390.cpp.
>>>>>>>>>>>>> udi
>>>>>>>>>>>>>>> ff.html
>>>>>>>>>>>>>> On s390, I quickly ran into stack overflows with the debug build.
>>>>>>>>>>>>>> Therefore
>>>>>>>>>>>>>> I increased the sizes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I assume that all these minimum stack sizes have been tested
>> in
>>>>>>>>>>>>>>> your lab.
>>>>>>>>>>>>>> Yes, the change is running with our nighly tests on the platforms
>>>>>>>>>>>>>> we have
>>>>>>>>>>>>> available.
>>>>>>>>>>>>>> This includes jck tests and hotspot jtreg tests.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> This seems like a good change but I hope this is the last of
>>>>>>>>>>>>>>> these until
>>>>>>>>>>>>>>> JDK10 opens.
>>>>>>>>>>>>>> :) Yes, stack changes kind of got my special hobby ... I
>>>>>>>>>>>>>> still have
>>>>>>>>>>>>> implementing
>>>>>>>>>>>>>> StackReservedPages support on s390 on my list, but that should
>> be
>>>>>>>>>>>>> platform-only.
>>>>>>>>>>>>>> Also I'm not sure whether I'll find time for that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>> Goetz.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>> Coleen
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> 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