Request for review: 6978641 ( was Re: Request for approval: 6929067: Stack guard pages should be removed when thread is detached)

[David Holmes]

Here's the official OpenJKDK webrev for this:

http://cr.openjdk.java.net/~dholmes/6978641/

If I can add you both as reviewers that would be great.

Coleen can you send me details on which release/build I should use in 
the CR and also if there is any special "magic" I need to submit via JPRT.

Thanks,
David

Coleen Phillimore said the following on 08/20/10 23:15:
> 
> This change looks fine to me and yes, thanks for pointing out that I did 
> add the test.
> 
> Coleen
> 
> Andrew Haley wrote:
>> On 08/19/2010 04:49 AM, David Holmes wrote:
>>
>>  
>>> diff -r f707b4c0c36f src/os/linux/vm/os_linux.cpp
>>> --- a/src/os/linux/vm/os_linux.cpp
>>> +++ b/src/os/linux/vm/os_linux.cpp
>>> @@ -2628,10 +2628,12 @@ get_stack_bounds(uintptr_t *bottom, uint
>>>  // where we're going to put our guard pages, truncate the mapping at
>>>  // that point by munmap()ping it.  This ensures that when we later
>>>  // munmap() the guard pages we don't leave a hole in the stack
>>> -// mapping.
>>> +// mapping. This only affects the main/initial thread, but guard
>>> +// against future OS changes
>>>  bool os::create_stack_guard_pages(char* addr, size_t size) {
>>>    uintptr_t stack_extent, stack_base;
>>> -  if (get_stack_bounds(&stack_extent, &stack_base)) {
>>> +  if (NOT_DEBUG(os::Linux::is_initial_thread() &&) 
>>> get_stack_bounds(&stack_extent, &stack_base)) {
>>> +      assert(os::Linux::is_initial_thread(), "growable stack in 
>>> non-initial thread");
>>>      if (stack_extent < (uintptr_t)addr)
>>>        ::munmap((void*)stack_extent, (uintptr_t)addr - stack_extent);
>>>    }
>>>     
>>
>> Is stack_extent initialized here in NOT_DEBUG mode?
>>
>>  
>>> @@ -2640,10 +2642,13 @@ bool os::create_stack_guard_pages(char*
>>>  }
>>>
>>>  // If this is a growable mapping, remove the guard pages entirely by
>>> -// munmap()ping them.  If not, just call uncommit_memory().
>>> +// munmap()ping them.  If not, just call uncommit_memory(). This only
>>> +// affects the main/initial thread, but guard against future OS changes
>>>  bool os::remove_stack_guard_pages(char* addr, size_t size) {
>>>    uintptr_t stack_extent, stack_base;
>>> -  if (get_stack_bounds(&stack_extent, &stack_base)) {
>>> +  if (NOT_DEBUG(os::Linux::is_initial_thread() &&) 
>>> get_stack_bounds(&stack_extent, &stack_base)) {
>>> +      assert(os::Linux::is_initial_thread(), "growable stack in 
>>> non-initial thread");
>>> +
>>>      return ::munmap(addr, size) == 0;
>>>    }     
>>
>> This is basically a good compromise, I think.
>>
>> Andrew.
>>