Chris, Looks good to me.
-Dmitry On 2017-03-17 10:31, Chris Plummer wrote: > I should have been more clear. I need one more "reviewer", not another > review from David. > > thanks, > > Chris > > On 3/17/17 12:30 AM, Chris Plummer wrote: >> Thanks for the review, David. >> >> I could still use one more review. Here's an updated webrev. >> >> http://cr.openjdk.java.net/~cjplummer/8175342/webrev.01/webrev.jdk >> >> cheers, >> >> Chris >> >> On 3/15/17 10:14 PM, David Holmes wrote: >>> Hi Chris, >>> >>> On 16/03/2017 2:57 PM, Chris Plummer wrote: >>>> Hello, >>>> >>>> Please review the following: >>>> >>>> https://bugs.openjdk.java.net/browse/JDK-8175342 >>>> http://cr.openjdk.java.net/~cjplummer/8175342/webrev.00/webrev.jdk >>> >>> I think you want to disable the guardpage always, not just when a >>> specific stack size is requested. You might not miss 64KB in 8MB but >>> logically the guard page is never needed. >>> >>> Thanks, >>> David >>> ----- >>> >>>> The assert is somewhat misleading, although it did properly detect a >>>> "too small" stack issue. The test was run with -Xss256k on a system >>>> with >>>> 64k pages. On this system 256k is suppose to be the smallest allowed >>>> stack size, so -Xss256k should work. The thread that the assert happens >>>> on is the main thread created by ContinueInNewThread0(). By default >>>> pthread gives new threads a guard page the size of an OS page. pthreads >>>> is suppose to add additional stack space for the guard page, but it >>>> doesn't. Later we call current_stack_region(), which among other >>>> things, >>>> computes the size of the stack. It has the following code to deal with >>>> the pthread guard page bug: >>>> >>>> // Work around NPTL stack guard error. >>>> size_t guard_size = 0; >>>> rslt = pthread_attr_getguardsize(&attr, &guard_size); >>>> *bottom += guard_size; >>>> *size -= guard_size; >>>> >>>> So the net effect is hotspot treats the stack as only being 192k, not >>>> 256k. However, in terms of usable stack space, hotspot then also >>>> subtracts the red, yellow, and shadow zones. Each of these is one OS >>>> page. So that subtracts another 192k, leaving us with 0k. The assert is >>>> a by product of this. >>>> >>>> It turns out this pthread guard page problem is already fixed for all >>>> java threads except the main thread. We do the following in >>>> os::create_thread(): >>>> >>>> pthread_attr_setguardsize(&attr, >>>> os::Linux::default_guard_size(thr_type)); >>>> >>>> For java threads, os::Linux::default_guard_size() returns 0, so the >>>> above code removes the guard page for java threads. The fix I'm >>>> proposing for the main thread does the same. >>>> >>>> Tested by running the test in question dozens of times on all supported >>>> platforms. Also ran most tests we do for nightlies except for longer >>>> running ones. >>>> >>>> thanks, >>>> >>>> Chris >> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
