RFR: Fix jtreg test runtime/StackGuardPages/

[David Holmes]

On 6/04/2018 6:23 PM, Siebenborn, Axel wrote:
> Hi David,
> You're right, it's better not to use a *_np function.
> 
> I prepared a new webrev:
> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_c/

That looks better! ;-)

Cheers,
David

> Thanks,
> Axel
> 
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Freitag, 6. April 2018 09:13
>> To: Siebenborn, Axel <axel.siebenborn at sap.com>; Mikael Vidstedt
>> <mikael.vidstedt at oracle.com>
>> Cc: portola-dev at openjdk.java.net
>> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
>>
>> On 6/04/2018 4:41 PM, Siebenborn, Axel wrote:
>>> Hi,
>>> As discussed off-list with Mikael, I get a reasonable stack size for
>> pthread_create by using JNI_GetDefaultJavaVMInitArgs.
>>> This way, we have a stack suitable to run java code.  The stacksize  is not
>> hardcoded in the test and platform independent.
>>
>> The test isn't platform independent when it uses a *_np function.
>>
>> I'm not sure why you are changing the default attributes used by
>> pthread_create instead of just passing a suitable attr object when
>> creating the new thread ref. pthread_attr_setstacksize ??
>>
>> Cheers,
>> David
>>
>>> I updated the webrev:
>>>
>> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_a
>> /
>>>
>>> Regards,
>>> Axel
>>>
>>>> -----Original Message-----
>>>> From: Mikael Vidstedt [mailto:mikael.vidstedt at oracle.com]
>>>> Sent: Dienstag, 3. April 2018 19:31
>>>> To: Siebenborn, Axel <axel.siebenborn at sap.com>
>>>> Cc: portola-dev at openjdk.java.net
>>>> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
>>>>
>>>>
>>>> The total stack size is 80kb, of which 40kb/10 pages worth is used for
>> shadow
>>>> pages (48kb/12 pages in debug mode). That leaves 80-48=32kb stack for
>>>> execution, which admittedly is not a lot in today’s world. However, I
>> would
>>>> have thought that the initialization would still have room to play in that,
>> but it
>>>> seems not?
>>>>
>>>> I also assume that the reason why this isn’t a problem for the normal
>>>> launcher case is that the launcher doesn’t use the primordial. Perhaps the
>>>> right way of fixing this is buy having the test specific launcher here mimic
>>>> what the real launcher does (something not entirely unlike
>>>> ContinueInNewThread)?
>>>>
>>>> Cheers,
>>>> Mikael
>>>>
>>>>> On Apr 3, 2018, at 5:30 AM, Siebenborn, Axel
>> <axel.siebenborn at sap.com>
>>>> wrote:
>>>>>
>>>>> Hi Mikael,
>>>>> Both  tests fails because
>>>>> res = (*_jvm)->AttachCurrentThread(_jvm, (void **)&env, NULL);
>>>>> returns with an error (res != JNI_OK).
>>>>>
>>>>> AttachCurrentThread uses the current thread as JavaThread and
>> allocates a
>>>> java level Thread object and needs to run initializing java code
>>>> (JavaThread::allocate_threadObj).
>>>>> In order to run java code, the remaining stack space is checked. There
>> must
>>>> be sufficient  space for an interpreter frame, the java frame and shadow
>>>> pages ( JavaCalls::call_helper() ). The default for the number of shadow
>>>> pages is 10 for a release build. If this check fails a StackOverflowException
>> is
>>>> thrown.
>>>>> As we return with a pending exception the attach fails and we return
>>>> JNI_ERR.
>>>>>
>>>>> This is a problem as we call AttachCurrentThread from a thread that we
>>>> created with the default stacksize. Threads created by the jvm are
>> created
>>>> with a much higher stacksize.
>>>>>
>>>>> webrev_a fixes the test by providing an argument for the stacksize to
>>>> pthread_create.
>>>>>
>>>>> webrev_b reduces the amount of shadow pages.
>>>>>
>>>>> Does that make things clearer?
>>>>>
>>>>> Regards,
>>>>> Axel
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Mikael Vidstedt [mailto:mikael.vidstedt at oracle.com]
>>>>>> Sent: Dienstag, 3. April 2018 00:01
>>>>>> To: Siebenborn, Axel <axel.siebenborn at sap.com>
>>>>>> Cc: portola-dev at openjdk.java.net
>>>>>> Subject: Re: RFR: Fix jtreg test runtime/StackGuardPages/
>>>>>>
>>>>>>
>>>>>> Alex,
>>>>>>
>>>>>> Can you please add some additional color/details to why it fails? Is it the
>>>>>> native or java variant that fails (or both)?
>>>>>>
>>>>>> I understand that the stack is smaller with musl (I ran into that problem
>> in
>>>> a
>>>>>> different context), but I’m not sure I see why Thread.init() would fail
>> just
>>>>>> because of that. Help? :)
>>>>>>
>>>>>> Cheers,
>>>>>> Mikael
>>>>>>
>>>>>>> On Mar 22, 2018, at 3:37 AM, Siebenborn, Axel
>>>>>> <axel.siebenborn at sap.com> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> the test  runtime/StackGuardPages/ fails, as AttachCurrentThread()
>>>> returns
>>>>>> with an error. The reason is a stackoverflow while trying to run java
>>>>>> Thread.init().
>>>>>>> The default pthread stack size is 80 K, much less than on other Unixes.
>>>>>>>
>>>>>>> The first fix is to check the pthread default stacksize and increase it if
>> it's
>>>>>> less than 160K.
>>>>>>>
>>>>>>> Webrev a:
>>>>>>>
>>>>>>
>>>>
>> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_a
>>>>>> /
>>>>>>>
>>>>>>> However, the reason we need that much stack, just to run a single
>> java
>>>>>> method, is the large amount of shadow pages.
>>>>>>> The second fix is just to add a flag to JNI_CreateJavaVM, to set the
>>>> number
>>>>>> of shadow pages to a lower value. However, a change in
>> globals_x86.hpp
>>>> is
>>>>>> needed to allow smaller minimum for that value. Here I miss a macro
>> like
>>>>>> MUSL_OLNLY(code).
>>>>>>>
>>>>>>> Webrev b:
>>>>>>>
>>>>>>
>>>>
>> http://cr.openjdk.java.net/~asiebenborn/jtreg_StackGuardPages/webrev_
>>>>>> b/
>>>>>>>
>>>>>>> I'm unsure which change to use.
>>>>>>> What do you think?
>>>>>>>
>>>>>>> Regards,
>>>>>>> Axel
>>>>>>>
>>>>>>>
>>>>>
>>>