Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 2 Jan 2006 19:10:30 -0800 (PST)
From:      Don Lewis <truckman@FreeBSD.org>
To:        dan@syz.com
Cc:        freebsd-stable@FreeBSD.org
Subject:   Re: FreeBSD unstable on Dell 1750 using SMP?
Message-ID:  <200601030310.k033AUij005775@gw.catspoiler.org>
In-Reply-To: <F65D78B2-B7A8-4CCF-A1C6-21F275A2F024@syz.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On 30 Nov, Dan Charrois wrote:
> This is encouraging - it's the first I've heard of someone who has  
> found a way to trigger the problem "on demand".  The problems I was  
> experiencing were on a dual Xeon with HTT enabled as well.    Perhaps  
> someone out there who knows much more about the inner workings of  
> FreeBSD may have an idea of why running top in "aggressive mode" like  
> this might trigger the random rebooting.  In particular, it would be  
> nice to *know* that someone out there specifically fixed whatever is  
> wrong in 5.4 when bringing it to 6.0.  It's encouraging that you  
> haven't had any problems since upgrading to 6.0, but I have to wonder  
> if the bug's actually fixed, or the specific trigger of running top  
> doesn't trigger the problem but the problem is still lurking in the  
> background waiting to strike with the right combination of events.
> 
> In any case, I'm anxious to try it out myself on our server to see if  
> "top -s0" brings it down "on command" with HTT enabled, and not with  
> HTT disabled.  But I'm going to have to wait until some time over the  
> Christmas holidays to do that sort of experimentation at a time when  
> it isn't affecting the end users of the machine.  I may also upgrade  
> to 6.0 at that time, since by then it will have been out for a couple  
> of months, so most of the worst quirks should be worked out by then.
> 
> In the meantime, disabling HTT as I've done seems like a reasonable  
> precaution to improve the stability..
> 
> Thanks for your help!
> 
> Dan

Try this patch, which I posted to stable@ on October 15.  I had hoped to
commit it to RELENG_5 in November, but my day job intervened.

------ Forwarded message ------
    From: Don Lewis <truckman@freebsd.org>
 Subject: testers wanted for 5.4-STABLE sysctl kern.proc patch
    Date: Sat, 15 Oct 2005 14:51:37 -0700 (PDT)
      To: stable@freebsd.org
      Cc: 

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);

_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscribe@freebsd.org"




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