From owner-p4-projects Sat May 4 4: 6:53 2002 Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 0C28537B41D; Sat, 4 May 2002 04:06:44 -0700 (PDT) Delivered-To: perforce@freebsd.org Received: from 12-234-96-171.client.attbi.com (12-234-96-171.client.attbi.com [12.234.96.171]) by hub.freebsd.org (Postfix) with ESMTP id EF53637B41C for ; Sat, 4 May 2002 04:06:42 -0700 (PDT) Received: by 12-234-96-171.client.attbi.com (Postfix, from userid 1000) id 77AFEA900; Sat, 4 May 2002 04:10:23 -0700 (PDT) Date: Sat, 4 May 2002 04:10:23 -0700 From: Jonathan Mini To: Perforce Change Reviews Subject: Re: PERFORCE change 10778 for review Message-ID: <20020504041023.N81190@stylus.haikugeek.com> References: <200205041059.g44Axfc02100@freefall.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <200205041059.g44Axfc02100@freefall.freebsd.org>; from mini@freebsd.org on Sat, May 04, 2002 at 03:59:41AM -0700 Sender: owner-p4-projects@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG With this change, the diff for kern_fork.c should integrate without handholding now. Also, as of this change, the KSE branch once again makes it to userland. The ksetest program works again, although I am still often seeing page faults in various areas of the KSE code. Jonathan Mini [mini@freebsd.org] wrote : > http://people.freebsd.org/~peter/p4db/chv.cgi?CH=10778 > > Change 10778 by mini@mini_stylus on 2002/05/04 03:59:35 > > - Obtain PROC_LOCK when funneling through thread_single() and > thread_single_end(). > - For now, we know that thread_alloc() always succeeds, so don't > check for failure and abort (the abort code was incorrect anyways). > - Add jhb's race comment, although its not entirely accurate. > > Affected files ... > > ... //depot/projects/kse/sys/kern/kern_fork.c#65 edit > > Differences ... > > ==== //depot/projects/kse/sys/kern/kern_fork.c#65 (text+ko) ==== > > @@ -289,10 +289,13 @@ > * be aborted in the child. > * (it is possible we could restart them there as well!) > */ > + PROC_LOCK(p1); > if (thread_single(SNGLE_WAIT)) { > /* abort.. someone else is single threading before us */ > + PROC_UNLOCK(p1); > return (ERESTART); > } > + PROC_UNLOCK(p1); > /* > * All other activity in this process > * is now suspended at the user boundary, > @@ -315,9 +318,12 @@ > if ((nprocs >= maxproc - 10 && uid != 0) || nprocs >= maxproc) { > sx_xunlock(&allproc_lock); > uma_zfree(proc_zone, newproc); > + if (p1->p_flag & P_KSES) { > + PROC_LOCK(p1); > + thread_single_end(); > + PROC_UNLOCK(p1); > + } > tsleep(&forksleep, PUSER, "fork", hz / 2); > - if (p1->p_flag & P_KSES) > - thread_single_end(); > return (EAGAIN); > } > /* > @@ -332,8 +338,11 @@ > sx_xunlock(&allproc_lock); > uma_zfree(proc_zone, newproc); > tsleep(&forksleep, PUSER, "fork", hz / 2); > - if (p1->p_flag & P_KSES) > + if (p1->p_flag & P_KSES) { > + PROC_LOCK(p1); > thread_single_end(); > + PROC_UNLOCK(p1); > + } > return (EAGAIN); > } > > @@ -459,14 +468,6 @@ > * then copy the section that is copied directly from the parent. > */ > td2 = thread_alloc(); > - if (td2 == NULL) { > - /* XXX need to take out of pid hash I think */ > - uma_zfree(proc_zone, p2); > - nprocs--; > - if (p1->p_flag & P_KSES) > - thread_single_end(); > - return (EAGAIN); > - } > ke2 = &p2->p_kse; > kg2 = &p2->p_ksegrp; > > @@ -670,6 +671,19 @@ > PROC_UNLOCK(p2); > sx_xunlock(&proctree_lock); > > + /* > + * XXXKSE: In KSE, there would be a race here if one thread was > + * dieing due to a signal (or calling exit1() for that matter) while > + * another thread was calling fork1(). Not sure how KSE wants to work > + * around that. The problem is that up until the point above, if p1 > + * gets killed, it won't find p2 in its list in order for it to be > + * reparented. Alternatively, we could add a new p_flag that gets set > + * before we reparent all the children that we check above and just > + * use init as our parent if that if that flag is set. (Either that > + * or abort the fork if the flag is set since our parent died trying > + * to fork us (which is evil)). > + */ > + > KASSERT(newprocsig == NULL, ("unused newprocsig")); > if (newsigacts != NULL) > FREE(newsigacts, M_SUBPROC); -- Jonathan Mini http://www.haikugeek.com "He who is not aware of his ignorance will be only misled by his knowledge." -- Richard Whatley To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe p4-projects" in the body of the message