Date: Fri, 23 Feb 2007 01:51:18 +0800 From: LI Xin <delphij@delphij.net> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-arch@freebsd.org Subject: Re: Patch for review: resolve a race condition in [sg]etpriority() Message-ID: <45DDD816.80303@delphij.net> In-Reply-To: <200702090837.04495.jhb@freebsd.org> References: <45CC0EB9.7030400@delphij.net> <200702090837.04495.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig7AEAF4ECD6333CE874CBC2A2 Content-Type: multipart/mixed; boundary="------------010706080103010700040902" This is a multi-part message in MIME format. --------------010706080103010700040902 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Hi, John, John Baldwin wrote: > My only reason for favoring the wakeup for complete initialization is t= hat > while this patch may solve the getprio/setprio race, it doesn't solve a= ll > 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. Cheers, --=20 Xin LI <delphij@delphij.net> http://www.delphij.net/ FreeBSD - The Power to Serve! --------------010706080103010700040902 Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0"; name="patch-priority-race.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline; filename="patch-priority-race.diff" Index: kern_exit.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/kern/kern_exit.c,v retrieving revision 1.294 diff -u -p -u -p -r1.294 kern_exit.c --- kern_exit.c 25 Oct 2006 06:18:04 -0000 1.294 +++ kern_exit.c 22 Feb 2007 16:19:17 -0000 @@ -526,6 +526,7 @@ retry: wakeup(p->p_pptr); mtx_lock_spin(&sched_lock); p->p_state =3D PRS_ZOMBIE; + wakeup(&p->p_state); PROC_UNLOCK(p->p_pptr); =20 sched_exit(p->p_pptr, td); Index: kern_fork.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/kern/kern_fork.c,v retrieving revision 1.266 diff -u -p -u -p -r1.266 kern_fork.c --- kern_fork.c 23 Jan 2007 08:46:50 -0000 1.266 +++ kern_fork.c 22 Feb 2007 14:58:41 -0000 @@ -695,8 +695,11 @@ again: * Set the child start time and mark the process as being complete. */ microuptime(&p2->p_stats->p_start); + PROC_LOCK(p2); mtx_lock_spin(&sched_lock); p2->p_state =3D PRS_NORMAL; + wakeup(&p2->p_state); + PROC_UNLOCK(p2); =20 /* * If RFSTOPPED not requested, make child runnable and add to Index: kern_resource.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/kern/kern_resource.c,v retrieving revision 1.166 diff -u -p -u -p -r1.166 kern_resource.c --- kern_resource.c 19 Feb 2007 13:22:36 -0000 1.166 +++ kern_resource.c 22 Feb 2007 17:44:04 -0000 @@ -144,6 +144,10 @@ getpriority(td, uap) sx_slock(&allproc_lock); FOREACH_PROC_IN_SYSTEM(p) { PROC_LOCK(p); + if (p->p_state =3D=3D PRS_NEW) { + PROC_UNLOCK(p); + continue; + } if (!p_cansee(td, p) && p->p_ucred->cr_uid =3D=3D uap->who) { if (p->p_nice < low) @@ -228,9 +232,18 @@ setpriority(td, uap) case PRIO_USER: if (uap->who =3D=3D 0) uap->who =3D td->td_ucred->cr_uid; +again: sx_slock(&allproc_lock); FOREACH_PROC_IN_SYSTEM(p) { PROC_LOCK(p); + if (p->p_state =3D=3D PRS_NEW) { + sx_sunlock(&allproc_lock); + msleep(&p->p_state, &p->p_mtx, + 0, "setpriority", 0); + PROC_UNLOCK(p); + goto again; + } + MPASS(p->p_state =3D=3D PRS_NORMAL); if (p->p_ucred->cr_uid =3D=3D uap->who && !p_cansee(td, p)) { error =3D donice(td, p, uap->prio); --------------010706080103010700040902-- --------------enig7AEAF4ECD6333CE874CBC2A2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFF3dgWOfuToMruuMARA/mkAJwJ818KFaTzI9k0DLwlyH/PYhqlugCfYRLJ Z35n0p5z90qABJMY5pTlFGQ= =q29b -----END PGP SIGNATURE----- --------------enig7AEAF4ECD6333CE874CBC2A2--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45DDD816.80303>