RFR: 8013398: Adjust number of stack guard pages on systems with large memory page size

Tue Apr 30 14:48:52 PDT 2013

Hi Volker,
Besides changing the parameters to be in terms of size rather than 
pages, the os_linux.cpp change is compatible with what Vlad has done.  
This part of the change fixes the problem with having too much reserved 
space on the stack.   Changing these parameter names is going to be 
problematic because we do have customers that we've given 
StackShadowPages as an option and they won't be happy if it goes away.

I think we should integrate Vlad's fix and think about your changes or 
how we can make these work compatibly.

Vlad - Should the default page size be changed to 4K though, as Volker 
suggested in his code review?

Thanks,
Coleen

On 04/30/2013 12:37 PM, Volker Simonis wrote:
> Hi Coleen, Vladimir,
>
> here you can find a first version of what I was thinking about:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8013398.v1/
>
> This may still need some polishing and I haven’t done extensive testing with it.
> I just wanted to give you a quick idea of my proposal as I will be out
> of office until Monday.
>
> Thank you and best regards,
> Volker
>
>
>
> On Mon, Apr 29, 2013 at 7:36 PM, Coleen Phillimore
> <coleen.phillimore at oracle.com> wrote:
>> Hi Volker,
>>
>> When looking at Vlad's change, I was thinking along these lines.
>> StackYellowPages, RedPages and ShadowPages would be better specified as a
>> size in bytes rather than page size to account for the differences in page
>> sizes on different platforms.
>>
>> The stack banging code is done in increments of "page_size" though, and the
>> protection code has to be done on page size boundaries.   So there has to be
>> some platform specific rounding to get the number of pages.   I think the
>> code change that Vlad sent is the minimally disruptive way to do this.
>> Adding new options for StackShadowSize and all that will just confuse
>> people, I think.   We tell people to use different StackShadowPages all the
>> time and it's even in our documentation.
>>
>> I guess we'll have to see your patch and it would be good to have your code
>> to be in open.
>>
>> Thanks,
>> Coleen
>>
>>
>> On 04/29/2013 12:55 PM, Volker Simonis wrote:
>>> On Mon, Apr 29, 2013 at 5:38 PM, Vladimir Danushevsky
>>> <vladimir.danushevsky at oracle.com> wrote:
>>>> Hi Volker,
>>>>
>>>> Thanks for the suggestion.
>>>>
>>>> If I understand your approach correctly you suggest providing a dual way
>>>> to specify guard area sizes (ether through number of pages or through an
>>>> absolute size in KB) and have the VM to correlate between the two: Size
>>>> value will dominate unless explicitly overwritten by the Page value.
>>>> Is that correct?
>>>>
>>> Yes, that's correct. The *Page values should become obsolete although
>>> I think there's currently no mechanism for that in OpenJDK.
>>>
>>> Internally, i.e. in globals_<ARCH>.hpp, we could define the values
>>> independently of the actual system page size with the *Size flags.
>>>
>>>> Will your approach eventually get same amount of allocated guard area as
>>>> the patch suggested earlier would?
>>> I think more or less yes:) It would depend on the settings of the
>>> *Size flags of course.
>>>
>>> E.g. currently on AMD64 we have:
>>>
>>> define_pd_global(intx, StackYellowPages, 2);
>>> define_pd_global(intx, StackRedPages, 1);
>>> define_pd_global(intx, StackShadowPages, NOT_WIN64(20) WIN64_ONLY(6)
>>> DEBUG_ONLY(+2));
>>>
>>> With your fix and a page size of 64K we would get:
>>>
>>> StackYellowPages = 1;
>>> StackRedPages = 1;
>>> StackShadowPages = round_to((StackShadowPages * 8K), 64K) / 64K =
>>> round_to(20 * 8K, 64K) / 64K = 3
>>>
>>> and:
>>>
>>> min_staqck =
>>> (StackYellowPages+StackRedPages+StackShadowPages) * Linux::page_size() +
>>> (2*BytesPerWord COMPILER2_PRESENT(+1)) * Linux::vm_default_page_size()
>>> =
>>> (1 + 1 + 3) * 64K +
>>> (16 + 1) * 8K
>>> =
>>> 5 * 64K + 17 * 8K
>>> =
>>> 456K
>>>
>>> With our proposed fix we would get:
>>>
>>> define_pd_global(intx, StackYellowSize, 8K);
>>> define_pd_global(intx, StackRedSize, 4K);
>>> define_pd_global(intx, StackShadowSize, NOT_WIN64(80K) WIN64_ONLY(24K)
>>> DEBUG_ONLY(+8K));
>>>
>>> this would result in:
>>>
>>> StackYellowPages = 1;
>>> StackRedPages = 1;
>>> StackShadowPages = 2
>>>
>>> and:
>>>
>>> min_staqck =
>>> (StackYellowPages+StackRedPages+StackShadowPages) * Linux::page_size() +
>>> (2*BytesPerWord COMPILER2_PRESENT(+1)) * Linux::vm_default_page_size()
>>> =
>>> (1 + 1 + 2) * 64K +
>>> (16 + 1) * 8K
>>> =
>>> 4 * 64K + 17 * 8K
>>> =
>>> 392
>>>
>>>> If an intent is to make the adjustments global (i.e. independent of the
>>>> OS) should we copy those to the arguments.cpp instead of os_<OS
>>>> name>.cpp?However  I'm not positive whether Windows, as an example, does
>>>> need a guard area adjustment (but I might be wrong here).
>>>>
>>> I think the adjustment should stay in the platform dependent code.
>>>
>>>> Please post the webrev with the approach you suggested. It should be
>>>> viewed as an alternative.
>>> I'll prepare it.
>>>
>>> Just one question - why did you choose:
>>>
>>> _vm_default_page_size = 8K
>>>
>>> Shouldn't this really be 4K?
>>>
>>>> Thanks,
>>>> Vlad
>>>>
>>>>
>>>> On Apr 29, 2013, at 11:03 AM, Volker Simonis wrote:
>>>>
>>>>> Hi Vlad,
>>>>>
>>>>> thanks for addressing this issue. We have this problem as well since
>>>>> long time and we would like to propose a more general solution:
>>>>>
>>>>> Introduce three new platform dependant product flags:
>>>>>
>>>>> StackYellowSize
>>>>> StackRedSize
>>>>> StackShadowSize
>>>>>
>>>>> which are to be used to specify the concrete size of the corresponding
>>>>> stack regions. These sizes would be rounded up to the next multiple of
>>>>> the current OS page size (or more precisely the next multiple of the
>>>>> current OS stack page size, because there are systems which have
>>>>> different page sizes for stacks than for heap).
>>>>>
>>>>> This would allow us to make simple, (i.e. not platform dependent)
>>>>> product flags out of:
>>>>>
>>>>> StackYellowPages
>>>>> StackRedPages
>>>>> StackShadowPages
>>>>>
>>>>> and set their default value to -1.
>>>>>
>>>>> At VM start-up, before argument parsing, we will compute the values
>>>>> for StackYellowPages, StackRedPages, StackShadowPages from
>>>>> StackYellowSize, StackRedSize, StackShadowSize and the actual OS page
>>>>> size. Later on, during argument parsing, if one of these flags is
>>>>> given on the command line, either part of these value pairs may be
>>>>> adjusted in order to be consistent with its counterpart.
>>>>>
>>>>> The drawback of this approach is that you get three more options but
>>>>> from our point of view this is outweighed by the fact that with this
>>>>> approach you 'fix' all the places where you currently waste a lot of
>>>>> memory if the VM is running on a system with a default page size
>>>>> bigger than 4K.
>>>>>
>>>>> Notice that we use this approach since years in our commercial VM and
>>>>> only haven't integrated it into our OpenJDK PowerPC/AIX Port because
>>>>> we wanted to minimize the amount of shared code changes. We would be
>>>>> happy however to contribute it now that the discussion on this problem
>>>>> has popped up publicly on the mailing list. So if you agree, I could
>>>>> post a webrev with the proposed changes.
>>>>>
>>>>> Regards,
>>>>> Volker
>>>>>
>>>>> On Sat, Apr 27, 2013 at 2:34 AM, Vladimir Danushevsky
>>>>> <vladimir.danushevsky at oracle.com> wrote:
>>>>>> Please review the following change:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~vladidan/8013398/webrev.00/
>>>>>>
>>>>>> for
>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8013398
>>>>>>
>>>>>> On a non-typical Linux system configuration with large page size (e.g.
>>>>>> 64KB) significant amount of virtual memory is wasted to multiple guard pages
>>>>>> when in fact that can be reduced to a single page per Red/Yellow/Shadow
>>>>>> categories.
>>>>>> In some edge cases a minimal stack requirement is calculated higher
>>>>>> than actual stack size specified by ThreadStackSize parameter causing VM to
>>>>>> abort unless stack size is overwritten via -Xss option.
>>>>>>
>>>>>> A patch currently being suggested adjusts amount of guard pages to a
>>>>>> single memory page each along with minimal stack size when memory page size
>>>>>> exceed 8KB unless explicitly overwritten in command line.
>>>>>>
>>>>>> Currently we are aware of the problem on some Linux platforms,
>>>>>> therefore the change is specific to that OS only (Solaris carries a similar
>>>>>> approach already).
>>>>>>
>>>>>> Shall the description string of StackRedPage, StackYellowPages and
>>>>>> StackShadow pages be modified to reflect the values could be scaled down? If
>>>>>> so the comment needs to indicate the change affects Linux and Solaris only.
>>>>>> Any comments welcome here.
>>>>>>
>>>>>> Thanks,
>>>>>> Vlad
>>