Patch for review: resolve a race condition in [sg]etpriority()

LI Xin delphij at delphij.net
Thu Feb 22 22:04:13 UTC 2007


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

John Baldwin wrote:
> On Thursday 22 February 2007 12:51, LI Xin wrote:
>> Hi, John,
>>
>> John Baldwin wrote:
>>> My only reason for favoring the wakeup for complete initialization is that
>>> while this patch may solve the getprio/setprio race, it doesn't solve all
>>> PRS_NEW-related races, which the sleep/wakeup proposal did.
>> Today I have some time and tried your approach for a second time.  It
>> looks like that we can not simply sleep with allproc_lock held.  The
>> attached patchset implements the proof-of-concept idea, please let me
>> know if you think this one is better.
> 
> Ok.  It would actually be really nice if we could not put the process onto
> the allproc list until it was really fully created.  Is the only reason we put 
> it on the list to prevent duplicate pid allocation?

I tend to disagree with that.  Here is a scenario that makes this
incorrect for setpriority, somewhat like what I have discussed with
Robert earlier:

A			B
(start forking)
(copy p2 from p1)
			(setpriority in PRIO_USER case)
<--user identical, but invisible to B
(insert p2 into allproc)

We end up with p2's priority unset, while p1's set.  I think this is not
what we want.  If this is acceptable, then the easiest solution would be
just skip PRS_NEW processes :-)

In order to make setpriority() correct, it seems that we should have B
either set p2's priority (initialized to a state where B can see it and
operate without problem), or have a not-copied p2 if B can not see it,
or at least block when the copy is in progress.

Cheers,
- --
Xin LI <delphij at delphij.net>	http://www.delphij.net/
FreeBSD - The Power to Serve!
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF3hNROfuToMruuMARCly/AJ9a5I8JPTUsIJBi57nkNbllSVUXGgCeIqV0
YKm7guWpFkiFe7Bxvs/Xvck=
=9NKM
-----END PGP SIGNATURE-----


More information about the freebsd-arch mailing list