From 20cdcc5b73ca7ca1c78ccaba30cf8dd1b2dd7ec0 Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Thu, 16 Nov 2000 02:16:44 +0000 Subject: [PATCH] Don't release and acquire Giant in mi_switch(). Instead, release and acquire Giant as needed in functions that call mi_switch(). The releases need to be done outside of the sched_lock to avoid potential deadlocks from trying to acquire Giant while interrupts are disabled. Submitted by: witness --- sys/alpha/alpha/trap.c | 2 ++ sys/amd64/amd64/trap.c | 2 ++ sys/i386/i386/trap.c | 2 ++ sys/ia64/ia64/trap.c | 2 ++ sys/kern/kern_mutex.c | 43 ++++----------------------------------- sys/kern/kern_shutdown.c | 2 ++ sys/kern/kern_sig.c | 8 ++++++++ sys/kern/kern_subr.c | 2 ++ sys/kern/kern_synch.c | 21 +++++-------------- sys/kern/subr_trap.c | 2 ++ sys/kern/subr_turnstile.c | 43 ++++----------------------------------- sys/kern/subr_witness.c | 43 ++++----------------------------------- 12 files changed, 39 insertions(+), 133 deletions(-) diff --git a/sys/alpha/alpha/trap.c b/sys/alpha/alpha/trap.c index f53fe7c96a94..a387910e8078 100644 --- a/sys/alpha/alpha/trap.c +++ b/sys/alpha/alpha/trap.c @@ -120,11 +120,13 @@ userret(p, pc, oticks, have_giant) * indicated by our priority. */ s = splstatclock(); + DROP_GIANT_NOSWITCH(); mtx_enter(&sched_lock, MTX_SPIN); setrunqueue(p); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); splx(s); while ((sig = CURSIG(p)) != 0) { if (have_giant == 0) { diff --git a/sys/amd64/amd64/trap.c b/sys/amd64/amd64/trap.c index 674f77a8a56d..21e5e12eadf5 100644 --- a/sys/amd64/amd64/trap.c +++ b/sys/amd64/amd64/trap.c @@ -192,11 +192,13 @@ userret(p, frame, oticks, have_giant) * our priority. */ s = splhigh(); + DROP_GIANT_NOSWITCH(); mtx_enter(&sched_lock, MTX_SPIN); setrunqueue(p); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); splx(s); while ((sig = CURSIG(p)) != 0) { if (have_giant == 0) { diff --git a/sys/i386/i386/trap.c b/sys/i386/i386/trap.c index 674f77a8a56d..21e5e12eadf5 100644 --- a/sys/i386/i386/trap.c +++ b/sys/i386/i386/trap.c @@ -192,11 +192,13 @@ userret(p, frame, oticks, have_giant) * our priority. */ s = splhigh(); + DROP_GIANT_NOSWITCH(); mtx_enter(&sched_lock, MTX_SPIN); setrunqueue(p); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); splx(s); while ((sig = CURSIG(p)) != 0) { if (have_giant == 0) { diff --git a/sys/ia64/ia64/trap.c b/sys/ia64/ia64/trap.c index 8a213c74821d..f5195438cd2c 100644 --- a/sys/ia64/ia64/trap.c +++ b/sys/ia64/ia64/trap.c @@ -102,11 +102,13 @@ userret(register struct proc *p, u_int64_t pc, u_quad_t oticks, int have_giant) * indicated by our priority. */ s = splstatclock(); + DROP_GIANT_NOSWITCH(); mtx_enter(&sched_lock, MTX_SPIN); setrunqueue(p); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); splx(s); while ((sig = CURSIG(p)) != 0) { if (have_giant == 0) { diff --git a/sys/kern/kern_mutex.c b/sys/kern/kern_mutex.c index 00df083a494a..354ff9149c40 100644 --- a/sys/kern/kern_mutex.c +++ b/sys/kern/kern_mutex.c @@ -223,7 +223,6 @@ void mtx_enter_hard(struct mtx *m, int type, int saveintr) { struct proc *p = CURPROC; - struct timeval new_switchtime; KASSERT(p != NULL, ("curproc is NULL in mutex")); @@ -323,42 +322,7 @@ mtx_enter_hard(struct mtx *m, int type, int saveintr) #endif CTR3(KTR_LOCK, "mtx_enter: p 0x%p blocked on [0x%p] %s", p, m, m->mtx_description); - /* - * Blatantly copied from mi_switch nearly verbatim. - * When Giant goes away and we stop dinking with it - * in mi_switch, we can go back to calling mi_switch - * directly here. - */ - - /* - * Compute the amount of time during which the current - * process was running, and add that to its total so - * far. - */ - microuptime(&new_switchtime); - if (timevalcmp(&new_switchtime, &switchtime, <)) { - printf( - "microuptime() went backwards (%ld.%06ld -> %ld.%06ld)\n", - switchtime.tv_sec, switchtime.tv_usec, - new_switchtime.tv_sec, - new_switchtime.tv_usec); - new_switchtime = switchtime; - } else { - p->p_runtime += (new_switchtime.tv_usec - - switchtime.tv_usec) + - (new_switchtime.tv_sec - switchtime.tv_sec) * - (int64_t)1000000; - } - - /* - * Pick a new current process and record its start time. - */ - cnt.v_swtch++; - switchtime = new_switchtime; - cpu_switch(); - if (switchtime.tv_sec == 0) - microuptime(&switchtime); - switchticks = ticks; + mi_switch(); CTR3(KTR_LOCK, "mtx_enter: p 0x%p free from blocked on [0x%p] %s", p, m, m->mtx_description); @@ -735,8 +699,10 @@ static char *ignore_list[] = { static char *spin_order_list[] = { "sched lock", - "clk", "sio", +#ifdef __i386__ + "clk", +#endif /* * leaf locks */ @@ -752,7 +718,6 @@ static char *dup_list[] = { }; static char *sleep_list[] = { - "Giant lock", NULL }; diff --git a/sys/kern/kern_shutdown.c b/sys/kern/kern_shutdown.c index 431f9660c1de..025ed0489ec8 100644 --- a/sys/kern/kern_shutdown.c +++ b/sys/kern/kern_shutdown.c @@ -254,6 +254,7 @@ boot(int howto) pbusy = nbusy; sync(&proc0, NULL); if (curproc != NULL) { + DROP_GIANT_NOSWITCH(); for (subiter = 0; subiter < 50 * iter; subiter++) { mtx_enter(&sched_lock, MTX_SPIN); setrunqueue(curproc); @@ -261,6 +262,7 @@ boot(int howto) mtx_exit(&sched_lock, MTX_SPIN); DELAY(1000); } + PICKUP_GIANT(); } else DELAY(50000 * iter); } diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index af7a32fe52db..dcc0ed3e7af9 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -1283,7 +1283,11 @@ issignal(p) psignal(p->p_pptr, SIGCHLD); do { stop(p); + DROP_GIANT_NOSWITCH(); + mtx_enter(&sched_lock, MTX_SPIN); mi_switch(); + mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); } while (!trace_req(p) && p->p_flag & P_TRACED); @@ -1352,7 +1356,11 @@ issignal(p) stop(p); if ((p->p_pptr->p_procsig->ps_flag & PS_NOCLDSTOP) == 0) psignal(p->p_pptr, SIGCHLD); + DROP_GIANT_NOSWITCH(); + mtx_enter(&sched_lock, MTX_SPIN); mi_switch(); + mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); break; } else if (prop & SA_IGNORE) { /* diff --git a/sys/kern/kern_subr.c b/sys/kern/kern_subr.c index 7c5c94afe65b..429f34b7f8f5 100644 --- a/sys/kern/kern_subr.c +++ b/sys/kern/kern_subr.c @@ -377,11 +377,13 @@ uio_yield() p = curproc; s = splhigh(); + DROP_GIANT_NOSWITCH(); mtx_enter(&sched_lock, MTX_SPIN); p->p_priority = p->p_usrpri; setrunqueue(p); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); splx(s); } diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c index 07e101416e6a..13b57073d109 100644 --- a/sys/kern/kern_synch.c +++ b/sys/kern/kern_synch.c @@ -420,6 +420,7 @@ msleep(ident, mtx, priority, wmesg, timo) if (p && KTRPOINT(p, KTR_CSW)) ktrcsw(p->p_tracep, 1, 0); #endif + DROP_GIANT_NOSWITCH(); WITNESS_SLEEP(0, mtx); mtx_enter(&sched_lock, MTX_SPIN); @@ -533,6 +534,7 @@ msleep(ident, mtx, priority, wmesg, timo) if (KTRPOINT(p, KTR_CSW)) ktrcsw(p->p_tracep, 0, 0); #endif + PICKUP_GIANT(); if (mtx != NULL) { mtx_enter(mtx, MTX_DEF); WITNESS_RESTORE(mtx, mtx); @@ -610,6 +612,7 @@ mawait(struct mtx *mtx, int priority, int timo) int s; WITNESS_SAVE_DECL(mtx); + DROP_GIANT_NOSWITCH(); WITNESS_SLEEP(0, mtx); mtx_enter(&sched_lock, MTX_SPIN); if (mtx != NULL) { @@ -724,6 +727,7 @@ mawait(struct mtx *mtx, int priority, int timo) p->p_asleep.as_priority = 0; out: + PICKUP_GIANT(); if (mtx != NULL) { mtx_enter(mtx, MTX_DEF); WITNESS_RESTORE(mtx, mtx); @@ -881,9 +885,7 @@ mi_switch() struct timeval new_switchtime; register struct proc *p = curproc; /* XXX */ register struct rlimit *rlim; - int giantreleased; int x; - WITNESS_SAVE_DECL(Giant); /* * XXX this spl is almost unnecessary. It is partly to allow for @@ -904,14 +906,7 @@ mi_switch() */ x = splstatclock(); - CTR4(KTR_PROC, "mi_switch: old proc %p (pid %d, %s), schedlock %p", - p, p->p_pid, p->p_comm, (void *) sched_lock.mtx_lock); - mtx_enter(&sched_lock, MTX_SPIN | MTX_RLIKELY); - - if (mtx_owned(&Giant)) - WITNESS_SAVE(&Giant, Giant); - for (giantreleased = 0; mtx_owned(&Giant); giantreleased++) - mtx_exit(&Giant, MTX_DEF | MTX_NOSWITCH); + mtx_assert(&sched_lock, MA_OWNED); #ifdef SIMPLELOCK_DEBUG if (p->p_simple_locks) @@ -965,12 +960,6 @@ mi_switch() if (switchtime.tv_sec == 0) microuptime(&switchtime); switchticks = ticks; - mtx_exit(&sched_lock, MTX_SPIN); - while (giantreleased--) - mtx_enter(&Giant, MTX_DEF); - if (mtx_owned(&Giant)) - WITNESS_RESTORE(&Giant, Giant); - splx(x); } diff --git a/sys/kern/subr_trap.c b/sys/kern/subr_trap.c index 674f77a8a56d..21e5e12eadf5 100644 --- a/sys/kern/subr_trap.c +++ b/sys/kern/subr_trap.c @@ -192,11 +192,13 @@ userret(p, frame, oticks, have_giant) * our priority. */ s = splhigh(); + DROP_GIANT_NOSWITCH(); mtx_enter(&sched_lock, MTX_SPIN); setrunqueue(p); p->p_stats->p_ru.ru_nivcsw++; mi_switch(); mtx_exit(&sched_lock, MTX_SPIN); + PICKUP_GIANT(); splx(s); while ((sig = CURSIG(p)) != 0) { if (have_giant == 0) { diff --git a/sys/kern/subr_turnstile.c b/sys/kern/subr_turnstile.c index 00df083a494a..354ff9149c40 100644 --- a/sys/kern/subr_turnstile.c +++ b/sys/kern/subr_turnstile.c @@ -223,7 +223,6 @@ void mtx_enter_hard(struct mtx *m, int type, int saveintr) { struct proc *p = CURPROC; - struct timeval new_switchtime; KASSERT(p != NULL, ("curproc is NULL in mutex")); @@ -323,42 +322,7 @@ mtx_enter_hard(struct mtx *m, int type, int saveintr) #endif CTR3(KTR_LOCK, "mtx_enter: p 0x%p blocked on [0x%p] %s", p, m, m->mtx_description); - /* - * Blatantly copied from mi_switch nearly verbatim. - * When Giant goes away and we stop dinking with it - * in mi_switch, we can go back to calling mi_switch - * directly here. - */ - - /* - * Compute the amount of time during which the current - * process was running, and add that to its total so - * far. - */ - microuptime(&new_switchtime); - if (timevalcmp(&new_switchtime, &switchtime, <)) { - printf( - "microuptime() went backwards (%ld.%06ld -> %ld.%06ld)\n", - switchtime.tv_sec, switchtime.tv_usec, - new_switchtime.tv_sec, - new_switchtime.tv_usec); - new_switchtime = switchtime; - } else { - p->p_runtime += (new_switchtime.tv_usec - - switchtime.tv_usec) + - (new_switchtime.tv_sec - switchtime.tv_sec) * - (int64_t)1000000; - } - - /* - * Pick a new current process and record its start time. - */ - cnt.v_swtch++; - switchtime = new_switchtime; - cpu_switch(); - if (switchtime.tv_sec == 0) - microuptime(&switchtime); - switchticks = ticks; + mi_switch(); CTR3(KTR_LOCK, "mtx_enter: p 0x%p free from blocked on [0x%p] %s", p, m, m->mtx_description); @@ -735,8 +699,10 @@ static char *ignore_list[] = { static char *spin_order_list[] = { "sched lock", - "clk", "sio", +#ifdef __i386__ + "clk", +#endif /* * leaf locks */ @@ -752,7 +718,6 @@ static char *dup_list[] = { }; static char *sleep_list[] = { - "Giant lock", NULL }; diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c index 00df083a494a..354ff9149c40 100644 --- a/sys/kern/subr_witness.c +++ b/sys/kern/subr_witness.c @@ -223,7 +223,6 @@ void mtx_enter_hard(struct mtx *m, int type, int saveintr) { struct proc *p = CURPROC; - struct timeval new_switchtime; KASSERT(p != NULL, ("curproc is NULL in mutex")); @@ -323,42 +322,7 @@ mtx_enter_hard(struct mtx *m, int type, int saveintr) #endif CTR3(KTR_LOCK, "mtx_enter: p 0x%p blocked on [0x%p] %s", p, m, m->mtx_description); - /* - * Blatantly copied from mi_switch nearly verbatim. - * When Giant goes away and we stop dinking with it - * in mi_switch, we can go back to calling mi_switch - * directly here. - */ - - /* - * Compute the amount of time during which the current - * process was running, and add that to its total so - * far. - */ - microuptime(&new_switchtime); - if (timevalcmp(&new_switchtime, &switchtime, <)) { - printf( - "microuptime() went backwards (%ld.%06ld -> %ld.%06ld)\n", - switchtime.tv_sec, switchtime.tv_usec, - new_switchtime.tv_sec, - new_switchtime.tv_usec); - new_switchtime = switchtime; - } else { - p->p_runtime += (new_switchtime.tv_usec - - switchtime.tv_usec) + - (new_switchtime.tv_sec - switchtime.tv_sec) * - (int64_t)1000000; - } - - /* - * Pick a new current process and record its start time. - */ - cnt.v_swtch++; - switchtime = new_switchtime; - cpu_switch(); - if (switchtime.tv_sec == 0) - microuptime(&switchtime); - switchticks = ticks; + mi_switch(); CTR3(KTR_LOCK, "mtx_enter: p 0x%p free from blocked on [0x%p] %s", p, m, m->mtx_description); @@ -735,8 +699,10 @@ static char *ignore_list[] = { static char *spin_order_list[] = { "sched lock", - "clk", "sio", +#ifdef __i386__ + "clk", +#endif /* * leaf locks */ @@ -752,7 +718,6 @@ static char *dup_list[] = { }; static char *sleep_list[] = { - "Giant lock", NULL };