Date: Thu, 19 Dec 2013 08:20:08 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Mateusz Guzik <mjguzik@gmail.com> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik <mjg@freebsd.org>, John Baldwin <jhb@freebsd.org> Subject: Re: svn commit: r259407 - head/sys/kern Message-ID: <20131219062008.GK59496@kib.kiev.ua> In-Reply-To: <20131219002824.GA5664@dft-labs.eu> References: <201312150411.rBF4Bhtg018852@svn.freebsd.org> <201312171141.49251.jhb@freebsd.org> <20131217181745.GB7535@dft-labs.eu> <201312171434.01345.jhb@freebsd.org> <20131219002824.GA5664@dft-labs.eu>
next in thread | previous in thread | raw e-mail | index | archive | help
--Bk/e7+bhc7oa9AZn Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 19, 2013 at 01:28:24AM +0100, Mateusz Guzik wrote: > That being said, instead of reverting the change (which would leave other= field > with similar issue in place) I propose adding the following: > --- a/sys/kern/kern_exit.c > +++ b/sys/kern/kern_exit.c > @@ -220,6 +220,12 @@ exit1(struct thread *td, int rv) > =20 > p->p_xstat =3D rv; /* Let event handler change exit status= */ > PROC_UNLOCK(p); > + > + /* > + * Some fields below are freed without having the proc locked, ch= eck > + * for P_WEXIT before accessing to make sure it is safe. > + */ > + >=20 > Which should make it clear. >=20 > But again, this is a cosmetic change and I have no strong opinion either > way. If you are still unconvinced I'm happy to revert it later. I think adding a comment is fine, but besides the comment you propose for describing what does the code do in the exit path, it is more important to put the same comment into the struct proc description. In other words, fields should be annotated to indicate which accesses require gating on exiting state. I think something similar should be done for the execing state. The kern_proc.c sysctls are already full of this knowledge, and since you are making it explicit in one case, doing the full pass makes sense. --Bk/e7+bhc7oa9AZn Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iQIcBAEBAgAGBQJSspAXAAoJEJDCuSvBvK1BqegQAKXPuyActrnrqIkjsZeswJmx 8l3IifWy8jdi37fAoPHba43TsysQk0AMIAgeUyERmM4nMOYBNUd2GlOkXLP7ixF2 2JXFNbJpPq672WrF+RvfyHUx+JtcDAQ3e76gElDrJnXBOvw7hR+oHSqJcLBQP14V 7DkCjYRV70XQxYgCeqyLdv1H5oMke6skfcrgMATarmmqt0XCgnKzAVbgnLK8QbJe 8dY4HvlofvVZRaCg6JiiurbFxT8Treg5vgNpdRS0sBP0YwF55TpQKvynnHARRhsW 8GsFCDI8sz7INXLNnb3xSq3gcu9VUBnk9l8UlWa8PKxga/CqrQupdLPuN7whRRUE sET38I/1znKhWixuFdk/Cc1Jhc36gJGUXGfy7tfexisZ+7+PTQ7ceyTIXf9nSfUj 7kMV5RHU4oIH0nbmQjI3nyTQajYv2qmyRriWaHGoyuid/3UO1uj5B2WrHn3FBdRa HJ7okbFLDznEhwxU2So7AKZYBCPCrGRoekZj/2TiPa9OMBC4y01akSYXOrSFs7gQ tGF3V498D0rhZO+n9PFMx4YOmH2zMjivmqOB2IdLQOXA6Q8CWp4nzCpObSp3pn3A nqLy71lasdyWTxmIRyylPtsTUEHmaMhVhvKTjUgcTE5Te1SP1bsBkRJdCSHX+Rzc 4Q6iWJSMOA+7znVC+lEL =pjGQ -----END PGP SIGNATURE----- --Bk/e7+bhc7oa9AZn--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131219062008.GK59496>