Date: Mon, 20 Sep 2004 16:59:40 -0400 From: David Schultz <das@FreeBSD.ORG> To: John Baldwin <jhb@FreeBSD.ORG> Cc: cvs-all@FreeBSD.ORG Subject: Re: cvs commit: src/sys/kern kern_proc.c kern_switch.c src/sys/sys sched.h src/sys/vm vm_glue.c Message-ID: <20040920205940.GA4784@VARK.MIT.EDU> In-Reply-To: <200409201410.58648.jhb@FreeBSD.org> References: <200409191834.i8JIYHXU089517@repoman.freebsd.org> <414E0DCA.5090601@elischer.org> <20040920001317.GA2829@VARK.MIT.EDU> <200409201410.58648.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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().
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040920205940.GA4784>