Date: Mon, 4 Feb 2019 09:10:17 +0100 From: Michal Meloun <meloun.michal@gmail.com> To: Konstantin Belousov <kib@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r343566 - in head/lib/libthr: . thread Message-ID: <420b91ef-b318-100b-a0b3-99f29d03cea8@freebsd.org> In-Reply-To: <20190204070017.GO24863@kib.kiev.ua> References: <201901292246.x0TMkjQH074121@repo.freebsd.org> <c5f22e60-53fd-3677-9067-dd597218935c@freebsd.org> <20190204070017.GO24863@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------98DC485539ACAB25B79005DB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 04.02.2019 8:00, Konstantin Belousov wrote: > On Mon, Feb 04, 2019 at 07:31:19AM +0100, Michal Meloun wrote: >> On 29.01.2019 23:46, Konstantin Belousov wrote: >>> Author: kib >>> Date: Tue Jan 29 22:46:44 2019 >>> New Revision: 343566 >>> URL: https://svnweb.freebsd.org/changeset/base/343566 >>> >>> Log: >>> Untangle jemalloc and mutexes initialization. >>> >>> The need to use libc malloc(3) from some places in libthr always >>> caused issues. For instance, per-thread key allocation was switched to >>> use plain mmap(2) to get storage, because some third party mallocs >>> used keys for implementation of calloc(3). >>> >>> Even more important, libthr calls calloc(3) during initialization of >>> pthread mutexes, and jemalloc uses pthread mutexes. Jemalloc provides >>> some way to both postpone the initialization, and to make >>> initialization to use specialized allocator, but this is very fragile >>> and often breaks. See the referenced PR for another example. >>> >>> Add the small malloc implementation used by rtld, to libthr. Use it in >>> thr_spec.c and for mutexes initialization. This avoids the issues with >>> mutual dependencies between malloc and libthr in principle. The >>> drawback is that some more allocations are not interceptable for >>> alternate malloc implementations. There should be not too much memory >>> use from this allocator, and the alternative, direct use of mmap(2) is >>> obviously worse. >>> >>> PR: 235211 >>> MFC after: 2 weeks >>> Sponsored by: The FreeBSD Foundation >>> Differential revision: https://reviews.freebsd.org/D18988 >>> >> This broke ARM static binaries (at least rescue/rescue). From first >> look it seems that __pthread_mutex_init() invoked by atomic_init() is >> called before curthread is set (before _thread_init_hack()). >> >> >> root@tegra124:/usr/src # gdb --args >> /usr/obj/usr/src/arm.armv7/rescue/rescue/rescue sh >> ... >> Reading symbols from /usr/obj/usr/src/arm.armv7/rescue/rescue/rescue...done. >> (gdb) b _thread_init_hack >> Breakpoint 1 at 0x67cad0: file /usr/src/lib/libthr/thread/thr_init.c, >> line 296. >> (gdb) r >> Starting program: /usr/obj/usr/src/arm.armv7/rescue/rescue/rescue sh >> >> Program received signal SIGSEGV, Segmentation fault. >> __thr_calloc (num=1, size=<optimized out>) at >> /usr/src/lib/libthr/thread/thr_malloc.c:82 >> 82 thr_malloc_lock(curthread); >> (gdb) bt >> #0 __thr_calloc (num=1, size=<optimized out>) at >> /usr/src/lib/libthr/thread/thr_malloc.c:82 >> #1 0x00676e1c in mutex_init (mutex=<optimized out>, >> mutex_attr=<optimized out>, calloc_cb=<optimized out>) >> at /usr/src/lib/libthr/thread/thr_mutex.c:294 >> #2 __pthread_mutex_init (mutex=0xc6e948 <atomic_mtx>, >> mutex_attr=<optimized out>) at /usr/src/lib/libthr/thread/thr_mutex.c:393 >> #3 0x001d41f0 in handle_static_init (argc=2, argv=<optimized out>, >> env=<optimized out>) at /usr/src/lib/csu/common/ignore_init.c:124 >> #4 0x001d40e8 in __start (argc=2, argv=<optimized out>, env=<optimized >> out>, ps_strings=<optimized out>, obj=0x0, cleanup=0x0) >> at /usr/src/lib/csu/arm/crt1.c:112 >> #5 0x001d4000 in ?? () >> Backtrace stopped: previous frame identical to this frame (corrupt stack?) >> (gdb) f 3 >> #3 0x001d41f0 in handle_static_init (argc=2, argv=<optimized out>, >> env=<optimized out>) at /usr/src/lib/csu/common/ignore_init.c:124 >> 124 fn(argc, argv, env); >> (gdb) list >> 119 _init(); >> 120 array_size = __init_array_end - __init_array_start; >> 121 for (n = 0; n < array_size; n++) { >> 122 fn = __init_array_start[n]; >> 123 if ((uintptr_t)fn != 0 && (uintptr_t)fn != 1) >> 124 fn(argc, argv, env); >> 125 } >> 126 } >> (gdb) p n >> $1 = 20 >> (gdb) p fn >> $6 = (void (*)(int, char **, char **)) 0x63f21c <atomic_init> >> (gdb) p (void *)&__init_array_start >> $19 = (void *) 0xa7ab60 >> (gdb) p (void *)&__init_array_end >> $20 = (void *) 0xa7abc0 >> (gdb) x/24a &__init_array_start >> 0xa7ab60: 0x1d4270 <register_classes> >> 0x2d926c <_$$hide$$ ifconfig.lo ifconfig_ctor> >> 0x2d98e0 <_$$hide$$ ifconfig.lo link_ctor> >> 0x2d9cb4 <_$$hide$$ ifconfig.lo inet_ctor> >> 0xa7ab70: 0x2da2e0 <_$$hide$$ ifconfig.lo inet6_ctor> >> 0x2db790 <_$$hide$$ ifconfig.lo clone_ctor> >> 0x2dbb8c <_$$hide$$ ifconfig.lo mac_ctor> >> 0x2dbe70 <_$$hide$$ ifconfig.lo ifmedia_ctor> >> 0xa7ab80: 0x2dced0 <_$$hide$$ ifconfig.lo fib_ctor> >> 0x2dd118 <_$$hide$$ ifconfig.lo vlan_ctor> >> 0x2dd668 <_$$hide$$ ifconfig.lo vxlan_ctor> >> 0x2df45c <_$$hide$$ ifconfig.lo gre_ctor> >> 0xa7ab90: 0x2df6d8 <_$$hide$$ ifconfig.lo gif_ctor> >> 0x2df874 <_$$hide$$ ifconfig.lo ipsec_ctor> >> 0x2e2144 <_$$hide$$ ifconfig.lo ieee80211_ctor> >> 0x2f0a10 <_$$hide$$ ifconfig.lo carp_ctor> >> 0xa7aba0: 0x2f0e6c <_$$hide$$ ifconfig.lo group_ctor> >> 0x2f199c <_$$hide$$ ifconfig.lo pfsync_ctor> >> 0x2f1a04 <_$$hide$$ ifconfig.lo bridge_ctor> >> 0x2f35dc <_$$hide$$ ifconfig.lo lagg_ctor> >> 0xa7abb0: 0x63f21c <atomic_init> >> 0x67cad0 <_thread_init_hack> >> 0x90e5b4 <OPENSSL_cpuid_setup> >> 0x9a6b98 <jemalloc_constructor> >> >> So it's clear that order of static constructors is invalid (moreover, I >> thing that we are inside undefined area here). >> Any idea/hint how to fix this. > > Try the following (I did not even compiled). It might require additional > handling of NULL curthread in thr_malloc.c, in which case the locking > should be elided. > > diff --git a/lib/libthr/thread/thr_malloc.c b/lib/libthr/thread/thr_malloc.c > index 157c72f10d6..8b72a1840f7 100644 > --- a/lib/libthr/thread/thr_malloc.c > +++ b/lib/libthr/thread/thr_malloc.c > @@ -46,6 +46,8 @@ void > __thr_malloc_init(void) > { > > + if (npagesizes != 0) > + return; > npagesizes = getpagesizes(pagesizes_d, nitems(pagesizes_d)); > if (npagesizes == -1) { > npagesizes = 1; > diff --git a/lib/libthr/thread/thr_mutex.c b/lib/libthr/thread/thr_mutex.c > index f6f37c1264e..4db65384331 100644 > --- a/lib/libthr/thread/thr_mutex.c > +++ b/lib/libthr/thread/thr_mutex.c > @@ -390,6 +390,7 @@ __pthread_mutex_init(pthread_mutex_t * __restrict mutex, > } > if (mutex_attr == NULL || > (*mutex_attr)->m_pshared == PTHREAD_PROCESS_PRIVATE) { > + __thr_malloc_init(); > return (mutex_init(mutex, mutex_attr ? *mutex_attr : NULL, > __thr_calloc)); > } > Thanks for fast response. It works, but yes, additional handling of NULL curthread is required. The following patch fixes this issue for me (tested only on ARMv7). Can you, please, check it and eventually commit it? Thanks, Michal --------------98DC485539ACAB25B79005DB Content-Type: text/plain; charset=UTF-8; name="thread.diff" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="thread.diff" SW5kZXg6IGxpYi9saWJ0aHIvdGhyZWFkL3Rocl9tYWxsb2MuYwo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBsaWIvbGlidGhyL3RocmVhZC90aHJfbWFsbG9jLmMJKHJldmlzaW9uIDM0MzczMikKKysr IGxpYi9saWJ0aHIvdGhyZWFkL3Rocl9tYWxsb2MuYwkod29ya2luZyBjb3B5KQpAQCAtNDUs NiArNDUsOCBAQAogdm9pZAogX190aHJfbWFsbG9jX2luaXQodm9pZCkKIHsKKwlpZiAobnBh Z2VzaXplcyAhPSAwKQorCQlyZXR1cm47CiAKIAlucGFnZXNpemVzID0gZ2V0cGFnZXNpemVz KHBhZ2VzaXplc19kLCBuaXRlbXMocGFnZXNpemVzX2QpKTsKIAlpZiAobnBhZ2VzaXplcyA9 PSAtMSkgewpAQCAtNTksNiArNjEsOCBAQAogdGhyX21hbGxvY19sb2NrKHN0cnVjdCBwdGhy ZWFkICpjdXJ0aHJlYWQpCiB7CiAKKwlpZiAoY3VydGhyZWFkID09IE5VTEwpCisJCXJldHVy bjsKIAljdXJ0aHJlYWQtPmxvY2tsZXZlbCsrOwogCV90aHJfdW11dGV4X2xvY2soJnRocl9t YWxsb2NfdW10eCwgVElEKGN1cnRocmVhZCkpOwogfQpAQCAtNjcsNiArNzEsOCBAQAogdGhy X21hbGxvY191bmxvY2soc3RydWN0IHB0aHJlYWQgKmN1cnRocmVhZCkKIHsKIAorCWlmIChj dXJ0aHJlYWQgPT0gTlVMTCkKKwkJcmV0dXJuOwogCV90aHJfdW11dGV4X3VubG9jaygmdGhy X21hbGxvY191bXR4LCBUSUQoY3VydGhyZWFkKSk7CiAJY3VydGhyZWFkLT5sb2NrbGV2ZWwt LTsKIAlfdGhyX2FzdChjdXJ0aHJlYWQpOwpJbmRleDogbGliL2xpYnRoci90aHJlYWQvdGhy X211dGV4LmMKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gbGliL2xpYnRoci90aHJlYWQvdGhyX211dGV4 LmMJKHJldmlzaW9uIDM0MzczMikKKysrIGxpYi9saWJ0aHIvdGhyZWFkL3Rocl9tdXRleC5j CSh3b3JraW5nIGNvcHkpCkBAIC0zOTAsNiArMzkwLDcgQEAKIAl9CiAJaWYgKG11dGV4X2F0 dHIgPT0gTlVMTCB8fAogCSAgICAoKm11dGV4X2F0dHIpLT5tX3BzaGFyZWQgPT0gUFRIUkVB RF9QUk9DRVNTX1BSSVZBVEUpIHsKKwkJX190aHJfbWFsbG9jX2luaXQoKTsKIAkJcmV0dXJu IChtdXRleF9pbml0KG11dGV4LCBtdXRleF9hdHRyID8gKm11dGV4X2F0dHIgOiBOVUxMLAog CQkgICAgX190aHJfY2FsbG9jKSk7CiAJfQo= --------------98DC485539ACAB25B79005DB--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?420b91ef-b318-100b-a0b3-99f29d03cea8>