svn commit: r259407 - head/sys/kern

[Konstantin Belousov]

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)
>  
>         p->p_xstat = rv;        /* Let event handler change exit status */
>         PROC_UNLOCK(p);
> +
> +       /*
> +        * Some fields below are freed without having the proc locked, check
> +        * for P_WEXIT before accessing to make sure it is safe.
> +        */
> +
> 
> Which should make it clear.
> 
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20131219/74eb906b/attachment.sig>