This is the mail archive of the glibc-bugs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug libc/11787] Program with large TLS segment fails aio_write


http://sourceware.org/bugzilla/show_bug.cgi?id=11787

--- Comment #8 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-23 22:46:37 UTC ---
(In reply to comment #6)
> (In reply to comment #5)
> 
> Carlos,
> 
> I don't believe you've understood the problem.
> 
> Default stack sizes are fine.
> 
> But aio_write creates a small (16K) stack, and chrome creates 128K stacks.
> 
> Normally this is also just fine; and all works.
> 
> But then application creates a larger-than-usual TLS (either by allocating
> 4096 thread-local ints as in the test case here, or by instrumenting for
> profiling), and suddenly things start crashing in hard-to-diagnose fashion.

That does make the problem clearer.

Please note that the aio helper thread *should* be using a default 2MB stack on
x86, not 16K, I don't see anywhere that sets the helper threads stack to 16K.

> > We do not want to penalize all of the other programs that don't need the extra
> > stack space.
> 
> You aren't penalizing them much if they aren't using TLS, and if they are
> using large TLS, then you are making them work instead of crashing.

You are also increasing the memory footprint of all applications that use TLS
that worked fine before.

Before making any changes we need to hear from the distros (RH, SuSE, Debian,
Gentoo, Ubuntu etc) about the kind of impact this might have e.g. a quick find
/ readelf -a / grep to determine on average the memory increase we are looking
at.

> From "man pthread_attr_setstacksize":
> 
> 
>     The pthread_attr_setstacksize() function sets the stack size attribute of
>     the thread attributes object referred to by attr to the value specified
>     in stacksize.
> 
> It doesn't say "to the value specified in stacksize minus the size of TLS".
> 
> The fact that GLIBC chops off space for TLS from the top of stack is an
> implementation detail, and (IMHO) should *not* reduce the stack size
> application actually gets!

The "stack" is purposely ambiguous and once handed over to the implementation
the implementation has complete control and can do what it wishes including use
some of it for TLS which is what we do.

http://pubs.opengroup.org/onlinepubs/007904975/functions/xsh_chap02_09.html#tag_02_09_08

Having said that it is *bad* for us to crash in the *DEFAULT* case because TLS
data takes up the entire default stack.

However, you are arguing for variable sized thread stacks based on TLS data,
which is a fine suggestion but needs serious consideration.

There are multiple cases here.

(a) App. provides memory (pthread_attr_setstackaddr, pthread_attr_setstack).

(b) App. requests minimum size (pthread_attr_setstacksize).

(c) App. makes no request, and RLIMIT_STACK is set and >= PTHREAD_STACK_MIN.

(d) App. makes no request, and RLIMIT_STACK is set and < PTHREAD_STACK_MIN.

(e) App. makes no request, and RLIMIT_STACK is 0/inf.

What do you suggest for each of these cases?

Are there any other cases I might have missed?

In the current implementation we do the following:

For (a) we use the user memory for everything we need.

For (b) we allocate the minimum and use that for everything we need.

For (c) we allocate the value of RLIMIT_STACK only if it's >= minstack,
otherwise minstack (nptl-init.c) and use that for everything we need.

For (d) we allocate the value of PTHREAD_STACK_MIN only if it's >= minstack,
otherwise minstack (nptl-init.c) and use that for everything we need.

For (e) we allocate the value of ARCH_STACK_DEFAULT_SIZE only if it's >=
minstack, otherwise minstack (nptl-init.c) and use that for everything we need.

You appear to be suggesting the following:

For (a) the behaviour remains the same.

For (b) we adjust upward by the size of the static TLS data.

For (c) "

For (d) "

For (e) "

In which case the patch is probably to nptl-init.c to change the computation of
the default size.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]