From 96d7f8ef4663a90a8826c0b5ab56e9036dcb8506 Mon Sep 17 00:00:00 2001 From: "Tim J. Robbins" Date: Mon, 17 Feb 2003 10:03:02 +0000 Subject: [PATCH] Use the proc lock to protect p_realtimer instead of Giant, and obtain sched_lock around accesses to p_stats->p_timer[] to avoid a potential race with hardclock. getitimer(), setitimer() and the realitexpire() callout are now Giant-free. --- sys/compat/linux/linux_misc.c | 5 +-- sys/kern/init_main.c | 2 +- sys/kern/kern_exit.c | 2 +- sys/kern/kern_time.c | 75 +++++++++++++++-------------------- sys/sys/proc.h | 2 +- 5 files changed, 38 insertions(+), 48 deletions(-) diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c index 57460a8bff44..b5057ecb52c9 100644 --- a/sys/compat/linux/linux_misc.c +++ b/sys/compat/linux/linux_misc.c @@ -164,7 +164,6 @@ linux_alarm(struct thread *td, struct linux_alarm_args *args) { struct itimerval it, old_it; struct timeval tv; - int s; #ifdef DEBUG if (ldebug(alarm)) @@ -178,7 +177,7 @@ linux_alarm(struct thread *td, struct linux_alarm_args *args) it.it_value.tv_usec = 0; it.it_interval.tv_sec = 0; it.it_interval.tv_usec = 0; - s = splsoftclock(); + PROC_LOCK(td->td_proc); old_it = td->td_proc->p_realtimer; getmicrouptime(&tv); if (timevalisset(&old_it.it_value)) @@ -189,7 +188,7 @@ linux_alarm(struct thread *td, struct linux_alarm_args *args) timevaladd(&it.it_value, &tv); } td->td_proc->p_realtimer = it; - splx(s); + PROC_UNLOCK(td->td_proc); if (timevalcmp(&old_it.it_value, &tv, >)) { timevalsub(&old_it.it_value, &tv); if (old_it.it_value.tv_usec != 0) diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c index d068ae07aa97..d135c9ba422b 100644 --- a/sys/kern/init_main.c +++ b/sys/kern/init_main.c @@ -385,7 +385,7 @@ proc0_init(void *dummy __unused) bcopy("swapper", p->p_comm, sizeof ("swapper")); - callout_init(&p->p_itcallout, 0); + callout_init(&p->p_itcallout, 1); callout_init(&td->td_slpcallout, 1); /* Create credentials. */ diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index 545fa3a1610d..abf9025c7a09 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -237,9 +237,9 @@ exit1(td, rv) stopprofclock(p); p->p_flag &= ~(P_TRACED | P_PPWAIT); SIGEMPTYSET(p->p_siglist); - PROC_UNLOCK(p); if (timevalisset(&p->p_realtimer.it_value)) callout_stop(&p->p_itcallout); + PROC_UNLOCK(p); /* * Reset any sigio structures pointing to us as a result of diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c index 405e930e3115..6568eda26d33 100644 --- a/sys/kern/kern_time.c +++ b/sys/kern/kern_time.c @@ -408,21 +408,16 @@ struct getitimer_args { /* * MPSAFE */ -/* ARGSUSED */ int getitimer(struct thread *td, struct getitimer_args *uap) { struct proc *p = td->td_proc; struct timeval ctv; struct itimerval aitv; - int s; if (uap->which > ITIMER_PROF) return (EINVAL); - mtx_lock(&Giant); - - s = splclock(); /* XXX still needed ? */ if (uap->which == ITIMER_REAL) { /* * Convert from absolute to relative time in .it_value @@ -430,7 +425,9 @@ getitimer(struct thread *td, struct getitimer_args *uap) * has passed return 0, else return difference between * current time and time for the timer to go off. */ + PROC_LOCK(p); aitv = p->p_realtimer; + PROC_UNLOCK(p); if (timevalisset(&aitv.it_value)) { getmicrouptime(&ctv); if (timevalcmp(&aitv.it_value, &ctv, <)) @@ -439,10 +436,10 @@ getitimer(struct thread *td, struct getitimer_args *uap) timevalsub(&aitv.it_value, &ctv); } } else { + mtx_lock_spin(&sched_lock); aitv = p->p_stats->p_timer[uap->which]; + mtx_unlock_spin(&sched_lock); } - splx(s); - mtx_unlock(&Giant); return (copyout(&aitv, uap->itv, sizeof (struct itimerval))); } @@ -455,44 +452,32 @@ struct setitimer_args { /* * MPSAFE */ -/* ARGSUSED */ int setitimer(struct thread *td, struct setitimer_args *uap) { struct proc *p = td->td_proc; - struct itimerval aitv; + struct itimerval aitv, oitv; struct timeval ctv; - struct itimerval *itvp; - int s, error = 0; + int error; + + if (uap->itv == NULL) { + uap->itv = uap->oitv; + return (getitimer(td, (struct getitimer_args *)uap)); + } if (uap->which > ITIMER_PROF) return (EINVAL); - itvp = uap->itv; - if (itvp && (error = copyin(itvp, &aitv, sizeof(struct itimerval)))) + if ((error = copyin(uap->itv, &aitv, sizeof(struct itimerval)))) return (error); - - mtx_lock(&Giant); - - if ((uap->itv = uap->oitv) && - (error = getitimer(td, (struct getitimer_args *)uap))) { - goto done2; - } - if (itvp == 0) { - error = 0; - goto done2; - } - if (itimerfix(&aitv.it_value)) { - error = EINVAL; - goto done2; - } - if (!timevalisset(&aitv.it_value)) { + if (itimerfix(&aitv.it_value)) + return (EINVAL); + if (!timevalisset(&aitv.it_value)) timevalclear(&aitv.it_interval); - } else if (itimerfix(&aitv.it_interval)) { - error = EINVAL; - goto done2; - } - s = splclock(); /* XXX: still needed ? */ + else if (itimerfix(&aitv.it_interval)) + return (EINVAL); + if (uap->which == ITIMER_REAL) { + PROC_LOCK(p); if (timevalisset(&p->p_realtimer.it_value)) callout_stop(&p->p_itcallout); if (timevalisset(&aitv.it_value)) @@ -500,14 +485,24 @@ setitimer(struct thread *td, struct setitimer_args *uap) realitexpire, p); getmicrouptime(&ctv); timevaladd(&aitv.it_value, &ctv); + oitv = p->p_realtimer; p->p_realtimer = aitv; + PROC_UNLOCK(p); + if (timevalisset(&oitv.it_value)) { + if (timevalcmp(&oitv.it_value, &ctv, <)) + timevalclear(&oitv.it_value); + else + timevalsub(&oitv.it_value, &ctv); + } } else { + mtx_lock_spin(&sched_lock); + oitv = p->p_stats->p_timer[uap->which]; p->p_stats->p_timer[uap->which] = aitv; + mtx_unlock_spin(&sched_lock); } - splx(s); -done2: - mtx_unlock(&Giant); - return (error); + if (uap->oitv == NULL) + return (0); + return (copyout(&oitv, uap->oitv, sizeof(struct itimerval))); } /* @@ -527,7 +522,6 @@ realitexpire(void *arg) { struct proc *p; struct timeval ctv, ntv; - int s; p = (struct proc *)arg; PROC_LOCK(p); @@ -538,7 +532,6 @@ realitexpire(void *arg) return; } for (;;) { - s = splclock(); /* XXX: still neeeded ? */ timevaladd(&p->p_realtimer.it_value, &p->p_realtimer.it_interval); getmicrouptime(&ctv); @@ -547,11 +540,9 @@ realitexpire(void *arg) timevalsub(&ntv, &ctv); callout_reset(&p->p_itcallout, tvtohz(&ntv) - 1, realitexpire, p); - splx(s); PROC_UNLOCK(p); return; } - splx(s); } /*NOTREACHED*/ } diff --git a/sys/sys/proc.h b/sys/sys/proc.h index afb225d8ef83..9933954f1e89 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -543,7 +543,7 @@ struct proc { pid_t p_oppid; /* (c + e) Save ppid in ptrace. XXX */ struct vmspace *p_vmspace; /* (b) Address space. */ u_int p_swtime; /* (j) Time swapped in or out. */ - struct itimerval p_realtimer; /* (h?/k?) Alarm timer. */ + struct itimerval p_realtimer; /* (c) Alarm timer. */ struct bintime p_runtime; /* (j) Real time. */ u_int64_t p_uu; /* (j) Previous user time in usec. */ u_int64_t p_su; /* (j) Previous system time in usec. */