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
>>>>>>>>>>>>>>>>>>>>>>