RFR: 8225035: Thread stack size issue caused by large TLS size

Thu Jun 13 01:32:13 UTC 2019

On 6/12/19 5:06 PM, David Holmes wrote:
> Hi Thomas,
>
> > I am not sure, but is adding the TLS sized buffer to just small thread
> > stacks the right thing to do? Why it is okay for thread stacks
> > slightly larger than the cut-off point to live without this additional
> > space? Will they not as well be hurt by the large TLS subtraction from
> > their stack sizes?
>
> This was discussed previously:
>
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-May/034460.html 
>
>
> The premise is that the smaller the requested stack the more likely 
> unexpected TLS usage will lead to stackoverflows. If I set 32K stacks 
> and TLS uses 16K I likely have a problem at runtime. If I have 2M 
> stacks and TLS uses 16K I probably don't really care. So the proposal 
> is to use the ratio of TLS-size to requested-stack-size as the trigger 
> - configurable by the end user. That way for the expected case of 
> large stacks with small TLS, and we're not running on the edge of 
> stackoverflow, there will be no change in stack usage or general 
> behaviour. In contrast really small stacks will be expanded by the TLS 
> size to avoid stackoverflow - with some user control. In the latter 
> case if TLS can be accommodated without overflow then the user can 
> increase the ratio to avoid the extra stack usage. If TLS-size causes 
> overflow then the user has no choice if they want their application to 
> run.
>
> It's not perfect but nothing is.
>

Hi David,

Thanks for the summary. I didn't follow this whole discussion so I 
apologize for asking dumb questions.

First, I think the bug description needs to be expanded to clearly 
describe the problem and the proposed fix (and alternatives considered). 
It currently basically says -- "this was discussed before and we are 
going to fix it in a way discussed elsewhere".

I am sure this has been discussed before -- is there any way for the 
users to specify both stack size (-XX:ThreadStackSize) and TLS size 
(using __thread in C code??), so they can live with their own perils 
without the VM making any guesses?

With the proposed patch, we have added a single knob, 
-XX:AdjustWithMinStackAt, for controlling scenarios that seem to be 
affected by multiple factors. Is this a sufficient solution for all 
foreseeable scenarios? E.g., TLS used by dynamically loaded DSOs as 
described by Andrew Haley in a separate email.

Thanks
- Ioi

> Cheers,
> David
> -----
>
> On 13/06/2019 6:45 am, Thomas Stüfe wrote:
>> Hi Jiangli,
>>
>> after learning a lot of interesting things today about TLS :-) I took 
>> a look at your change.
>>
>> First another question, just to be sure I understand this: From what 
>> think I understood about TLS, and from the way you wrote your test 
>> (writing an own launcher and all) I assume the problem only happens 
>> for TLS of modules which are loaded at program start, right? So 
>> Libjvm.so and dynamically loaded JNI libs can allocate as many 
>> __thread space as they like, they would not go on the thread stacks? 
>> And this is only a problem when you embed the libjvm into other 
>> launchers which use a lot of TLS?
>>
>> ---
>>
>> I do not understand why the complicated dynamic lookup of 
>> __pthread_get_minstack. What is the advantage of this two-level 
>> approach vs just dlsym'ing the pointer like we do for all other 
>> dynamically loaded functions? Also, would this not cause problems 
>> when building for other libc implementations, e.g. muslc?
>>
>> And why RTLD_NEXT, why not RTLD_DEFAULT? Any special reason? Since 
>> that seems to be how we normally load dynamic symbols from the libc.
>>
>> About the fkt pointer, in contrast to David I would prefer to see it 
>> is a pointer, not a function, so your first name which I assume was 
>> "p_.." I would have liked more.
>>
>> ---
>>
>> +  size_t min_stack_size = align_up(pthread_get_minstack(&attr), 
>> vm_page_size());
>>
>> Why the align up? The only condition we have to meet is for the total 
>> stack size to be page aligned. On some platforms page size can be 
>> large (I think we have 64K pages on ppc) and TLS size could be 
>> smaller than that. Could you move the align up do right before the 
>> call to pthread_attr_setstacksize() (and probably remove the assert 
>> there)?
>>
>> ---
>>
>> What is the logic behind giving attr to pthread_get_minstack? If 
>> Florian is right and this: 
>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-May/034593.html 
>> is the implementation, that parameter seems not to be used. In that 
>> case, just to be sure, I would feed it a dummy attr.
>>
>> ---
>>
>> I am not sure, but is adding the TLS sized buffer to just small 
>> thread stacks the right thing to do? Why it is okay for thread stacks 
>> slightly larger than the cut-off point to live without this 
>> additional space? Will they not as well be hurt by the large TLS 
>> subtraction from their stack sizes?
>>
>> Thank you!
>>
>> Cheers, Thomas
>>
>>
>>
>>
>> On Tue, Jun 11, 2019 at 12:30 AM Jiangli Zhou <jianglizhou at google.com 
>> <mailto:jianglizhou at google.com>> wrote:
>>
>>     Hi David and Florian,
>>
>>     This the a follow up of the discussion regarding the TLS (Thread 
>> Local
>>     Storage) issue:
>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2019-May/034459.html.
>>     The following is an updated patch with your suggestions 
>> incorporated:
>>
>>     1) Use __pthread_get_minstack
>>     2) By default only adjust the thread stack size if the
>>     minstack-to-stacksize ratio is >= 10%
>>     3) Introduce a new command-line option (Linux only),
>>     AdjustWithMinStackAt, which can be used to change the default 10%
>>     ratio for triggering stack size adjustment
>>
>>     CSR: https://bugs.openjdk.java.net/browse/JDK-8225498
>>     webrev: http://cr.openjdk.java.net/~jiangli/8225035/webrev.00/
>>     bug: https://bugs.openjdk.java.net/browse/JDK-8225035
>>
>>     Best regards,
>>
>>     Jiangli
>>