From owner-cvs-all@FreeBSD.ORG Mon Sep 20 20:59:43 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8CFF016A4CE; Mon, 20 Sep 2004 20:59:43 +0000 (GMT) Received: from VARK.MIT.EDU (VARK.MIT.EDU [18.95.3.179]) by mx1.FreeBSD.org (Postfix) with ESMTP id 113C143D53; Mon, 20 Sep 2004 20:59:43 +0000 (GMT) (envelope-from das@FreeBSD.ORG) Received: from VARK.MIT.EDU (localhost [127.0.0.1]) by VARK.MIT.EDU (8.13.1/8.12.10) with ESMTP id i8KKxemC005047; Mon, 20 Sep 2004 16:59:40 -0400 (EDT) (envelope-from das@FreeBSD.ORG) Received: (from das@localhost) by VARK.MIT.EDU (8.13.1/8.12.10/Submit) id i8KKxeXa005046; Mon, 20 Sep 2004 16:59:40 -0400 (EDT) (envelope-from das@FreeBSD.ORG) Date: Mon, 20 Sep 2004 16:59:40 -0400 From: David Schultz To: John Baldwin Message-ID: <20040920205940.GA4784@VARK.MIT.EDU> Mail-Followup-To: John Baldwin , Julian Elischer , src-committers@FreeBSD.ORG, cvs-src@FreeBSD.ORG, cvs-all@FreeBSD.ORG References: <200409191834.i8JIYHXU089517@repoman.freebsd.org> <414E0DCA.5090601@elischer.org> <20040920001317.GA2829@VARK.MIT.EDU> <200409201410.58648.jhb@FreeBSD.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200409201410.58648.jhb@FreeBSD.org> cc: cvs-src@FreeBSD.ORG cc: src-committers@FreeBSD.ORG cc: Julian Elischer 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 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Sep 2004 20:59:43 -0000 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().