- Move PS_PROFIL and its new cousin PS_STOPPROF back over to p_flag and

  rename them appropriately.  Protect both flags with both the proc lock
  and the sched_lock.
- Protect p_profthreads with the proc lock.
- Remove Giant from profil(2).

5 files changed, 38 insertions, 51 deletions

diff --git a/sys/kern/kern_clock.c b/sys/kern/kern_clock.c
index d6c49d2..3d04cf0 100644
--- a/sys/kern/kern_clock.c
+++ b/sys/kern/kern_clock.c
@@ -303,17 +303,16 @@ startprofclock(p)
 	 * it should be protected later on by a time_lock, which would
 	 * cover psdiv, etc. as well.
 	 */
-	mtx_lock_spin(&sched_lock);
-	if (p->p_sflag & PS_STOPPROF) {
-		mtx_unlock_spin(&sched_lock);
+	PROC_LOCK_ASSERT(p, MA_OWNED);
+	if (p->p_flag & P_STOPPROF)
 		return;
-	}
-	if ((p->p_sflag & PS_PROFIL) == 0) {
-		p->p_sflag |= PS_PROFIL;
+	if ((p->p_flag & P_PROFIL) == 0) {
+		mtx_lock_spin(&sched_lock);
+		p->p_flag |= P_PROFIL;
 		if (++profprocs == 1)
 			cpu_startprofclock();
+		mtx_unlock_spin(&sched_lock);
 	}
-	mtx_unlock_spin(&sched_lock);
 }
 
 /*
@@ -325,21 +324,20 @@ stopprofclock(p)
 {
 
 	PROC_LOCK_ASSERT(p, MA_OWNED);
-retry:
-	mtx_lock_spin(&sched_lock);
-	if (p->p_sflag & PS_PROFIL) {
-		if (p->p_profthreads) {
-			p->p_sflag |= PS_STOPPROF;
-			mtx_unlock_spin(&sched_lock);
-			msleep(&p->p_profthreads, &p->p_mtx, PPAUSE,
-			       "stopprof", NULL);
-			goto retry;
+	if (p->p_flag & P_PROFIL) {
+		if (p->p_profthreads != 0) {
+			p->p_flag |= P_STOPPROF;
+			while (p->p_profthreads != 0)
+				msleep(&p->p_profthreads, &p->p_mtx, PPAUSE,
+				    "stopprof", NULL);
+			p->p_flag &= ~P_STOPPROF;
 		}
-		p->p_sflag &= ~(PS_PROFIL|PS_STOPPROF);
+		mtx_lock_spin(&sched_lock);
+		p->p_flag &= ~P_PROFIL;
 		if (--profprocs == 0)
 			cpu_stopprofclock();
+		mtx_unlock_spin(&sched_lock);
 	}
-	mtx_unlock_spin(&sched_lock);
 }
 
 /*
@@ -440,7 +438,7 @@ profclock(frame)
 		 * bother trying to count it.
 		 */
 		td = curthread;
-		if (td->td_proc->p_sflag & PS_PROFIL)
+		if (td->td_proc->p_flag & P_PROFIL)
 			addupc_intr(td, CLKF_PC(frame), 1);
 	}
 #ifdef GPROF
diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c
index 6be5da9..3dc5f2b 100644
--- a/sys/kern/kern_fork.c
+++ b/sys/kern/kern_fork.c
@@ -483,10 +483,10 @@ again:
 	 * The p_stats and p_sigacts substructs are set in vm_forkproc.
 	 */
 	p2->p_flag = 0;
+	if (p1->p_flag & P_PROFIL)
+		startprofclock(p2);
 	mtx_lock_spin(&sched_lock);
 	p2->p_sflag = PS_INMEM;
-	if (p1->p_sflag & PS_PROFIL)
-		startprofclock(p2);
 	/*
 	 * Allow the scheduler to adjust the priority of the child and
 	 * parent while we hold the sched_lock.
@@ -580,7 +580,7 @@ again:
 	PROC_LOCK(p1);
 
 	/*
-	 * Preserve some more flags in subprocess.  PS_PROFIL has already
+	 * Preserve some more flags in subprocess.  P_PROFIL has already
 	 * been preserved.
 	 */
 	p2->p_flag |= p1->p_flag & (P_SUGID | P_ALTSTACK);
diff --git a/sys/kern/subr_prof.c b/sys/kern/subr_prof.c
index cf4a6e9..569d53c 100644
--- a/sys/kern/subr_prof.c
+++ b/sys/kern/subr_prof.c
@@ -366,7 +366,9 @@ sysctl_kern_prof(SYSCTL_HANDLER_ARGS)
 			gp->state = GMON_PROF_OFF;
 			stopguprof(gp);
 			gp->profrate = profhz;
+			PROC_LOCK(&proc0);
 			startprofclock(&proc0);
+			PROC_UNLOCK(&proc0);
 			gp->state = state;
 #ifdef GUPROF
 		} else if (state == GMON_PROF_HIRES) {
@@ -424,35 +426,28 @@ profil(td, uap)
 	register struct profil_args *uap;
 {
 	struct uprof *upp;
-	int s;
-	int error = 0;
+	struct proc *p;
 
-	mtx_lock(&Giant);
+	if (uap->scale > (1 << 16))
+		return (EINVAL);
 
-	if (uap->scale > (1 << 16)) {
-		error = EINVAL;
-		goto done2;
-	}
+	p = td->td_proc;
 	if (uap->scale == 0) {
 		PROC_LOCK(td->td_proc);
 		stopprofclock(td->td_proc);
 		PROC_UNLOCK(td->td_proc);
-		goto done2;
+		return (0);
 	}
 	upp = &td->td_proc->p_stats->p_prof;
-
-	/* Block profile interrupts while changing state. */
-	s = splstatclock();
 	upp->pr_off = uap->offset;
 	upp->pr_scale = uap->scale;
 	upp->pr_base = uap->samples;
 	upp->pr_size = uap->size;
-	startprofclock(td->td_proc);
-	splx(s);
+	PROC_LOCK(p);
+	startprofclock(p);
+	PROC_UNLOCK(p);
 
-done2:
-	mtx_unlock(&Giant);
-	return (error);
+	return (0);
 }
 
 /*
@@ -521,14 +516,11 @@ addupc_task(struct thread *td, uintptr_t pc, u_int ticks)
 		return;
 
 	PROC_LOCK(p);
-	mtx_lock_spin(&sched_lock);
-	if (!(p->p_sflag & PS_PROFIL)) {
-		mtx_unlock_spin(&sched_lock);
+	if (!(p->p_flag & P_PROFIL)) {
 		PROC_UNLOCK(p);
 		return;
 	}
 	p->p_profthreads++;
-	mtx_unlock_spin(&sched_lock);
 	PROC_UNLOCK(p);
 	prof = &p->p_stats->p_prof;
 	if (pc < prof->pr_off ||
@@ -547,7 +539,7 @@ addupc_task(struct thread *td, uintptr_t pc, u_int ticks)
 out:
 	PROC_LOCK(p);
 	if (--p->p_profthreads == 0) {
-		if (p->p_sflag & PS_STOPPROF) {
+		if (p->p_flag & P_STOPPROF) {
 			wakeup(&p->p_profthreads);
 			stop = 0;
 		}
diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c
index 74d8fd6..11e25e4 100644
--- a/sys/kern/subr_trap.c
+++ b/sys/kern/subr_trap.c
@@ -115,11 +115,8 @@ userret(td, frame, oticks)
 
 	/*
 	 * Charge system time if profiling.
-	 *
-	 * XXX should move PS_PROFIL to a place that can obviously be
-	 * accessed safely without sched_lock.
 	 */
-	if (p->p_sflag & PS_PROFIL) {
+	if (p->p_flag & P_PROFIL) {
 		quad_t ticks;
 
 		mtx_lock_spin(&sched_lock);
@@ -182,7 +179,7 @@ ast(struct trapframe *framep)
 	    TDF_NEEDRESCHED | TDF_OWEUPC);
 	cnt.v_soft++;
 	prticks = 0;
-	if (flags & TDF_OWEUPC && sflag & PS_PROFIL) {
+	if (flags & TDF_OWEUPC && p->p_flag & P_PROFIL) {
 		prticks = p->p_stats->p_prof.pr_ticks;
 		p->p_stats->p_prof.pr_ticks = 0;
 	}
@@ -197,7 +194,7 @@ ast(struct trapframe *framep)
 
 	if (td->td_ucred != p->p_ucred) 
 		cred_update_thread(td);
-	if (flags & TDF_OWEUPC && sflag & PS_PROFIL)
+	if (flags & TDF_OWEUPC && p->p_flag & P_PROFIL)
 		addupc_task(td, p->p_stats->p_prof.pr_addr, prticks);
 	if (sflag & PS_ALRMPEND) {
 		PROC_LOCK(p);
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index 40c42bd..7b10346 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -630,6 +630,8 @@ struct proc {
 #define	P_KTHREAD	0x00004	/* Kernel thread. (*)*/
 #define	P_NOLOAD	0x00008	/* Ignore during load avg calculations. */
 #define	P_PPWAIT	0x00010	/* Parent is waiting for child to exec/exit. */
+#define	P_PROFIL	0x00020	/* Has started profiling. */
+#define	P_STOPPROF	0x00040	/* Has thread in requesting to stop prof */
 #define	P_SUGID		0x00100	/* Had set id privileges since last exec. */
 #define	P_SYSTEM	0x00200	/* System proc: no sigs, stats or swapping. */
 #define	P_WAITED	0x01000	/* Someone is waiting for us */
@@ -661,8 +663,6 @@ struct proc {
 /* These flags are kept in p_sflag and are protected with sched_lock. */
 #define	PS_INMEM	0x00001	/* Loaded into memory. */
 #define	PS_XCPU		0x00002 /* Exceeded CPU limit. */
-#define	PS_PROFIL	0x00004	/* Has started profiling. */
-#define	PS_STOPPROF	0x00008	/* Has thread in requesting to stop prof */
 #define	PS_ALRMPEND	0x00020	/* Pending SIGVTALRM needs to be posted. */
 #define	PS_PROFPEND	0x00040	/* Pending SIGPROF needs to be posted. */
 #define	PS_SWAPINREQ	0x00100	/* Swapin request due to wakeup. */