cvs commit: src/sys/kern kern_proc.c kern_switch.c src/sys/sys sched.h src/sys/vm vm_glue.c

[David Schultz]

On Mon, Sep 20, 2004, John Baldwin wrote:
> Note that the actual p_stats pointer is (b) as correctly annotated in 
> sys/proc.h.  I think I've actually already committed the locking for the 
> pstats structure.  I'm not sure why you think procfs or ptrace() requires 
> type stable procs since you have to lookup the proc by pid first before you 
> can try to deference the pointer.  The ptrace code at least is careful to 
> PHOLD() before it ever releases the proc lock it gets from pfind().  It may 
> be that wait1() isn't handing that edge case well (maybe it doesn't check 
> p_lock before doing uma_zfree()) but if that is the case that can be easily 
> fixed.  Are there any other ares you are concerned about?

Right, PHOLD() doesn't currently prevent a proc structure from
being recycled AFAIK; it just prevents the U area from being
swapped out.  Fixing this would presumably require PRELE() (and
other code that does p_lock-- directly) to do a wakeup(), since
wait4() would need to msleep on the proc lock.  This didn't matter
for the swapper because it would just skip processes with a
nonzero hold count.

It was the above observation in the context of places like
procfs_doprocmap() that led me to believe that type stability was
still required.  That's the only issue I know of; when I committed
this, I wasn't aware that it was likely to be the only one.  I'd
be happy to try to tackle it next weekend if I have a few hours
free (which is looking doubtful at this point...)

Actually, the situation for procfs_doprocmap() appears to be worse
than I originally thought, because I didn't notice that
vmspace_exitfree() sets p_vmspace to NULL.  Therefore, it's wrong
for procfs_doprocmap() to dereference it unless PHOLD() causes
exit1() to block.

BTW, shouldn't PHOLD() assert that (p == curproc)?  I think this is
the only race-free way to use it (as opposed to _PHOLD() with the
process already locked).  Maybe PHOLD(p) should simply be renamed
PHOLD_CURPROC().