From owner-dev-commits-src-all@freebsd.org Tue Jan 12 11:38:47 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 087784D43C6; Tue, 12 Jan 2021 11:38:47 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DFT9f5WFsz4qWp; Tue, 12 Jan 2021 11:38:46 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.16.1/8.16.1) with ESMTPS id 10CBcbXr004114 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Tue, 12 Jan 2021 13:38:41 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 10CBcbXr004114 Received: (from kostik@localhost) by tom.home (8.16.1/8.16.1/Submit) id 10CBcbRX004113; Tue, 12 Jan 2021 13:38:37 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 12 Jan 2021 13:38:36 +0200 From: Konstantin Belousov To: Alexander Richardson Cc: src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 85d028223bc2 - main - libthr malloc: support recursion on thr_malloc_umtx. Message-ID: References: <202101121046.10CAkLxI018198@gitrepo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on tom.home X-Rspamd-Queue-Id: 4DFT9f5WFsz4qWp X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jan 2021 11:38:47 -0000 On Tue, Jan 12, 2021 at 11:16:25AM +0000, Alexander Richardson wrote: > On Tue, 12 Jan 2021 at 10:46, Konstantin Belousov wrote: > > > > The branch main has been updated by kib: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=85d028223bc2768651f4d44881644ceb5dc2a664 > > > > commit 85d028223bc2768651f4d44881644ceb5dc2a664 > > Author: Konstantin Belousov > > AuthorDate: 2021-01-12 09:02:37 +0000 > > Commit: Konstantin Belousov > > CommitDate: 2021-01-12 10:45:44 +0000 > > > > libthr malloc: support recursion on thr_malloc_umtx. > > > > One possible way the recursion can happen is during fork: suppose > > that fork is called from early code that did not triggered > > jemalloc(3) initialization yet. Then we lock thr_malloc lock, and > > call malloc_prefork() that might require initialization of jemalloc > > pthread_mutexes, calling into libthr malloc. It is safe to allow > > recursion for this occurence. > > > > PR: 252579 > > Reported by: Vasily Postnicov > > MFC after: 1 week > > Sponsored by: The FreeBSD Foundation > > --- > > lib/libthr/thread/thr_malloc.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/lib/libthr/thread/thr_malloc.c b/lib/libthr/thread/thr_malloc.c > > index 2f1c81142942..80a81f9c6c27 100644 > > --- a/lib/libthr/thread/thr_malloc.c > > +++ b/lib/libthr/thread/thr_malloc.c > > @@ -41,6 +41,7 @@ int npagesizes; > > size_t *pagesizes; > > static size_t pagesizes_d[2]; > > static struct umutex thr_malloc_umtx; > > +static u_int thr_malloc_umtx_level; > > > > void > > __thr_malloc_init(void) > > @@ -60,11 +61,16 @@ __thr_malloc_init(void) > > static void > > thr_malloc_lock(struct pthread *curthread) > > { > > + uint32_t curtid; > > > > if (curthread == NULL) > > return; > > curthread->locklevel++; > > - _thr_umutex_lock(&thr_malloc_umtx, TID(curthread)); > > + curtid = TID(curthread); > > + if ((uint32_t)thr_malloc_umtx.m_owner == curtid) > > + thr_malloc_umtx_level++; > > + else > > + _thr_umutex_lock(&thr_malloc_umtx, curtid); > > } > > > > static void > > @@ -73,7 +79,10 @@ thr_malloc_unlock(struct pthread *curthread) > > > > if (curthread == NULL) > > return; > > - _thr_umutex_unlock(&thr_malloc_umtx, TID(curthread)); > > + if (thr_malloc_umtx_level > 0) > > + thr_malloc_umtx_level--; > > Should this also assert that "thr_malloc_umtx.m_owner == curtid"? I think a generic principle for userspace libraries is to not abort the application. Ideally we would return some error indicating memory corruption, but libthr is not set up for this. There are some asserts in libthr that are currently always compiled in. I wanted to make libthr in stable branches to avoid any assertions, but did not handled that.