Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 12 Mar 2007 10:17:25 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        LI Xin <delphij@delphij.net>
Cc:        MingyanGuo <guomingyan@gmail.com>, freebsd-arch@freebsd.org
Subject:   Re: locking reasoning within fork1()
Message-ID:  <200703121017.25782.jhb@freebsd.org>
In-Reply-To: <45F2C2CB.5000204@delphij.net>
References:  <45F2C2CB.5000204@delphij.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday 10 March 2007 09:38, LI Xin wrote:
> Hi,
> 
> During the AsiaBSDCon DevSummit we have go through the current KSE and
> some userland threading code, and I think that brings me back to the
> fork1() vs others races.
> 
> The current logic, especially the locking order found in fork1() looks
> not very ideal according to my read.  I have pursued some code from
> other BSDs, and I think we might want to address the following problems:
> 
>  - At which point we should consider that a process really exists?  At
> this point, there is no clear point that we can call a process as
> "really born".  It looks to me that PRS_NEW just indicate that a process
> is not "fully initialized", but it does not provide information about
> "how much initialization did we done".  This would make several
> operation very questionable, and is more error-prone.  As Guo (cc'ed)
> pointed out, there are chances that kill(0, ..) and kill(-1, ..) would
> not cover PRS_NEW processes, there might be also some other places where
> should take care of.

This is why I had advocated using a sleep so that consumers either ignore 
PRS_NEW processes or wait until they are completely initalized and 
PRS_NORMAL.

>  - The locking scheme does not look pretty.  We grab and release locks
> again and again, and it might be more optimal to collapse some work
> together, and re-consider synchornization with other parts of the kernel.

To a large extent this reorganization has already been done where possible.

>  - Certain parts of struct proc is mostly not accessed frequently.  For
> the sake of better exploit of cache, we may want to consider to move
> certain parts out from the struct.

You mean to the bottom of the struct maybe?  I'm not sure the overhead of 
having separately allocated structures and extra pointer indirections will do 
anything but hurt.

>  - The PID allocation is somewhat expensive when there are a lot of
> processes.  This might not be a very big deal, though, but given that it
> requires to hold a sx_xlock, our scalability could be limited due to
> this.  tjr@ has a proposed hash based PID allocation patch in his p4
> branch, and NetBSD have an O(1) algorithm that may worth to have a look at.

This has been brought up before, and when tjr's stuff was tested it didn't 
help IIRC.  Part of the issue here is that pid space is not just a simple 
walk of processes, but also of process groups and sessions.  If you didn't 
want to walk all the data structures you'd have to have some sort of PID 
reference counting for the 3 possible references on a pid: process, pgrp, and 
session.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200703121017.25782.jhb>