From owner-freebsd-stable@FreeBSD.ORG Sat Oct 15 21:51:45 2005 Return-Path: X-Original-To: stable@FreeBSD.org Delivered-To: freebsd-stable@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1882116A41F for ; Sat, 15 Oct 2005 21:51:45 +0000 (GMT) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id AB3B643D46 for ; Sat, 15 Oct 2005 21:51:44 +0000 (GMT) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.13.3/8.13.3) with ESMTP id j9FLpbvL057740 for ; Sat, 15 Oct 2005 14:51:41 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <200510152151.j9FLpbvL057740@gw.catspoiler.org> Date: Sat, 15 Oct 2005 14:51:37 -0700 (PDT) From: Don Lewis To: stable@FreeBSD.org MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii Cc: Subject: testers wanted for 5.4-STABLE sysctl kern.proc patch X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Oct 2005 21:51:45 -0000 The patch below is the 5.4-STABLE version of a patch that was recently committed to HEAD and 6.0-BETA5 to fix locking problems in the kern.proc sysctl handler that could cause panics or deadlocks. It has already been tested by myself and one other person in 5.4-STABLE, but I think it deserves wider testing before I commit it. Testing on SMP systems, while running threaded applications, and on systems that have experienced panics in the existing code is of the most interest. Also be on the lookout for any regressions, such as incorrect data being returned. Index: sys/kern/kern_proc.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_proc.c,v retrieving revision 1.215.2.6 diff -u -r1.215.2.6 kern_proc.c --- sys/kern/kern_proc.c 22 Mar 2005 13:40:23 -0000 1.215.2.6 +++ sys/kern/kern_proc.c 12 Oct 2005 19:13:14 -0000 @@ -72,6 +72,8 @@ static void doenterpgrp(struct proc *, struct pgrp *); static void orphanpg(struct pgrp *pg); +static void fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp); +static void fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp); static void pgadjustjobc(struct pgrp *pgrp, int entering); static void pgdelete(struct pgrp *); static int proc_ctor(void *mem, int size, void *arg, int flags); @@ -601,33 +603,22 @@ } } #endif /* DDB */ -void -fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp); /* - * Fill in a kinfo_proc structure for the specified process. + * Clear kinfo_proc and fill in any information that is common + * to all threads in the process. * Must be called with the target process locked. */ -void -fill_kinfo_proc(struct proc *p, struct kinfo_proc *kp) -{ - fill_kinfo_thread(FIRST_THREAD_IN_PROC(p), kp); -} - -void -fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp) +static void +fill_kinfo_proc_only(struct proc *p, struct kinfo_proc *kp) { - struct proc *p; struct thread *td0; - struct ksegrp *kg; struct tty *tp; struct session *sp; struct timeval tv; struct ucred *cred; struct sigacts *ps; - p = td->td_proc; - bzero(kp, sizeof(*kp)); kp->ki_structsize = sizeof(*kp); @@ -685,7 +676,8 @@ kp->ki_tsize = vm->vm_tsize; kp->ki_dsize = vm->vm_dsize; kp->ki_ssize = vm->vm_ssize; - } + } else if (p->p_state == PRS_ZOMBIE) + kp->ki_stat = SZOMB; if ((p->p_sflag & PS_INMEM) && p->p_stats) { kp->ki_start = p->p_stats->p_start; timevaladd(&kp->ki_start, &boottime); @@ -704,71 +696,6 @@ kp->ki_nice = p->p_nice; bintime2timeval(&p->p_runtime, &tv); kp->ki_runtime = tv.tv_sec * (u_int64_t)1000000 + tv.tv_usec; - if (p->p_state != PRS_ZOMBIE) { -#if 0 - if (td == NULL) { - /* XXXKSE: This should never happen. */ - printf("fill_kinfo_proc(): pid %d has no threads!\n", - p->p_pid); - mtx_unlock_spin(&sched_lock); - return; - } -#endif - if (td->td_wmesg != NULL) { - strlcpy(kp->ki_wmesg, td->td_wmesg, - sizeof(kp->ki_wmesg)); - } - if (TD_ON_LOCK(td)) { - kp->ki_kiflag |= KI_LOCKBLOCK; - strlcpy(kp->ki_lockname, td->td_lockname, - sizeof(kp->ki_lockname)); - } - - if (p->p_state == PRS_NORMAL) { /* XXXKSE very approximate */ - if (TD_ON_RUNQ(td) || - TD_CAN_RUN(td) || - TD_IS_RUNNING(td)) { - kp->ki_stat = SRUN; - } else if (P_SHOULDSTOP(p)) { - kp->ki_stat = SSTOP; - } else if (TD_IS_SLEEPING(td)) { - kp->ki_stat = SSLEEP; - } else if (TD_ON_LOCK(td)) { - kp->ki_stat = SLOCK; - } else { - kp->ki_stat = SWAIT; - } - } else { - kp->ki_stat = SIDL; - } - - kg = td->td_ksegrp; - - /* things in the KSE GROUP */ - kp->ki_estcpu = kg->kg_estcpu; - kp->ki_slptime = kg->kg_slptime; - kp->ki_pri.pri_user = kg->kg_user_pri; - kp->ki_pri.pri_class = kg->kg_pri_class; - - /* Things in the thread */ - kp->ki_wchan = td->td_wchan; - kp->ki_pri.pri_level = td->td_priority; - kp->ki_pri.pri_native = td->td_base_pri; - kp->ki_lastcpu = td->td_lastcpu; - kp->ki_oncpu = td->td_oncpu; - kp->ki_tdflags = td->td_flags; - kp->ki_tid = td->td_tid; - kp->ki_numthreads = p->p_numthreads; - kp->ki_pcb = td->td_pcb; - kp->ki_kstack = (void *)td->td_kstack; - kp->ki_pctcpu = sched_pctcpu(td); - - /* We can't get this anymore but ps etc never used it anyway. */ - kp->ki_rqindex = 0; - - } else { - kp->ki_stat = SZOMB; - } mtx_unlock_spin(&sched_lock); tp = NULL; if (p->p_pgrp) { @@ -804,8 +731,6 @@ p->p_sysent->sv_name[0] != '\0') strlcpy(kp->ki_emul, p->p_sysent->sv_name, sizeof(kp->ki_emul)); kp->ki_siglist = p->p_siglist; - SIGSETOR(kp->ki_siglist, td->td_siglist); - kp->ki_sigmask = td->td_sigmask; kp->ki_xstat = p->p_xstat; kp->ki_acflag = p->p_acflag; kp->ki_lock = p->p_lock; @@ -813,6 +738,92 @@ kp->ki_ppid = p->p_pptr->p_pid; } +/* + * Fill in information that is thread specific. + * Must be called with sched_lock locked. + */ +static void +fill_kinfo_thread(struct thread *td, struct kinfo_proc *kp) +{ + struct ksegrp *kg; + struct proc *p; + + p = td->td_proc; + + if (td->td_wmesg != NULL) + strlcpy(kp->ki_wmesg, td->td_wmesg, sizeof(kp->ki_wmesg)); + else + bzero(kp->ki_wmesg, sizeof(kp->ki_wmesg)); + if (TD_ON_LOCK(td)) { + kp->ki_kiflag |= KI_LOCKBLOCK; + strlcpy(kp->ki_lockname, td->td_lockname, + sizeof(kp->ki_lockname)); + } else { + kp->ki_kiflag &= ~KI_LOCKBLOCK; + bzero(kp->ki_lockname, sizeof(kp->ki_lockname)); + } + + if (p->p_state == PRS_NORMAL) { /* XXXKSE very approximate */ + if (TD_ON_RUNQ(td) || + TD_CAN_RUN(td) || + TD_IS_RUNNING(td)) { + kp->ki_stat = SRUN; + } else if (P_SHOULDSTOP(p)) { + kp->ki_stat = SSTOP; + } else if (TD_IS_SLEEPING(td)) { + kp->ki_stat = SSLEEP; + } else if (TD_ON_LOCK(td)) { + kp->ki_stat = SLOCK; + } else { + kp->ki_stat = SWAIT; + } + } else { + kp->ki_stat = SIDL; + } + + kg = td->td_ksegrp; + + /* things in the KSE GROUP */ + kp->ki_estcpu = kg->kg_estcpu; + kp->ki_slptime = kg->kg_slptime; + kp->ki_pri.pri_user = kg->kg_user_pri; + kp->ki_pri.pri_class = kg->kg_pri_class; + + /* Things in the thread */ + kp->ki_wchan = td->td_wchan; + kp->ki_pri.pri_level = td->td_priority; + kp->ki_pri.pri_native = td->td_base_pri; + kp->ki_lastcpu = td->td_lastcpu; + kp->ki_oncpu = td->td_oncpu; + kp->ki_tdflags = td->td_flags; + kp->ki_tid = td->td_tid; + kp->ki_numthreads = p->p_numthreads; + kp->ki_pcb = td->td_pcb; + kp->ki_kstack = (void *)td->td_kstack; + kp->ki_pctcpu = sched_pctcpu(td); + + /* We can't get this anymore but ps etc never used it anyway. */ + kp->ki_rqindex = 0; + + SIGSETOR(kp->ki_siglist, td->td_siglist); + kp->ki_sigmask = td->td_sigmask; +} + +/* + * Fill in a kinfo_proc structure for the specified process. + * Must be called with the target process locked. + */ +void +fill_kinfo_proc(struct proc *p, struct kinfo_proc *kp) +{ + + fill_kinfo_proc_only(p, kp); + mtx_lock_spin(&sched_lock); + if (FIRST_THREAD_IN_PROC(p) != NULL) + fill_kinfo_thread(FIRST_THREAD_IN_PROC(p), kp); + mtx_unlock_spin(&sched_lock); +} + struct pstats * pstats_alloc(void) { @@ -875,24 +886,28 @@ PROC_LOCK_ASSERT(p, MA_OWNED); + fill_kinfo_proc_only(p, &kinfo_proc); if (flags & KERN_PROC_NOTHREADS) { - fill_kinfo_proc(p, &kinfo_proc); - PROC_UNLOCK(p); + mtx_lock_spin(&sched_lock); + if (FIRST_THREAD_IN_PROC(p) != NULL) + fill_kinfo_thread(FIRST_THREAD_IN_PROC(p), &kinfo_proc); + mtx_unlock_spin(&sched_lock); error = SYSCTL_OUT(req, (caddr_t)&kinfo_proc, sizeof(kinfo_proc)); - PROC_LOCK(p); } else { - _PHOLD(p); - FOREACH_THREAD_IN_PROC(p, td) { - fill_kinfo_thread(td, &kinfo_proc); - PROC_UNLOCK(p); + mtx_lock_spin(&sched_lock); + if (FIRST_THREAD_IN_PROC(p) != NULL) + FOREACH_THREAD_IN_PROC(p, td) { + fill_kinfo_thread(td, &kinfo_proc); + error = SYSCTL_OUT(req, (caddr_t)&kinfo_proc, + sizeof(kinfo_proc)); + if (error) + break; + } + else error = SYSCTL_OUT(req, (caddr_t)&kinfo_proc, sizeof(kinfo_proc)); - PROC_LOCK(p); - if (error) - break; - } - _PRELE(p); + mtx_unlock_spin(&sched_lock); } PROC_UNLOCK(p); if (error) @@ -934,6 +949,9 @@ if (oid_number == KERN_PROC_PID) { if (namelen != 1) return (EINVAL); + error = sysctl_wire_old_buffer(req, 0); + if (error) + return (error); p = pfind((pid_t)name[0]); if (!p) return (ESRCH);