From 9752f794c7d5bb40e6c3feab4c7ed871ac3fffbc Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Tue, 22 Apr 2003 20:54:04 +0000 Subject: [PATCH] - 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). --- sys/kern/kern_clock.c | 36 +++++++++++++++++------------------- sys/kern/kern_fork.c | 6 +++--- sys/kern/subr_prof.c | 34 +++++++++++++--------------------- sys/kern/subr_trap.c | 9 +++------ sys/sys/proc.h | 4 ++-- 5 files changed, 38 insertions(+), 51 deletions(-) diff --git a/sys/kern/kern_clock.c b/sys/kern/kern_clock.c index d6c49d27bbfa..3d04cf0eeab5 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 6be5da97befb..3dc5f2b51dd5 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -483,10 +483,10 @@ fork1(td, flags, pages, procp) * 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 @@ fork1(td, flags, pages, procp) 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 cf4a6e98b827..569d53c634fc 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 74d8fd6650fe..11e25e4192e1 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 40c42bd2a45e..7b103468fd8b 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. */