Date: Tue, 12 Jan 2021 13:38:36 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Alexander Richardson <arichardson@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, 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: <X/2KPCMM75mhoay%2B@kib.kiev.ua> In-Reply-To: <CA%2BZ_v8qh6%2BP5x88ziXQS3q90%2BG-gFTDVgO%2B7bV3RmGYakLVkDw@mail.gmail.com> References: <202101121046.10CAkLxI018198@gitrepo.freebsd.org> <CA%2BZ_v8qh6%2BP5x88ziXQS3q90%2BG-gFTDVgO%2B7bV3RmGYakLVkDw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Jan 12, 2021 at 11:16:25AM +0000, Alexander Richardson wrote: > On Tue, 12 Jan 2021 at 10:46, Konstantin Belousov <kib@freebsd.org> wrote: > > > > The branch main has been updated by kib: > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=85d028223bc2768651f4d44881644ceb5dc2a664 > > > > commit 85d028223bc2768651f4d44881644ceb5dc2a664 > > Author: Konstantin Belousov <kib@FreeBSD.org> > > AuthorDate: 2021-01-12 09:02:37 +0000 > > Commit: Konstantin Belousov <kib@FreeBSD.org> > > 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 <shamaz.mazum@gmail.com> > > 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?X/2KPCMM75mhoay%2B>