d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix some locking nits with the p_state field of struct proc:

Browse Source 
- Hold the proc lock while changing the state from PRS_NEW to PRS_NORMAL
  in fork to honor the locking requirements.  While here, expand the scope
  of the PROC_LOCK() on the new process (p2) to avoid some LORs.  Previously
  the code was locking the new child process (p2) after it had locked the
  parent process (p1).  However, when locking two processes, the safe order
  is to lock the child first, then the parent.
- Fix various places that were checking p_state against PRS_NEW without
  having the process locked to use PROC_LOCK().  Every place was already
  locking the process, just after the PRS_NEW check.
- Remove or reduce the use of PROC_SLOCK() for places that were checking
  p_state against PRS_NEW.  The PROC_LOCK() alone is sufficient for reading
  the current state.
- Reorder fill_kinfo_proc() slightly so it only acquires PROC_SLOCK() once.

MFC after:	1 week
This commit is contained in:
John Baldwin 2011-03-24 18:40:11 +00:00
parent 3bf92decd3
commit 8e6fa660f2
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=219968
9 changed files with 28 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sys/compat/linprocfs/linprocfs.c
View File

@ -740,7 +740,6 @@ linprocfs_doprocstatus(PFS_FILL_ARGS)
if (P_SHOULDSTOP(p)) {
state = "T (stopped)";
} else {
PROC_SLOCK(p);
switch(p->p_state) {
case PRS_NEW:
state = "I (idle)";
@ -770,7 +769,6 @@ linprocfs_doprocstatus(PFS_FILL_ARGS)
state = "? (unknown)";
break;
}
PROC_SUNLOCK(p);
}
fill_kinfo_proc(p, &kp);

8
sys/fs/nfsclient/nfs_clport.c
View File

@ -1102,11 +1102,11 @@ pfind_locked(pid_t pid)
LIST_FOREACH(p, PIDHASH(pid), p_hash)
if (p->p_pid == pid) {
if (p->p_state == PRS_NEW) {
p = NULL;
break;
}
PROC_LOCK(p);
if (p->p_state == PRS_NEW) {
PROC_UNLOCK(p);
p = NULL;
}
break;
}
return (p);

6
sys/kern/kern_descrip.c
View File

@ -2634,9 +2634,11 @@ sysctl_kern_file(SYSCTL_HANDLER_ARGS)
xf.xf_size = sizeof(xf);
sx_slock(&allproc_lock);
FOREACH_PROC_IN_SYSTEM(p) {
if (p->p_state == PRS_NEW)
continue;
PROC_LOCK(p);
if (p->p_state == PRS_NEW) {
PROC_UNLOCK(p);
continue;
}
if (p_cansee(req->td, p) != 0) {
PROC_UNLOCK(p);
continue;

11
sys/kern/kern_fork.c
View File

@ -630,12 +630,13 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
/*
* Set the child start time and mark the process as being complete.
*/
PROC_LOCK(p2);
PROC_LOCK(p1);
microuptime(&p2->p_stats->p_start);
PROC_SLOCK(p2);
p2->p_state = PRS_NORMAL;
PROC_SUNLOCK(p2);
PROC_LOCK(p1);
#ifdef KDTRACE_HOOKS
/*
* Tell the DTrace fasttrap provider about the new process
@ -643,11 +644,8 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
* p_state is PRS_NORMAL since the fasttrap module will use pfind()
* later on.
*/
if (dtrace_fasttrap_fork) {
PROC_LOCK(p2);
if (dtrace_fasttrap_fork)
dtrace_fasttrap_fork(p1, p2);
PROC_UNLOCK(p2);
}
#endif
if ((p1->p_flag & (P_TRACED | P_FOLLOWFORK)) == (P_TRACED |
P_FOLLOWFORK)) {
@ -660,12 +658,11 @@ do_fork(struct thread *td, int flags, struct proc *p2, struct thread *td2,
*/
td->td_dbgflags |= TDB_FORK;
td->td_dbg_forked = p2->p_pid;
PROC_LOCK(p2);
td2->td_dbgflags |= TDB_STOPATFORK;
_PHOLD(p2);
p2_held = 1;
PROC_UNLOCK(p2);
}
PROC_UNLOCK(p2);
if ((flags & RFSTOPPED) == 0) {
/*
* If RFSTOPPED not requested, make child runnable and

20
sys/kern/kern_proc.c
View File

@ -291,11 +291,11 @@ pfind(pid)
sx_slock(&allproc_lock);
LIST_FOREACH(p, PIDHASH(pid), p_hash)
if (p->p_pid == pid) {
if (p->p_state == PRS_NEW) {
p = NULL;
break;
}
PROC_LOCK(p);
if (p->p_state == PRS_NEW) {
PROC_UNLOCK(p);
p = NULL;
}
break;
}
sx_sunlock(&allproc_lock);
@ -756,7 +756,6 @@ fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp)
kp->ki_sigcatch = ps->ps_sigcatch;
mtx_unlock(&ps->ps_mtx);
}
PROC_SLOCK(p);
if (p->p_state != PRS_NEW &&
p->p_state != PRS_ZOMBIE &&
p->p_vmspace != NULL) {
@ -782,12 +781,11 @@ fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp)
kp->ki_swtime = (ticks - p->p_swtick) / hz;
kp->ki_pid = p->p_pid;
kp->ki_nice = p->p_nice;
rufetch(p, &kp->ki_rusage);
kp->ki_runtime = cputick2usec(p->p_rux.rux_runtime);
PROC_SUNLOCK(p);
kp->ki_start = p->p_stats->p_start;
timevaladd(&kp->ki_start, &boottime);
PROC_SLOCK(p);
rufetch(p, &kp->ki_rusage);
kp->ki_runtime = cputick2usec(p->p_rux.rux_runtime);
calcru(p, &kp->ki_rusage.ru_utime, &kp->ki_rusage.ru_stime);
PROC_SUNLOCK(p);
calccru(p, &kp->ki_childutime, &kp->ki_childstime);
@ -1213,13 +1211,11 @@ sysctl_kern_proc(SYSCTL_HANDLER_ARGS)
/*
* Skip embryonic processes.
*/
PROC_SLOCK(p);
PROC_LOCK(p);
if (p->p_state == PRS_NEW) {
PROC_SUNLOCK(p);
PROC_UNLOCK(p);
continue;
}
PROC_SUNLOCK(p);
PROC_LOCK(p);
KASSERT(p->p_ucred != NULL,
("process credential is NULL for non-NEW proc"));
/*

6
sys/kern/kern_resource.c
View File

@ -142,11 +142,9 @@ getpriority(td, uap)
uap->who = td->td_ucred->cr_uid;
sx_slock(&allproc_lock);
FOREACH_PROC_IN_SYSTEM(p) {
/* Do not bother to check PRS_NEW processes */
if (p->p_state == PRS_NEW)
continue;
PROC_LOCK(p);
if (p_cansee(td, p) == 0 &&
if (p->p_state == PRS_NORMAL &&
p_cansee(td, p) == 0 &&
p->p_ucred->cr_uid == uap->who) {
if (p->p_nice < low)
low = p->p_nice;

4
sys/kern/kern_thread.c
View File

@ -981,7 +981,9 @@ tdfind(lwpid_t tid, pid_t pid)
td = NULL;
break;
}
PROC_LOCK(td->td_proc);
if (td->td_proc->p_state == PRS_NEW) {
PROC_UNLOCK(td->td_proc);
td = NULL;
break;
}
@ -990,12 +992,10 @@ tdfind(lwpid_t tid, pid_t pid)
LIST_REMOVE(td, td_hash);
LIST_INSERT_HEAD(TIDHASH(td->td_tid),
td, td_hash);
PROC_LOCK(td->td_proc);
rw_wunlock(&tidhash_lock);
return (td);
}
}
PROC_LOCK(td->td_proc);
break;
}
run++;

3
sys/vm/vm_meter.c
View File

@ -130,15 +130,12 @@ vmtotal(SYSCTL_HANDLER_ARGS)
if (p->p_flag & P_SYSTEM)
continue;
PROC_LOCK(p);
PROC_SLOCK(p);
switch (p->p_state) {
case PRS_NEW:
PROC_SUNLOCK(p);
PROC_UNLOCK(p);
continue;
break;
default:
PROC_SUNLOCK(p);
FOREACH_THREAD_IN_PROC(p, td) {
thread_lock(td);
switch (td->td_state) {

10
sys/vm/vm_pageout.c
View File

@ -1281,14 +1281,13 @@ vm_pageout_oom(int shortage)
FOREACH_PROC_IN_SYSTEM(p) {
int breakout;
if (p->p_state != PRS_NORMAL)
continue;
if (PROC_TRYLOCK(p) == 0)
continue;
/*
* If this is a system, protected or killed process, skip it.
*/
if ((p->p_flag & (P_INEXEC | P_PROTECTED | P_SYSTEM)) ||
if (p->p_state != PRS_NORMAL ||
(p->p_flag & (P_INEXEC | P_PROTECTED | P_SYSTEM)) ||
(p->p_pid == 1) || P_KILLED(p) ||
((p->p_pid < 48) && (swap_pager_avail != 0))) {
PROC_UNLOCK(p);
@ -1651,14 +1650,13 @@ vm_daemon()
FOREACH_PROC_IN_SYSTEM(p) {
vm_pindex_t limit, size;
if (p->p_state != PRS_NORMAL)
continue;
/*
* if this is a system process or if we have already
* looked at this process, skip it.
*/
PROC_LOCK(p);
if (p->p_flag & (P_INEXEC | P_SYSTEM | P_WEXIT)) {
if (p->p_state != PRS_NORMAL ||
p->p_flag & (P_INEXEC | P_SYSTEM | P_WEXIT)) {
PROC_UNLOCK(p);
continue;
}