RFR(M): 8170655: [posix] Fix minimum stack size computations
Lindenmaier, Goetz
goetz.lindenmaier at sap.com
Wed Dec 14 07:09:25 UTC 2016
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