Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 5 Apr 2002 14:35:21 -0800 (PST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 9138 for review
Message-ID:  <200204052235.g35MZLc10168@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help 
http://people.freebsd.org/~peter/p4db/chv.cgi?CH=9138

Change 9138 by jhb@jhb_laptop on 2002/04/05 14:35:08

	From jhb_proc: change fill_kinfo_proc() to require the proc to
	be locked.  Fixup callers.  Change sysctl_out_proc() to require
	the proc to already be locked and drop the lock before returning.
	Lock the proc only once during the kern.proc sysctl loop.

Affected files ...

... //depot/projects/smpng/sys/compat/linprocfs/linprocfs.c#13 integrate
... //depot/projects/smpng/sys/compat/pecoff/imgact_pecoff.c#8 integrate
... //depot/projects/smpng/sys/kern/imgact_aout.c#10 integrate
... //depot/projects/smpng/sys/kern/kern_proc.c#23 integrate
... //depot/projects/smpng/sys/kern/sys_process.c#11 integrate

Differences ...

==== //depot/projects/smpng/sys/compat/linprocfs/linprocfs.c#13 (text+ko) ====

@@ -489,12 +489,12 @@
 {
 	struct kinfo_proc kp;
 
+	PROC_LOCK(p);
 	fill_kinfo_proc(p, &kp);
 	sbuf_printf(sb, "%d", p->p_pid);
 #define PS_ADD(name, fmt, arg) sbuf_printf(sb, " " fmt, arg)
 	PS_ADD("comm",		"(%s)",	p->p_comm);
 	PS_ADD("statr",		"%c",	'0'); /* XXX */
-	PROC_LOCK(p);
 	PS_ADD("ppid",		"%d",	p->p_pptr ? p->p_pptr->p_pid : 0);
 	PS_ADD("pgrp",		"%d",	p->p_pgid);
 	PS_ADD("session",	"%d",	p->p_session->s_sid);
@@ -571,6 +571,7 @@
 		state = state_str[(int)p->p_stat];
 	mtx_unlock_spin(&sched_lock);
 
+	PROC_LOCK(p);
 	fill_kinfo_proc(p, &kp);
 	sbuf_printf(sb, "Name:\t%s\n",		p->p_comm); /* XXX escape */
 	sbuf_printf(sb, "State:\t%s\n",		state);
@@ -579,7 +580,6 @@
 	 * Credentials
 	 */
 	sbuf_printf(sb, "Pid:\t%d\n",		p->p_pid);
-	PROC_LOCK(p);
 	sbuf_printf(sb, "PPid:\t%d\n",		p->p_pptr ?
 						p->p_pptr->p_pid : 0);
 	sbuf_printf(sb, "Uid:\t%d %d %d %d\n",	p->p_ucred->cr_ruid,

==== //depot/projects/smpng/sys/compat/pecoff/imgact_pecoff.c#8 (text+ko) ====

@@ -173,7 +173,9 @@
 #endif
 	if (ctob((UAREA_PAGES+KSTACK_PAGES) + vm->vm_dsize + vm->vm_ssize) >= limit)
 		return (EFAULT);
+	PROC_LOCK(td->td_proc);
 	fill_kinfo_proc(td->td_proc, &td->td_proc->p_uarea->u_kproc);
+	PROC_UNLOCK(td-td_proc);
 
 #if PECOFF_DEBUG
 	fill_regs(td, &regs);

==== //depot/projects/smpng/sys/kern/imgact_aout.c#10 (text+ko) ====

@@ -263,7 +263,9 @@
 	if (ctob((UAREA_PAGES + KSTACK_PAGES)
 	    + vm->vm_dsize + vm->vm_ssize) >= limit)
 		return (EFAULT);
+	PROC_LOCK(p);
 	fill_kinfo_proc(p, &p->p_uarea->u_kproc);
+	PROC_UNLOCK(p);
 	error = cpu_coredump(td, vp, cred);
 	if (error == 0)
 		error = vn_rdwr(UIO_WRITE, vp, vm->vm_daddr,

==== //depot/projects/smpng/sys/kern/kern_proc.c#23 (text+ko) ====

@@ -643,6 +643,7 @@
 
 /*
  * Fill in an kinfo_proc structure for the specified process.
+ * Must be called with the target process locked.
  */
 void
 fill_kinfo_proc(p, kp)
@@ -658,7 +659,7 @@
 
 	kp->ki_structsize = sizeof(*kp);
 	kp->ki_paddr = p;
-	PROC_LOCK(p);
+	PROC_LOCK_ASSERT(p, MA_OWNED);
 	kp->ki_addr =/* p->p_addr; */0; /* XXXKSE */
 	kp->ki_args = p->p_args;
 	kp->ki_tracep = p->p_tracep;
@@ -778,7 +779,6 @@
 	kp->ki_lock = p->p_lock;
 	if (p->p_pptr)
 		kp->ki_ppid = p->p_pptr->p_pid;
-	PROC_UNLOCK(p);
 }
 
 /*
@@ -800,6 +800,9 @@
 }
 
 
+/*
+ * Must be called with the process locked and will return with it unlocked.
+ */
 static int
 sysctl_out_proc(struct proc *p, struct sysctl_req *req, int doingzomb)
 {
@@ -808,7 +811,9 @@
 	struct proc *np;
 	pid_t pid = p->p_pid;
 
+	PROC_LOCK_ASSERT(p, MA_OWNED);
 	fill_kinfo_proc(p, &kinfo_proc);
+	PROC_UNLOCK(p);
 	error = SYSCTL_OUT(req, (caddr_t)&kinfo_proc, sizeof(kinfo_proc));
 	if (error)
 		return (error);
@@ -872,16 +877,21 @@
 		else
 			p = LIST_FIRST(&zombproc);
 		for (; p != 0; p = LIST_NEXT(p, p_list)) {
+			PROC_LOCK(p);
 			/*
 			 * Show a user only appropriate processes.
 			 */
-			if (p_cansee(curproc, p))
+			if (p_cansee(curproc, p)) {
+				PROC_UNLOCK(p);
 				continue;
+			}
 			/*
 			 * Skip embryonic processes.
 			 */
-			if (p->p_stat == SIDL)
+			if (p->p_stat == SIDL) {
+				PROC_UNLOCK(p);
 				continue;
+			}
 			/*
 			 * TODO - make more efficient (see notes below).
 			 * do by session.
@@ -890,17 +900,14 @@
 
 			case KERN_PROC_PGRP:
 				/* could do this by traversing pgrp */
-				PROC_LOCK(p);
 				if (p->p_pgrp == NULL || 
 				    p->p_pgrp->pg_id != (pid_t)name[0]) {
 					PROC_UNLOCK(p);
 					continue;
 				}
-				PROC_UNLOCK(p);
 				break;
 
 			case KERN_PROC_TTY:
-				PROC_LOCK(p);
 				if ((p->p_flag & P_CONTROLT) == 0 ||
 				    p->p_session == NULL) {
 					PROC_UNLOCK(p);
@@ -915,25 +922,25 @@
 					continue;
 				}
 				SESS_UNLOCK(p->p_session);
-				PROC_UNLOCK(p);
 				break;
 
 			case KERN_PROC_UID:
 				if (p->p_ucred == NULL || 
-				    p->p_ucred->cr_uid != (uid_t)name[0])
+				    p->p_ucred->cr_uid != (uid_t)name[0]) {
+					PROC_UNLOCK(p);
 					continue;
+				}
 				break;
 
 			case KERN_PROC_RUID:
 				if (p->p_ucred == NULL || 
-				    p->p_ucred->cr_ruid != (uid_t)name[0])
+				    p->p_ucred->cr_ruid != (uid_t)name[0]) {
+					PROC_UNLOCK(p);
 					continue;
+				}
 				break;
 			}
 
-			if (p_cansee(curproc, p))
-				continue;
-
 			error = sysctl_out_proc(p, req, doingzomb);
 			if (error) {
 				sx_sunlock(&allproc_lock);

==== //depot/projects/smpng/sys/kern/sys_process.c#11 (text+ko) ====

@@ -488,7 +488,9 @@
 		}
 
 		if (uap->addr != (caddr_t)1) {
+			PROC_LOCK(p);
 			fill_kinfo_proc(p, &p->p_uarea->u_kproc);
+			PROC_UNLOCK(p);
 			error = ptrace_set_pc(td2,
 			    (u_long)(uintfptr_t)uap->addr);
 			if (error) {

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe p4-projects" in the body of the message

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200204052235.g35MZLc10168>