RFR(L): 8032410: compiler/uncommontrap/TestStackBangRbp.java times out on Solaris-Sparc V9

[Vladimir Kozlov]

On 3/14/14 6:42 AM, Roland Westrelin wrote:
>
> Hi Vladimir,
>
>>>> With +1 you will touch yellow page because it is inclusive if I read it
>>>> right:
>>>>
>>>>     while (bang_offset <= bang_end) {
>>>>
>>>> Can you test with StackShadowPages=1?
>>>
>>> Are you suggesting I run with StackShadowPages=1 to check if:
>>>
>>> 137     int bang_end = (StackShadowPages+1)*page_size;
>>>
>>> is ok?
>>
>> Yes, because you may be creating hole in banging if compiled code called from interpreter. It should be consistent with AbstractInterpreterGenerator::bang_stack_shadow_pages().
>
> The VM doesn’t like running with StackShadowPages=1. Every crash that I have running the regression tests with (StackShadowPages+1)*page_size, I can reproduce with StackShadowPages*page_size.
>
> The bad thing that could happen would be for stack banging to hit the red zone or even past the red zone, right? I tried to write a test case that would cause this to happen and my conclusion is that it’s not possible as long as we have more than one yellow page. Let’s say StackShadowPages=1. Worst case is that sp points somewhere in the shadow page when we enter a compiled method. Then we bang sp + 2 pages which hits in the second yellow page.

You are right, I agree that we can touch the yellow page. May be we 
should change low limit for StackYellowPages in 
Arguments::check_stack_pages() (current default values are >=2).

>
> This said, I don’t understand why 8026775 changed, in macroAssembler_<arch>.cpp:
>
> -  for (int i = 0; i< StackShadowPages-1; i++) {
>
> to
>
> +  for (int i = 1; i <= StackShadowPages; i++) {
>
> for the stack banging during deoptimizations. To me:
>
> +  for (int i = 1; i < StackShadowPages; i++) {
>
> would have been good enough. So what would make sense to me is to use:
>
> +  for (int i = 1; i < StackShadowPages; i++) {
>
> for the stack banging from the deopt blobs. And:
>
> int bang_end = StackShadowPages*page_size;
>
> for the stack banging from compiled code.

Mikael matched code in interpreter in 
AbstractInterpreterGenerator::bang_stack_shadow_pages(). This way we 
consistently touching the same number of pages.

What I want to avoid is 8026775 situation when a middle page is not 
touched. My main concern now is that you changed the page where banging 
*starts* even so the variable is named bang_end:

   // This is how far the previous frame's stack banging extended.
   const int bang_end_safe = bang_end;

But after 8026775 change (<=StackShadowPages) your changes seems right 
since StackShadowPages*page_size is already touched.

You may be touching +1 page but as we discussed above with 
StackYellowPages >=2 it should be fine.

We need to share drawing board :)

thanks,
Vladimir

>
> Roland.
>