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

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

[ppluzhnikov at google dot com]

This is a continuation of issue 10643.

Using test case from issue 10643 (will reattach shortly),
built with -DCRASH results in:

do_aio_write: Invalid argument
do_aio_write: Invalid argument
failure: Invalid argument
failure: Invalid argument

AFAICT, this is happening because size of TLS and TCB is subtracted from
the stack size that the application requested (in the test case, aio_write
is trying to create a thread with 16K stack, but if user requested a thread
with small stack size, pthread_create would have failed just as well).

Since glibc is the only library that knows TLS and TCB sizes, shouldn't
it *add* these sizes to the user requested size, so the user gets exactly
16K of stack he asked for, and not 16K minus "some value we wouldn't
tell you"?

-- 
           Summary: Program with large TLS segment fails aio_write
           Product: glibc
           Version: 2.12
            Status: NEW
          Severity: normal
          Priority: P2
         Component: libc
        AssignedTo: drepper at redhat dot com
        ReportedBy: ppluzhnikov at google dot com
                CC: glibc-bugs at sources dot redhat dot com
 GCC build triplet: x86_64-unknown-linux-gnu
  GCC host triplet: x86_64-unknown-linux-gnu
GCC target triplet: x86_64-unknown-linux-gnu


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

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

[ppluzhnikov at google dot com]

------- Additional Comments From ppluzhnikov at google dot com  2010-07-02 23:03 -------
Created an attachment (id=4869)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4869&action=view)
test case from pr10643


-- 


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

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.

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

[carlos_odonell at mentor dot com]

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

--- Comment #31 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-04-25 20:09:56 UTC ---
(In reply to comment #30)
> Perhaps I should round it up to the next page size after doing the increment.
> In #25 Paul suggested I do the roundup for the increment before adding.
> 
> Or perhaps the safest is to do the roundup of the TLS size and after the add.

You need to have a reason for doing the rounding, otherwise you are wasting
space. What use and what alignment is required by that use should guide your
decision.

The size is going to be aligned up to __static_tls_align_m1 in the next
statement which is the larger of the stack alignment or the maximum alignment
for any given TLS slot (slot minimum alignment is generally 16 bytes).

If you are going to use the extra space for stack then you're fine, the
alignment in the next line has ensured that you get the right size given the
alignment restrictions.

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

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

[asharif.tools at gmail dot com]

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

--- Comment #30 from asharif.tools at gmail dot com 2012-04-25 18:24:01 UTC ---
Perhaps I should round it up to the next page size after doing the increment.
In #25 Paul suggested I do the roundup for the increment before adding.

Or perhaps the safest is to do the roundup of the TLS size and after the add.

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

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

[vapier at gentoo dot org]

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

--- Comment #29 from Mike Frysinger <vapier at gentoo dot org> 2012-04-25 02:54:54 UTC ---
maybe i'm reading it wrong, but if your goal is to round up to the next page,
wouldn't you want to add __static_tls_size to size and then do the round up ?

size = roundup(size + __static_tls_size, __getpagesize());

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #28 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-04-24 22:35:39 UTC ---
(In reply to comment #27)
> Created attachment 6363 [details]
> Proposed fix that only adds to the stack size if it's less than 16 times
> __static_tls_size
> 
> This patch checks if the stack size is less than 16 times the
> __static_tls_size, and adds to it in that case.
> 
> Carlos, I know you're working on this for trunk, but we'd like a fix for
> ChromeOS.
> 
> What are your thoughts about this patch, Paul/Carlos/Mike?

Your proposed patch seems like a fine temporary solution for the ChromeOS
threads.

There is no ABI here so you can feel free to patch locally and use that to fix
the problem.

I would like to more carefully evaluate the heuristic and the point at which we
go from old to new behaviour and make that deterministic instead of dependent
on the size of a random variable (in the statistical sense).

To be clear I would not accept your patch for trunk.

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

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

[asharif.tools at gmail dot com]

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

asharif.tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6297|0                           |1
        is obsolete|                            |

--- Comment #27 from asharif.tools at gmail dot com 2012-04-24 22:02:55 UTC ---
Created attachment 6363
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6363
Proposed fix that only adds to the stack size if it's less than 16 times
__static_tls_size

This patch checks if the stack size is less than 16 times the
__static_tls_size, and adds to it in that case.

Carlos, I know you're working on this for trunk, but we'd like a fix for
ChromeOS.

What are your thoughts about this patch, Paul/Carlos/Mike?

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #26 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-04-01 19:46:32 UTC ---
(In reply to comment #25)
> There's no perfect solution here.  I wonder if we could come up with a good
> heuristic based on the relative sizes of the TLS and thread requested stack. 
> If the TLS is sufficiently small relative to the size of the requested stack,
> then just fold the TLS into the requested stack like we've always done. 
> Otherwise, add the size of the TLS to the size of the requested stack (rounding
> to a page of course).  Harder to document and implement, but seems like it'd
> strike a better balance.
> 
> I don't know where the cutoff should be, 1%, 10%, 50%? Some experimentation may
> guide us.

Jeff,

Thank you for your feedback. I'll consider this as feedback from Red Hat :-)

I'll will work on a fix for this issue.

I'm going to consider slightly more than just TLS size in the heuristic though.

In truth all of the following contribute to the implementation-defined space
that is "stolen" from the stack:

(a) alignment requirements

(b) static tls

(c) guard page

(d) pthread descriptor

(e) minimal rest stack

(f) coloring increment

(g) TLS TCB/DTV size

On top of this I have "stack grows up" patches from HPPA that need applying to
pthread_create.c and allocatestack.c which I'll work into the fixup.

I'll post incremental patches to libc-alpha showing each step of the cleanup.

I also noticed there are some weird pieces of code like:

      /* Adjust the stack size for alignment.  */
      size &= ~__static_tls_align_m1;
      assert (size != 0);

Which makes *no* sense to me. It should make size larger and avoid the assert
(this is the case where we allocate our own stack).

Which brings me to yet another issue I'm going to fix *before* I start work on
this issue: We need generic macros for aligning up and aligning down correctly.

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

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

[ppluzhnikov at google dot com]

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

--- Comment #25 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-03-30 16:44:43 UTC ---
(In reply to comment #24)
> I think the real question here isn't the change in the size of the stack but
> the increase in the # of pages and potential wastage.
> 
> If the TLS is small won't we end up burning an entire page for this small
> amount of data?  For apps that use small stacks and lots of threads, that extra
> page could turn out to be significant.

Yes, as I said in comment#12, an application that may notice this is one
that uses lots of threads, and is *almost* running out of VM space.

> I'm not sure I really buy the argument that if a thread asks for 16k that they
> get exactly 16k usable.

Let's also make "malloc(16K)" return 16K or less of usable memory ;-)

> There's no perfect solution here.  I wonder if we could come up with a good
> heuristic based on the relative sizes of the TLS and thread requested stack. 

Great idea!

> If the TLS is sufficiently small relative to the size of the requested stack,
> then just fold the TLS into the requested stack like we've always done. 
> Otherwise, add the size of the TLS to the size of the requested stack (rounding
> to a page of course).  Harder to document and implement, but seems like it'd
> strike a better balance.
> 
> I don't know where the cutoff should be, 1%, 10%, 50%? Some experimentation may
> guide us.

Given the data Mike Frysinger and I collected (most binaries using <512
bytes of TLS), I say:

  if stack_size_requested >= 16 * __static_tls_sze 
    use current calculation
  else
    increment size request by roundup(__static_tls_size, pagesize())

Most applications today request at least 64K stack (most actually default
to *much* more than that), which would allow up to 4K of static TLS.

But if that same application is instrumented for profiling and suddenly
requires 128K of TLS, it would still work.

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

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

[law at redhat dot com]

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

law at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com

--- Comment #24 from law at redhat dot com 2012-03-30 16:19:32 UTC ---
I think the real question here isn't the change in the size of the stack but
the increase in the # of pages and potential wastage.

If the TLS is small won't we end up burning an entire page for this small
amount of data?  For apps that use small stacks and lots of threads, that extra
page could turn out to be significant.

I'm not sure I really buy the argument that if a thread asks for 16k that they
get exactly 16k usable.

However, obviously, if the TLS is taking up a significant portion of the
thread's requested stack, then something needs to be done.

There's no perfect solution here.  I wonder if we could come up with a good
heuristic based on the relative sizes of the TLS and thread requested stack. 
If the TLS is sufficiently small relative to the size of the requested stack,
then just fold the TLS into the requested stack like we've always done. 
Otherwise, add the size of the TLS to the size of the requested stack (rounding
to a page of course).  Harder to document and implement, but seems like it'd
strike a better balance.

I don't know where the cutoff should be, 1%, 10%, 50%? Some experimentation may
guide us.

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

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

[carlos_odonell at mentor dot com]

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

Carlos O'Donell <carlos_odonell at mentor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

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

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

[carlos_odonell at mentor dot com]

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

Carlos O'Donell <carlos_odonell at mentor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at sourceware    |carlos_odonell at mentor
                   |dot org                     |dot com
   Target Milestone|---                         |2.16

--- Comment #23 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-27 19:08:13 UTC ---
I'm assigning this issue to myself since it seems that nptl/allocatestack.c and
nptl/nptl-ini.c need cleanup regarding the definition of stack.

The fact that POSIX has a seperate set of functions for getting and setting the
guard size indicates that the guard is *not* considered a part of the stack.
Not to mention the POSIX wording "the implementation allocates extra memory"
means that the current GLIBC implementation isn't correct (but one or two pages
doesn't help here). The guard size should be removed from the stack
computation.

Given that the guard size shouldn't be part of the stack size brings even more
doubt on the current practice of placing the static tls and pthread descriptor
into the stack.

Yet another corner case is that using PTHREAD_STACK_MIN as a minimal stack
should get you a thread that starts, but under a system where a variable sized
TLS data region is included you are not guaranteed this at all. In fact using
PTHREAD_STACK_MIN as a size might or might not work which is in my opinion
another failure to meet the principle of "least surprise."

I'm asking for some interpretation help from the Austin Group regarding the
meaning of stack.

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #22 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-26 14:28:05 UTC ---
(In reply to comment #21)
> I think it will least surprise users if "stack" means the stack actually 
> available to the thread, not including implementation details such as TLS, 
> so they can work with tools that determine the amount of stack space used 
> by individual functions to work out how large a stack may be needed.

That sounds like a reasonable position to take. Do we know if other OSs
interpret POSIX `stack' in this way?

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

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

[joseph at codesourcery dot com]

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

--- Comment #21 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2012-03-26 11:02:14 UTC ---
I think it will least surprise users if "stack" means the stack actually 
available to the thread, not including implementation details such as TLS, 
so they can work with tools that determine the amount of stack space used 
by individual functions to work out how large a stack may be needed.

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

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

[vapier at gentoo dot org]

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

--- Comment #20 from Mike Frysinger <vapier at gentoo dot org> 2012-03-25 05:28:43 UTC ---
on my Gentoo/x86_64 system with about 2000 packages installed that covers quite
a range of categories of software, here's my stats (using scanelf to count
non-symlinked ELFs in various dirs):

$ scanelf -yBRF '%F' /bin/ /sbin/ /usr/bin/ /usr/sbin/ | wc -l
3596
$ scanelf -yBRF '%F' {/usr,}/lib64/ | grep -v -e '\.ko$' -e '/debug/' | wc -l
7806

of those ELFs, 297 have TLS regions.  although i've got quite a number of
cross-compilers installed, so if i filter out all the gcc libraries (since
they're more or less just double counting), i get 85 ELFs.

of those, all but 2 are all easily under 0x100.  the only ones over are
libpixman which is 0x180, and then x11vnc/libvncserver which is 0x28a0.  in
this last case, it's almost entirely made up of a structure declaring the vnc
color palette.  and i don't think libvncserver really needs many threads ...
seems to launch short lived threads on the fly for processing input/output
requests.  i doubt it'll be a problem for these at all.

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #19 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-24 20:56:57 UTC ---
(In reply to comment #18)
> On my desktop (based on Ubuntu Lucid), in /usr/bin, there are 430 executables
> that link to libpthread.so, but only 4 of them have TLS:
[At worst 160 bytes] 
> I rest my case ;-)

This is excellent data and is the kind of information you need to make the case
for this kind of change. Gathering more data from the other distro
installations makes it clearer that the change won't severely impact VM
reservation sizes.

I still want to hear from our contacts with the distros.

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

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

[ppluzhnikov at google dot com]

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

--- Comment #18 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-03-24 20:45:11 UTC ---
On my desktop (based on Ubuntu Lucid), in /usr/bin, there are 430 executables
that link to libpthread.so, but only 4 of them have TLS:

lftp
luatex
mono
Xvfb

Of these, the largest TLS MemSize is in mono:

 readelf -l mono | grep -A1 TLS
  TLS            0x00000000002685b0 0x00000000008685b0 0x00000000008685b0
                 0x0000000000000000 0x0000000000000048  R      8

a whopping 72 bytes. The other three have 16-byte TLS.

This isn't of course the whole story, as shared libraries to contribute to
static tls size as well.

In /lib, the following 4 libraries have TLS segment:

libc-2.11.1.so
libcom_err.so.2.1
libmemusage.so
libselinux.so.1
libuuid.so.1.3.0

The largest one is in libselinux.so.1 at 160 bytes:

  TLS            0x000000000001bd50 0x000000000021bd50 0x000000000021bd50
                 0x0000000000000000 0x00000000000000a0  R      8

followed by glibc itself, at 104 bytes:

  TLS            0x0000000000179738 0x0000000000379738 0x0000000000379738
                 0x0000000000000010 0x0000000000000068  R      8

In /usr/lib, the following have TLS:

libcap-ng.so.0.0.0
libdw-0.143.so
libelf-0.143.so
libexempi.so.3.2.1
libgomp.so.1.0.0
libpulsecore-0.9.21.so
libstdc++.so.6.0.13

The largest one is libcap-ng.so.0.0.0 at 48 bytes:

  TLS            0x0000000000003db4 0x0000000000203db4 0x0000000000203db4
                 0x0000000000000030 0x0000000000000030  R      4


I rest my case ;-)

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #17 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-24 20:12:51 UTC ---
(In reply to comment #14)
> the increased size should only matter if the new size pushes past a page
> boundary right ?  and even then, it'd only be 4KiB per thread ?
> 
> i don't see that being a problem.

No the new size is going to be the full size of the PT_TLS segment e.g. static
.tdata/.tbss which is normally allocated out of the thread stack.

Could you do a canvas of a gentoo install to see how big those TLS segments are
in executables?

e.g.
Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
...
  TLS            0x185194 0x00186194 0x00186194 0x00008 0x00040 R   0x4
...

The reservations grow by MemSiz.

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #16 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-24 18:29:24 UTC ---
(In reply to comment #15)
> But I will go ahead and contact distribution maintainers,
> as Carlos suggested in comment#10.

Paul,

Many thanks for spearheading the effort. I think the idea is a good
enhancement, but we need to communicate with the distros about this class of
change and get consensus. By getting input from the distros we might find out
that there are other problems with our solution. I don't see any, but it
doesn't hurt to be careful.

I suggest an email to libc-alpha summarizing the issue, current state of glibc
trunk, and CC the glibc distro contacts and see what they have to say. After a
comment period we'll be ready to implement a solution. If you feel like being
thorough you can provide the reworked patch based on changes to
nptl/nptl-init.c along with that summarizing email.

Thank you again for working through this process.

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

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

[ppluzhnikov at google dot com]

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

--- Comment #15 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-03-24 17:39:41 UTC ---
(In reply to comment #14)
> the increased size should only matter if the new size pushes past a page
> boundary right ?  and even then, it'd only be 4KiB per thread ?

We really have to be careful when talking about "size".
The actual used memory (RSS) is unchanged, only the reserved
size (VM) is changing.

To re-state Carlos' example, consider a 32-bit application that creates
1000s of threads, and has large TLS (1000s of __thread variables).

Each thread is created with 64K stack (just barely sufficient to
accomodate all of TLS variables) and otherwise does not use much
of stack (e.g. a non-recursive computation).

This application works fine currently.

Under proposed change, the actual stack usage (RSS) is unchanged,
but the reserved (VM) stack size for each thread nearly doubles
to 64K+__static_tls_size.

So the application would only be able to create half as many
threads as it used to.



I assert that this is a contrived example, and has a trivial fix:
reduce 64K to 4K (or whatever *actual* stack the threads use).

But I will go ahead and contact distribution maintainers,
as Carlos suggested in comment#10.

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

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

[vapier at gentoo dot org]

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

Mike Frysinger <vapier at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vapier at gentoo dot org

--- Comment #14 from Mike Frysinger <vapier at gentoo dot org> 2012-03-24 17:08:10 UTC ---
the increased size should only matter if the new size pushes past a page
boundary right ?  and even then, it'd only be 4KiB per thread ?

i don't see that being a problem.

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

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

[carlos_odonell at mentor dot com]

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

Carlos O'Donell <carlos_odonell at mentor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carlos at systemhalted dot
                   |                            |org

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #13 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-24 16:05:19 UTC ---
(In reply to comment #12)
> (In reply to comment #8)
> 
> > 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.
> 
> The helper thread stack size *is* set by __pthread_get_minstack.

Thanks, I just spotted that in nptl/sysdeps/unix/sysv/linux/aio_misc.h.

> This was done here:
> 
> 2011-12-22  Ulrich Drepper  <drepper@gmail.com>
> 
>        [BZ #13088]
>        * sysdeps/unix/sysv/linux/timer_routines.c: Get minimum stack size
>        through __pthread_get_minstack.
>        * nptl-init.c (__pthread_initialize_minimal_internal): Get page size
>        directly from _rtld_global_ro.
>        (__pthread_get_minstack): New function.
>        * pthreadP.h: Declare __pthread_get_minstack.
>        * Versions (libpthread) [GLIBC_PRIVATE]: Add __pthread_get_minstack.
> 
> and is *precisely* the change we are asking for for the other threads.

Thanks for pointing this out.

> I see that Rich Felker is in exact agreement with me:
> http://sourceware.org/bugzilla/show_bug.cgi?id=13088#c1
> 
> > You are also increasing the memory footprint of all applications that use TLS
> > that worked fine before.
> 
> It depends on what you call "memory footprint". Yes, we'll increase memory
> we *reserve* for stacks (VM size). But we will not *use* any more memory
> than what's already used (RSS).

Reservations still consume VM space and limit the maximum number of threads.
Reservation is still a serious problem for kernel VM management and layout. We
should not increase the reserved VM space without due consideration.

> I think the only application that would notice this is one that was almost
> running out of VM space. An answer for such app would be to decrease its
> default stack sizes, use smaller number of threads, or switch to 64 bits.

Not true. What about applications with *lots* of threads, each thread running
computational kernels (little real stack usage), and lots of thread-local data.
In those cases you could be doubling the reservation per thread and causing the
application to be unable to spawn a larger number of threads.

> > 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.
> 
> Would above still apply if it's just VM size we are talking about?
> I don't see how readelf will reveal anything.

Yes. We need to get input from the distros. We should not make a change like
this without talking to them.

You use readelf to find the TLS segment and see how much bigger the per-thread
memory reservation will be and collect this data for as many applications as
possible to see the expected real-world impact.

> > There are multiple cases here.
> 
> > 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) "
> 
> Yes, I believe that's what we are proposing.

I'm glad to see that I understand the problem.

Cheers,
Carlos.

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

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

[ppluzhnikov at google dot com]

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

--- Comment #12 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-03-24 02:21:06 UTC ---
(In reply to comment #8)

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

The helper thread stack size *is* set by __pthread_get_minstack.

The test case here no longer crashes using the current git trunk, but only
because the aio thread stack is now increased to accomodate static tls:

  // nptl/nptl-init.c
  size_t
  __pthread_get_minstack (const pthread_attr_t *attr)
  {
    struct pthread_attr *iattr = (struct pthread_attr *) attr;

    return (GLRO(dl_pagesize) + __static_tls_size + PTHREAD_STACK_MIN
        + iattr->guardsize);
  }

This was done here:

2011-12-22  Ulrich Drepper  <drepper@gmail.com>

       [BZ #13088]
       * sysdeps/unix/sysv/linux/timer_routines.c: Get minimum stack size
       through __pthread_get_minstack.
       * nptl-init.c (__pthread_initialize_minimal_internal): Get page size
       directly from _rtld_global_ro.
       (__pthread_get_minstack): New function.
       * pthreadP.h: Declare __pthread_get_minstack.
       * Versions (libpthread) [GLIBC_PRIVATE]: Add __pthread_get_minstack.

and is *precisely* the change we are asking for for the other threads.

I see that Rich Felker is in exact agreement with me:
http://sourceware.org/bugzilla/show_bug.cgi?id=13088#c1

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

It depends on what you call "memory footprint". Yes, we'll increase memory
we *reserve* for stacks (VM size). But we will not *use* any more memory
than what's already used (RSS).

I think the only application that would notice this is one that was almost
running out of VM space. An answer for such app would be to decrease its
default stack sizes, use smaller number of threads, or switch to 64 bits.

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

Would above still apply if it's just VM size we are talking about?
I don't see how readelf will reveal anything.

> There are multiple cases here.

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

Yes, I believe that's what we are proposing.

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

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

[carlos_odonell at mentor dot com]

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

Carlos O'Donell <carlos_odonell at mentor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joseph at codesourcery dot
                   |                            |com
              Flags|                            |review?(joseph at
                   |                            |codesourcery dot com)

--- Comment #11 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-23 23:02:38 UTC ---
Joseph,

Any comments on this issue specifically with respect to the POSIX meaning of
"stack" and wether it should or should include implementation dependent details
like where static TLS data is stored?

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #10 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-23 23:01:14 UTC ---
Paul et al.,

The next steps for this issue are:

(a) Rewrite the patch to adjust the default stack size in nptl/nptl-init.c to
account for large TLS data as *additional* space.

(b) Find people from each of the major distributions to comment on this issue.
Asking them the pointed question "Would you mind if the memory footprint of all
applications that use TLS goes up by the size of the static TLS in order to
provide assurances that your application will have some reasonable amount of
thread stack not consumed by the TLS data?" I suggest: Jeff Law (Red Hat),
Aurelien Jarno (Debian), Andreas Jaeger (SuSE), Ubuntu (??), Mike Frysinger
(Gentoo), ... 

I'm here to guide you, but this kind of work needs to come from the ground up
and from the people most interested in seeing this fixed. We need volunteers
like you on the product to help fix these kinds of issues, but it does require
some work to reach consensus :-)

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #9 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-23 22:50:12 UTC ---
(In reply to comment #7)
> I can provide some information about Chrome built with -fprofile-generate:
> 
> p __static_tls_size
> $1 = 114880
> 
> Now let's look at how Chrome allocates thread stacks:
> 
> http://code.google.com/p/chromium/source/search?q=kShutdownDetectorThreadStackSize&origq=kShutdownDetectorThreadStackSize&btnG=Search+Trunk
> 
> http://code.google.com/p/chromium/source/search?q=kWorkerThreadStackSize&origq=kWorkerThreadStackSize&btnG=Search+Trunk
> 
> are two examples. There are other stack sizes too. From what I have seen, if
> the stack size is less than __static_tls_size, it just fails to allocate the
> stack. If it is something like 128k, it gets allocated, but soon runs out of
> stack into the guard page where it causes a segfault on a random function (as
> it allocating locals for the function).

That's brutal. You have a 128k stack with 114k of thread local data, a guard
page, the thread descriptor, and you've barely got anything left.

Thank you for some real-world data, that helps put things into perspective.

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

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

[carlos_odonell at mentor dot com]

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.

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

[asharif.tools at gmail dot com]

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

--- Comment #7 from asharif.tools at gmail dot com 2012-03-23 22:06:50 UTC ---
I can provide some information about Chrome built with -fprofile-generate:

p __static_tls_size
$1 = 114880

Now let's look at how Chrome allocates thread stacks:

http://code.google.com/p/chromium/source/search?q=kShutdownDetectorThreadStackSize&origq=kShutdownDetectorThreadStackSize&btnG=Search+Trunk

http://code.google.com/p/chromium/source/search?q=kWorkerThreadStackSize&origq=kWorkerThreadStackSize&btnG=Search+Trunk

are two examples. There are other stack sizes too. From what I have seen, if
the stack size is less than __static_tls_size, it just fails to allocate the
stack. If it is something like 128k, it gets allocated, but soon runs out of
stack into the guard page where it causes a segfault on a random function (as
it allocating locals for the function).

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

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

[ppluzhnikov at google dot com]

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

--- Comment #6 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-03-23 21:32:30 UTC ---
(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.

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

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

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

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

[carlos_odonell at mentor dot com]

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

Carlos O'Donell <carlos_odonell at mentor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|drepper.fsp at gmail dot    |unassigned at sourceware
                   |com                         |dot org

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

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

[carlos_odonell at mentor dot com]

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

Carlos O'Donell <carlos_odonell at mentor dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carlos_odonell at mentor
                   |                            |dot com

--- Comment #5 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-23 20:37:19 UTC ---
If the minimal values computes by the code are not enough then the only real
solution is to fix the programs with large per-thread memory requirements e.g.
set RLIMIT_STACK to a reasonable value.

We do not want to penalize all of the other programs that don't need the extra
stack space.

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

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

[carlos_odonell at mentor dot com]

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

--- Comment #4 from Carlos O'Donell <carlos_odonell at mentor dot com> 2012-03-23 20:34:37 UTC ---
I've reviewed the patch sent to the mailing list and provided comments:
http://sourceware.org/ml/libc-alpha/2012-03/msg01002.html

I just noticed that what I recommended shouldn't be required since the code
should already take this into account.

Nobody has given any hard numbers about the static TLS size so I'm marking this
issue as unconfirmed.

Please provide some real-world figures so we know how bad the problem is
compared to the default stack size for x86 e.g. 2MB.
sysdeps/i386/pthreaddef.h:#define ARCH_STACK_DEFAULT_SIZE       (2 * 1024 *
1024)

For example code in nptl-init.c tries to take things into account:
~~~ nptl/nptl-inic.c:
414   /* Determine the default allowed stack size.  This is the size used
415      in case the user does not specify one.  */
416   struct rlimit limit;
417   if (getrlimit (RLIMIT_STACK, &limit) != 0
418       || limit.rlim_cur == RLIM_INFINITY)
419     /* The system limit is not usable.  Use an architecture-specific
420        default.  */
421     limit.rlim_cur = ARCH_STACK_DEFAULT_SIZE;
422   else if (limit.rlim_cur < PTHREAD_STACK_MIN)
423     /* The system limit is unusably small.
424        Use the minimal size acceptable.  */
425     limit.rlim_cur = PTHREAD_STACK_MIN;
426
427   /* Make sure it meets the minimum size that allocate_stack
428      (allocatestack.c) will demand, which depends on the page size.  */
429   const uintptr_t pagesz = GLRO(dl_pagesize);
430   const size_t minstack = pagesz + __static_tls_size + MINIMAL_REST_STACK;
431   if (limit.rlim_cur < minstack)
432     limit.rlim_cur = minstack;
433
434   /* Round the resource limit up to page size.  */
435   limit.rlim_cur = (limit.rlim_cur + pagesz - 1) & -pagesz;
436   __default_stacksize = limit.rlim_cur;
~~~

Note that we check to see that __default_stacksize takes into account
__static_tls_size and a minimal rest size (2K on x86).

Is the problem that the minimal computed is *still* not enough?

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

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

[vapier at gentoo dot org]

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

Mike Frysinger <vapier at gentoo dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |toolchain at gentoo dot org

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

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

[asharif.tools at gmail dot com]

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

--- Comment #3 from asharif.tools at gmail dot com 2012-03-22 19:39:25 UTC ---
Created attachment 6297
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6297
Proposed fix.

It increases size by TLS size. The test case passes as well as Chrome with
-fprofile-generate.

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

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

[asharif.tools at gmail dot com]

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

asharif.tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |asharif.tools at gmail dot
                   |                            |com

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

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

[ppluzhnikov at google dot com]

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

--- Comment #2 from Paul Pluzhnikov <ppluzhnikov at google dot com> 2012-02-24 04:34:48 UTC ---
Ping?

This hit us again today in a different context: Chrome uses 128K thread stacks,
which is normally plenty.

But when instrumented for profiling, the counters reside in TLS, and consumed
most of the 128K stack, resulting in a crash that was hard to diagnose.

We then had to change the source before instrumented Chrome could run.

It would be nice if glibc just did the right thing, getting rid of the crash
and the need to modify sources.

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