Fix for BZ 22207 -- intermittent failure to create threads on 32-bit machines
diff mbox series

Message ID CALoOobOc1oypj=cDpFHoL-bCJ8qC9aEPB6RnbSQ7=umAWp4KHw@mail.gmail.com
State New
Headers show
Series
  • Fix for BZ 22207 -- intermittent failure to create threads on 32-bit machines
Related show

Commit Message

Paul Pluzhnikov Sept. 26, 2017, 12:09 a.m. UTC
Greetings,

In stdlib/test-{atexit,at_quick_exit,cxa_atexit,on_exit} tests, I am
creating 1024 new threads.

With default 8MiB Linux thread stack size, if all threads are alive at
the same time, this would require 8GiB of VM, which is a bit of a
problem on 32-bit machines.

Attached patch reduces total VM space required for all threads to 128MiB.

Proposed commit message:

---
Reduce total memory required to create all threads to 128MiB.

This fixes intermittent failure in
stdlib/test-{atexit,at_quick_exit,cxa_atexit,on_exit} tests (Bug
22207).
---

Thanks,

Comments

H.J. Lu Sept. 26, 2017, 12:46 a.m. UTC | #1
On 9/25/17, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> Greetings,
>
> In stdlib/test-{atexit,at_quick_exit,cxa_atexit,on_exit} tests, I am
> creating 1024 new threads.
>
> With default 8MiB Linux thread stack size, if all threads are alive at
> the same time, this would require 8GiB of VM, which is a bit of a
> problem on 32-bit machines.
>
> Attached patch reduces total VM space required for all threads to 128MiB.
>
> Proposed commit message:
>
> ---
> Reduce total memory required to create all threads to 128MiB.
>
> This fixes intermittent failure in
> stdlib/test-{atexit,at_quick_exit,cxa_atexit,on_exit} tests (Bug
> 22207).
> ---
>

This is OK.

Thanks.
Florian Weimer Sept. 26, 2017, 6:04 a.m. UTC | #2
On 09/26/2017 02:09 AM, Paul Pluzhnikov wrote:
> +  /* With default 8MiB Linux stack size, creating 1024 threads can cause
> +     VM exhausiton on 32-bit machines.  Reduce stack size of each thread to
> +     128KiB for a maximum required VM size of 128MiB.  */
> +  xpthread_attr_setstacksize (&attr, 128 * 1024);

Is there a reason why you do not use PTHREAD_STACK_MIN?

Thanks,
Florian
Paul Pluzhnikov Sept. 26, 2017, 6:23 a.m. UTC | #3
On Mon, Sep 25, 2017 at 11:04 PM, Florian Weimer <fweimer@redhat.com> wrote:
>
> On 09/26/2017 02:09 AM, Paul Pluzhnikov wrote:
>>
>> +  /* With default 8MiB Linux stack size, creating 1024 threads can cause
>> +     VM exhausiton on 32-bit machines.  Reduce stack size of each thread to
>> +     128KiB for a maximum required VM size of 128MiB.  */
>> +  xpthread_attr_setstacksize (&attr, 128 * 1024);
>
>
> Is there a reason why you do not use PTHREAD_STACK_MIN?

It would possibly work here, but often PTHREAD_STACK_MIN doesn't leave
enough space to run any code at all: the limit is minimal acceptable
to the pthread library, not a "reasonable default if your call chains
aren't deep".

And setting the limit too low opens the test to random out-of-stack
failures depending on how the test is built, what machine it's built
for, etc. etc.

I don't think it's worth trying to get to the absolute minimum here:
most of the stack space will (hopefully) be unused and not allocated
any real memory, and so as long as we stay under 4GiB of VM, the test
should work fine whether PTHREAD_STACK_MIN or 128KiB is used.
Florian Weimer May 28, 2018, 9:51 a.m. UTC | #4
On 09/26/2017 08:23 AM, Paul Pluzhnikov wrote:
> On Mon, Sep 25, 2017 at 11:04 PM, Florian Weimer <fweimer@redhat.com> wrote:
>>
>> On 09/26/2017 02:09 AM, Paul Pluzhnikov wrote:
>>>
>>> +  /* With default 8MiB Linux stack size, creating 1024 threads can cause
>>> +     VM exhausiton on 32-bit machines.  Reduce stack size of each thread to
>>> +     128KiB for a maximum required VM size of 128MiB.  */
>>> +  xpthread_attr_setstacksize (&attr, 128 * 1024);
>>
>>
>> Is there a reason why you do not use PTHREAD_STACK_MIN?
> 
> It would possibly work here, but often PTHREAD_STACK_MIN doesn't leave
> enough space to run any code at all: the limit is minimal acceptable
> to the pthread library, not a "reasonable default if your call chains
> aren't deep".

Agreed, PTHREAD_STACK_MIN would not work here.

Thanks,
Florian

Patch
diff mbox series

diff --git a/stdlib/test-atexit-race-common.c b/stdlib/test-atexit-race-common.c
index b3792837c7..3a03e06cb9 100644
--- a/stdlib/test-atexit-race-common.c
+++ b/stdlib/test-atexit-race-common.c
@@ -57,6 +57,11 @@  do_test (void)
   xpthread_attr_init (&attr);
   xpthread_attr_setdetachstate (&attr, 1);
 
+  /* With default 8MiB Linux stack size, creating 1024 threads can cause
+     VM exhausiton on 32-bit machines.  Reduce stack size of each thread to
+     128KiB for a maximum required VM size of 128MiB.  */
+  xpthread_attr_setstacksize (&attr, 128 * 1024);
+
   for (i = 0; i < kNumThreads; ++i) {
     xpthread_create (&attr, threadfunc, NULL);
   }