Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 30 Jan 2008 11:22:51 -0800
From:      Landon Fuller <landonf@bikemonkey.org>
To:        Daniel Eischen <eischen@vigrid.com>
Cc:        freebsd-threads@freebsd.org
Subject:   Re: threads/119920: fork broken in libpthread
Message-ID:  <D3DFD2DE-E6A3-4373-AC37-42C3CAB9B963@bikemonkey.org>
In-Reply-To: <Pine.GSO.4.64.0801292320260.15096@sea.ntplx.net>
References:  <200801240850.m0O8o2JQ023500@freefall.freebsd.org> <4798564B.7070500@elischer.org> <Pine.GSO.4.64.0801240957550.16059@sea.ntplx.net> <488DBC6A-CF33-4E50-B1BB-C396C8957F92@bikemonkey.org> <Pine.GSO.4.64.0801291611130.12689@sea.ntplx.net> <892A73B3-0114-4718-ABC0-CADD45D9D0FA@bikemonkey.org> <Pine.GSO.4.64.0801292320260.15096@sea.ntplx.net>

next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--Apple-Mail-14-75471841
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed


On Jan 29, 2008, at 20:23, Daniel Eischen wrote:

> On Tue, 29 Jan 2008, Landon Fuller wrote:
>
>>
>> On Jan 29, 2008, at 13:13, Daniel Eischen wrote:
>>> There is a bug somewhere else or something is stomping
>>> on the thread's lockuser.  It is allocated once when the
>>> thread is created and should never be null thereafter.
>>> Hence, it should never be malloc'd and the reinit should
>>> be sufficient.
>>
>> I'm totally unfamiliar with KSE, so perhaps this a stupid question  
>> -- it seems to solve the issue locally, so I'll ask it --
>> Why not place the fork() code inside of _kse_critical_enter /  
>> _kse_critical_leave, to ensure upcalls are blocked while re- 
>> initializing in the child process post-fork?
>
> That just prevents an upcall from happening (which my patch solves),
> but doesn't prevent the corruption of the lockuser or lock.

My assumption was that lockuser was being corrupted in a post-fork  
upcall; after wrapping fork in kse_critical_enter(), and can not  
reproduce the lockuser-reallocation I was seeing before.

Is my assumption that kse_critical_enter() prevents any other code  
from being run during the critical section incorrect?

--- thread/thr_fork.c.orig      2008-01-30 11:08:45.000000000 -0800
+++ thread/thr_fork.c   2008-01-30 11:09:36.000000000 -0800
@@ -93,12 +93,16 @@
         if (_kse_isthreaded() != 0) {
                 _spinlock(__malloc_lock);
         }
+
+       kse_critical_t crit = _kse_critical_enter();
+
         if ((ret = __sys_fork()) == 0) {
                 /* Child process */
                 errsave = errno;

                 /* Kernel signal mask is restored in  
_kse_single_thread */
                 _kse_single_thread(curthread);
+               _kse_critical_leave(crit);

                 /* Run down atfork child handlers. */
                 TAILQ_FOREACH(af, &_thr_atfork_list, qe) {
@@ -107,6 +111,7 @@
                 }
                 _thr_mutex_reinit(&_thr_atfork_mutex);
         } else {
+               _kse_critical_leave(crit);
                 if ((_kse_isthreaded() != 0) && (__malloc_lock !=  
NULL)) {
                         _spinunlock(__malloc_lock);
                 }

> My patch does solve this in -current, but -stable probably lacks
> a few other patches.  It (-stable) really needs all of -current's
> code, not just this patch.


Is this change viable for an errata fix for 6.3?

-landonf

--Apple-Mail-14-75471841
content-type: application/pgp-signature; x-mac-type=70674453;
	name=PGP.sig
content-description: This is a digitally signed message part
content-disposition: inline; filename=PGP.sig
content-transfer-encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iD8DBQFHoM6NlplZCE/15mMRAiL0AJ44TC7y1TPRcETD6w5aXo+/bSlPtACffYQ0
hwOy02U/Y5KBmHcX4UuYMB4=
=6kkq
-----END PGP SIGNATURE-----

--Apple-Mail-14-75471841--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?D3DFD2DE-E6A3-4373-AC37-42C3CAB9B963>