| Message ID | 5836CC80.9070101@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
On 11/24/2016 12:18 PM, Szabolcs Nagy wrote: > Test concurrent dlopen and pthread_create when the loaded > modules have TLS. This triggers dl-tls assertion failures > more reliably than the tst-stack4 test. > > The dlopened module has 100 DT_NEEDED dependencies and > for me about 4000 concurrent thread creations are needed > to see failures on x86_64. > > 2016-11-24 Szabolcs Nagy <szabolcs.nagy@arm.com> > > [BZ #19329] > * nptl/Makefile (tests): Add tst-tls7. > (modules-names): Add tst-tls7mod, tst-tls7mod-dep. > * nptl/tst-tls7.c: New file. > * nptl/tst-tls7mod-dep.c: New file. > * nptl/tst-tls7mod.c: New file. > Please do not use assert in tests. Use the xpthread_* functions in test-skeleton.c instead. This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there. A less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each time, with a smaller number of threads created in parallel. Florian
On 24/11/16 11:40, Florian Weimer wrote: > On 11/24/2016 12:18 PM, Szabolcs Nagy wrote: >> Test concurrent dlopen and pthread_create when the loaded >> modules have TLS. This triggers dl-tls assertion failures >> more reliably than the tst-stack4 test. >> >> The dlopened module has 100 DT_NEEDED dependencies and >> for me about 4000 concurrent thread creations are needed >> to see failures on x86_64. >> >> 2016-11-24 Szabolcs Nagy <szabolcs.nagy@arm.com> >> >> [BZ #19329] >> * nptl/Makefile (tests): Add tst-tls7. >> (modules-names): Add tst-tls7mod, tst-tls7mod-dep. >> * nptl/tst-tls7.c: New file. >> * nptl/tst-tls7mod-dep.c: New file. >> * nptl/tst-tls7mod.c: New file. >> > > Please do not use assert in tests. Use the xpthread_* functions in test-skeleton.c instead. > i can do that but it looks ugly. we are looking for an assertion failure anyway so it would be better to make the test system deal with that.. > This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there. A > less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each > time, with a smaller number of threads created in parallel. > i can try, but iterating is not easy: there is dynamic linker internal state that cannot be reset (generation counter, max dtv idx, allocated slotininfo entries) so i'd need to start a new process for every iteration.
On 11/24/2016 01:31 PM, Szabolcs Nagy wrote: >> Please do not use assert in tests. Use the xpthread_* functions in test-skeleton.c instead. >> > > i can do that but it looks ugly. Well. The asserts are incorrect because you rely on their side effects. >> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there. A >> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each >> time, with a smaller number of threads created in parallel. >> > > i can try, but iterating is not easy: there > is dynamic linker internal state that cannot > be reset (generation counter, max dtv idx, > allocated slotininfo entries) so i'd need to > start a new process for every iteration. Can you use fork before each round? Thanks, Florian
On 24/11/16 12:34, Florian Weimer wrote: > On 11/24/2016 01:31 PM, Szabolcs Nagy wrote: > >>> Please do not use assert in tests. Use the xpthread_* functions in test-skeleton.c instead. >>> >> >> i can do that but it looks ugly. > > Well. The asserts are incorrect because you rely on their side effects. > ah i thought you are worried that the error output does not end up in the test .out i can undef NDEBUG before assert.h >>> This test will fail on large SMP machines due to timeouts because thread creation is rather expensive there. A >>> less resource-intense way of writing this test would be to iterate multiple times and try to hit the race each >>> time, with a smaller number of threads created in parallel. >>> >> >> i can try, but iterating is not easy: there >> is dynamic linker internal state that cannot >> be reset (generation counter, max dtv idx, >> allocated slotininfo entries) so i'd need to >> start a new process for every iteration. > > Can you use fork before each round? > ok, i'll try that.
On Thu, 24 Nov 2016, Szabolcs Nagy wrote: > Test concurrent dlopen and pthread_create when the loaded > modules have TLS. This triggers dl-tls assertion failures > more reliably than the tst-stack4 test. > > The dlopened module has 100 DT_NEEDED dependencies and > for me about 4000 concurrent thread creations are needed > to see failures on x86_64. I'd be concerned about 4000 threads exceeding task or memory limits, possibly interfering with other things the same user is doing that also try to create tasks. (I rountinely see tst-eintr1 failing with "pthread_create failed: Resource temporarily unavailable", sometimes terminating the test run because make fails to fork to run evaluate-test.sh, apparently not all threads having properly terminated by the time the test does. I don't know how many threads that is creating.) -- Joseph S. Myers joseph@codesourcery.com
On 11/24/2016 02:41 PM, Joseph Myers wrote: > On Thu, 24 Nov 2016, Szabolcs Nagy wrote: > >> Test concurrent dlopen and pthread_create when the loaded >> modules have TLS. This triggers dl-tls assertion failures >> more reliably than the tst-stack4 test. >> >> The dlopened module has 100 DT_NEEDED dependencies and >> for me about 4000 concurrent thread creations are needed >> to see failures on x86_64. > > I'd be concerned about 4000 threads exceeding task or memory limits, > possibly interfering with other things the same user is doing that also > try to create tasks. Yes, reducing the number of threads which are created in parallel is strongly recommended. > (I rountinely see tst-eintr1 failing with "pthread_create failed: Resource > temporarily unavailable", sometimes terminating the test run because make > fails to fork to run evaluate-test.sh, apparently not all threads having > properly terminated by the time the test does. I don't know how many > threads that is creating.) As far as I can tell, tst-eintr1 creates only twenty threads at most: ten outer threads, and each outer thread creates one inner thread. It's likely you are running into a kernel bug, where task exit is reported before the kernel has deallocated the resources used by the task: https://bugzilla.kernel.org/show_bug.cgi?id=154011 Florian
diff --git a/nptl/Makefile b/nptl/Makefile index 11588fe..90152d2 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -327,7 +327,7 @@ tests += tst-cancelx2 tst-cancelx3 tst-cancelx4 tst-cancelx5 \ tst-oncex3 tst-oncex4 ifeq ($(build-shared),yes) tests += tst-atfork2 tst-tls3 tst-tls3-malloc tst-tls4 tst-tls5 tst-_res1 \ - tst-fini1 tst-stackguard1 + tst-fini1 tst-stackguard1 tst-tls7 tests-nolibpthread += tst-fini1 ifeq ($(have-z-execstack),yes) tests += tst-execstack @@ -338,7 +338,7 @@ modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \ tst-tls5mod tst-tls5moda tst-tls5modb tst-tls5modc \ tst-tls5modd tst-tls5mode tst-tls5modf tst-stack4mod \ tst-_res1mod1 tst-_res1mod2 tst-execstack-mod tst-fini1mod \ - tst-join7mod + tst-join7mod tst-tls7mod tst-tls7mod-dep extra-test-objs += $(addsuffix .os,$(strip $(modules-names))) \ tst-cleanup4aux.o tst-cleanupx4aux.o test-extras += $(modules-names) tst-cleanup4aux tst-cleanupx4aux @@ -545,6 +545,18 @@ $(objpfx)tst-tls5: $(objpfx)tst-tls5mod.so $(shared-thread-library) LDFLAGS-tst-tls5 = $(no-as-needed) LDFLAGS-tst-tls5mod.so = -Wl,-soname,tst-tls5mod.so +$(objpfx)tst-tls7: $(libdl) $(shared-thread-library) +tst-tls7mod-deps = $(shell for i in 0 1 2 3 4 5 6 7 8 9; do \ + for j in 0 1 2 3 4 5 6 7 8 9; do \ + echo $(objpfx)tst-tls7mod-dep-$$i-$$j.so; \ + done; done) +$(objpfx)tst-tls7.out: $(objpfx)tst-tls7mod.so +$(objpfx)tst-tls7mod.so: $(tst-tls7mod-deps) +$(tst-tls7mod-deps): $(objpfx)tst-tls7mod-dep.so + cp -f $< $@ +clean: + rm -f $(tst-tls7mod-deps) + ifeq ($(build-shared),yes) $(objpfx)tst-tls6.out: tst-tls6.sh $(objpfx)tst-tls5 \ $(objpfx)tst-tls5moda.so $(objpfx)tst-tls5modb.so \ diff --git a/nptl/tst-tls7.c b/nptl/tst-tls7.c new file mode 100644 index 0000000..e4c7db2 --- /dev/null +++ b/nptl/tst-tls7.c @@ -0,0 +1,62 @@ +/* Test concurrent dlopen and pthread_create: BZ 19329. + Copyright (C) 2016 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <assert.h> +#include <dlfcn.h> +#include <pthread.h> +#include <limits.h> + +#define THREADS 4000 + +static void * +start (void *a) +{ + assert (dlopen ("tst-tls7mod.so", RTLD_LAZY)); + return 0; +} + +static void * +nop (void *a) +{ + return 0; +} + +static int +do_test (void) +{ + pthread_t td; + pthread_attr_t attr; + + assert (pthread_attr_init (&attr) == 0); + assert (pthread_attr_setstacksize (&attr, PTHREAD_STACK_MIN) == 0); + + /* Load a module with lots of dependencies and TLS. */ + assert (pthread_create (&td, 0, start, 0) == 0); + + /* Concurrently create lots of threads. */ + for (int i = 0; i < THREADS; i++) + assert (pthread_create (&(pthread_t){0}, &attr, nop, 0) == 0); + + /* Wait for the loading to finish, ignore other threads. */ + assert (pthread_join (td, 0) == 0); + + return 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/nptl/tst-tls7mod-dep.c b/nptl/tst-tls7mod-dep.c new file mode 100644 index 0000000..206ece4 --- /dev/null +++ b/nptl/tst-tls7mod-dep.c @@ -0,0 +1 @@ +int __thread x; diff --git a/nptl/tst-tls7mod.c b/nptl/tst-tls7mod.c new file mode 100644 index 0000000..206ece4 --- /dev/null +++ b/nptl/tst-tls7mod.c @@ -0,0 +1 @@ +int __thread x;