diff mbox

tst-malloc-thread-exit: run less aggressively

Message ID 56A26836.5020509@redhat.com
State New
Headers show

Commit Message

Florian Weimer Jan. 22, 2016, 5:34 p.m. UTC 
On 01/19/2016 09:04 PM, Florian Weimer wrote:
> On 01/19/2016 08:58 PM, Chris Metcalf wrote:
>> On a 64-bit machine, the previous code would run up to
>> 5 x 8 x NPROCS threads, or 2,880 threads on a 72-core TILE-Gx.
>> But since typically userspace runs with an RLIMIT_NPROC value
>> of 1024, the test will fail on such a machine.  Instead, bound
>> the outer thread count to 200, rather than 8 x NPROCS, so that
>> the total number of threads created is max 1000.  This may still
>> be a little risky when running in a "make -j check" context but
>> should at least pass when run separately to confirm a FAIL.
> 
> I posted a patch due to similar issues:
> 
>   <https://sourceware.org/ml/libc-alpha/2015-12/msg00537.html>
> 
> This change will bring down the run time considerably.
> 
>> Can I push this for 2.23?  It fixes a test failure for tilegx.
> 
> Not just there.
> 
> I wasn't aware of the thread count limit.  I can post a patch tomorrow
> which uses mallopt to reduce the arena count, then we should be able to
> run this test with just 8 threads (but I will have to verify it still
> triggers the original failure).

I haven't finished the mallopt approach.  It seems that my mental model
how the test works is wrong.  Two threads with eight arenas appear to
trigger the issue reliably, but I want to test on a larger machine as
well, to make sure the test is any good there.  The problem there is
that I wasn't

I'm attaching what I'm using for testing.  Warning: It contains a revert
of the bug fix.

Florian

Comments

Florian Weimer Jan. 22, 2016, 7 p.m. UTC | #1 
On 01/22/2016 06:34 PM, Florian Weimer wrote:
> I haven't finished the mallopt approach.  It seems that my mental model
> how the test works is wrong.  Two threads with eight arenas appear to
> trigger the issue reliably, but I want to test on a larger machine as
> well, to make sure the test is any good there.  The problem there is
> that I wasn't

… able to find sufficiently large box.  That has now changed, and I can
confirm that the bug still reproduces with the changed test.

Chris, can you confirm that the tweaked test addresses your problem?
Then I'll send a real patch for review.

Thanks,
Florian
Chris Metcalf Jan. 23, 2016, 2:59 a.m. UTC | #2 
On 1/22/2016 2:00 PM, Florian Weimer wrote:
> On 01/22/2016 06:34 PM, Florian Weimer wrote:
>> I haven't finished the mallopt approach.  It seems that my mental model
>> how the test works is wrong.  Two threads with eight arenas appear to
>> trigger the issue reliably, but I want to test on a larger machine as
>> well, to make sure the test is any good there.  The problem there is
>> that I wasn't
> … able to find sufficiently large box.  That has now changed, and I can
> confirm that the bug still reproduces with the changed test.
>
> Chris, can you confirm that the tweaked test addresses your problem?
> Then I'll send a real patch for review.

Yes, with your patch the test passes, in just over 5 seconds of runtime.

Thanks!
Florian Weimer Jan. 25, 2016, 11:05 a.m. UTC | #3 
On 01/23/2016 03:59 AM, Chris Metcalf wrote:
> On 1/22/2016 2:00 PM, Florian Weimer wrote:
>> On 01/22/2016 06:34 PM, Florian Weimer wrote:
>>> I haven't finished the mallopt approach.  It seems that my mental model
>>> how the test works is wrong.  Two threads with eight arenas appear to
>>> trigger the issue reliably, but I want to test on a larger machine as
>>> well, to make sure the test is any good there.  The problem there is
>>> that I wasn't
>> … able to find sufficiently large box.  That has now changed, and I can
>> confirm that the bug still reproduces with the changed test.
>>
>> Chris, can you confirm that the tweaked test addresses your problem?
>> Then I'll send a real patch for review.
> 
> Yes, with your patch the test passes, in just over 5 seconds of runtime.

Did you try this patch with the fix backed out?  Does it still fail?

Thanks,
Florian
Chris Metcalf Jan. 25, 2016, 6:10 p.m. UTC | #4 
On 01/25/2016 06:05 AM, Florian Weimer wrote:
> On 01/23/2016 03:59 AM, Chris Metcalf wrote:
>> On 1/22/2016 2:00 PM, Florian Weimer wrote:
>>> On 01/22/2016 06:34 PM, Florian Weimer wrote:
>>>> I haven't finished the mallopt approach.  It seems that my mental model
>>>> how the test works is wrong.  Two threads with eight arenas appear to
>>>> trigger the issue reliably, but I want to test on a larger machine as
>>>> well, to make sure the test is any good there.  The problem there is
>>>> that I wasn't
>>> … able to find sufficiently large box.  That has now changed, and I can
>>> confirm that the bug still reproduces with the changed test.
>>>
>>> Chris, can you confirm that the tweaked test addresses your problem?
>>> Then I'll send a real patch for review.
>> Yes, with your patch the test passes, in just over 5 seconds of runtime.
> Did you try this patch with the fix backed out?  Does it still fail?

I reverted your change to malloc/arena.c and the test failed with an abort.
diff mbox

Patch

diff --git a/malloc/tst-malloc-thread-exit.c b/malloc/tst-malloc-thread-exit.c
index f4aa21a..e1c8972 100644
--- a/malloc/tst-malloc-thread-exit.c
+++ b/malloc/tst-malloc-thread-exit.c
@@ -26,6 +26,7 @@ 
    particularly related to the arena free list.  */
 
 #include <errno.h>
+#include <malloc.h>
 #include <pthread.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -153,19 +154,18 @@  outer_thread (void *closure)
   return NULL;
 }
 
+
 static int
 do_test (void)
 {
-  /* The number of top-level threads should be equal to the number of
-     arenas.  See arena_get2.  */
-  long outer_thread_count = sysconf (_SC_NPROCESSORS_ONLN);
-  if (outer_thread_count >= 1)
+  /* The number of threads should be smaller than the number of
+     arenas, so that there will be some free arenas to add to the
+     arena free list.  */
+  enum { outer_thread_count = 2 };
+  if (mallopt (M_ARENA_MAX, 8) == 0)
     {
-      /* See NARENAS_FROM_NCORES in malloc.c.  */
-      if (sizeof (long) == 4)
-        outer_thread_count *= 2;
-      else
-        outer_thread_count *= 8;
+      printf ("error: mallopt (M_ARENA_MAX) failed\n");
+      return 1;
     }
 
   /* Leave some room for shutting down all threads gracefully.  */