From owner-freebsd-threads@FreeBSD.ORG Thu Mar 3 09:26:29 2005 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C5EAA16A4CE; Thu, 3 Mar 2005 09:26:29 +0000 (GMT) Received: from mail.emict.com (brig.emict.com [212.90.172.42]) by mx1.FreeBSD.org (Postfix) with ESMTP id D663443D46; Thu, 3 Mar 2005 09:26:28 +0000 (GMT) (envelope-from andrit@ukr.net) Received: from BORJA (unknown [203.199.120.221]) by mail.emict.com (Postfix) with ESMTP id 198293394D; Thu, 3 Mar 2005 11:26:25 +0200 (EET) Message-ID: <012901c51fd3$131e23b0$090210ac@BORJA> From: "Andriy Tkachuk" To: "David Xu" Date: Thu, 3 Mar 2005 14:56:21 +0530 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0126_01C52001.298C64D0" X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 6.00.2800.1158 x-mimeole: Produced By Microsoft MimeOLE V6.00.2800.1165 cc: threads@freebsd.org Subject: Re: patch for threads/76690 - critical - fork hang in child for-lc_r X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Mar 2005 09:26:29 -0000 This is a multi-part message in MIME format. ------=_NextPart_000_0126_01C52001.298C64D0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit > > > Hmm, libc_r and libpthread handle spinlock differently which malloc > > > uses to protect itself, some real world benchmarks are better than > this. yes , you right, David. one have to check __isthreaded before firing _SPINLOCK. there will be nothing wrong, because static spinlock_t thread_lock = _SPINLOCK_INITIALIZER; initialyzed regardless __isthreaded in malloc.c but for optimization probably it is worth to add this check. Take a look on updated patch. btw: i don't see the unlock in child in libpthread. there must be two unlocks - in child & in parent, doesn't it? : # grep __malloc_lock -r libpthread libpthread/thread/thr_fork.c: _spinlock(__malloc_lock); libpthread/thread/thr_fork.c: if ((_kse_isthreaded() != 0) && (__malloc_lock != NULL)) { libpthread/thread/thr_fork.c: _spinunlock(__malloc_lock); # ------=_NextPart_000_0126_01C52001.298C64D0 Content-Type: application/octet-stream; name="libc_r-76690pr.patch2" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="libc_r-76690pr.patch2" diff -r -u lib/libc_r/uthread/uthread_fork.c = lib.patched/libc_r/uthread/uthread_fork.c=0A= --- lib/libc_r/uthread/uthread_fork.c Fri Dec 10 09:06:45 2004=0A= +++ lib.patched/libc_r/uthread/uthread_fork.c Thu Mar 3 14:50:06 2005=0A= @@ -68,8 +68,15 @@=0A= af->prepare();=0A= }=0A= =0A= + extern spinlock_t *__malloc_lock;=0A= + if (__isthreaded && __malloc_lock !=3D NULL)=0A= + _SPINLOCK(__malloc_lock);=0A= +=0A= /* Fork a new process: */=0A= if ((ret =3D __sys_fork()) !=3D 0) {=0A= + if (__isthreaded && __malloc_lock !=3D NULL)=0A= + _SPINUNLOCK(__malloc_lock);=0A= +=0A= /* Run down atfork parent handlers. */=0A= TAILQ_FOREACH(af, &_atfork_list, qe) {=0A= if (af->parent !=3D NULL)=0A= @@ -78,6 +85,9 @@=0A= _pthread_mutex_unlock(&_atfork_mutex);=0A= =0A= } else {=0A= + if (__isthreaded && __malloc_lock !=3D NULL)=0A= + _SPINUNLOCK(__malloc_lock);=0A= +=0A= /* Close the pthread kernel pipe: */=0A= __sys_close(_thread_kern_pipe[0]);=0A= __sys_close(_thread_kern_pipe[1]);=0A= ------=_NextPart_000_0126_01C52001.298C64D0--