Fix a long standing race between sleep queue and thread

suspension code. When a thread A is going to sleep, it calls
sleepq_catch_signals() to detect any pending signals or thread
suspension request, if nothing happens, it returns without
holding process lock or scheduler lock, this opens a race
window which allows thread B to come in and do process
suspension work, however since A is still at running state,
thread B can do nothing to A, thread A continues, and puts
itself into actually sleeping state, but B has never seen it,
and it sits there forever until B is woken up by other threads
sometimes later(this can be very long delay or never
happen). Fix this bug by forcing sleepq_catch_signals to
return with scheduler lock held.
Fix sleepq_abort() by passing it an interrupted code, previously,
it worked as wakeup_one(), and the interruption can not be
identified correctly by sleep queue code when the sleeping
thread is resumed.
Let thread_suspend_check() returns EINTR or ERESTART, so sleep
queue no longer has to use SIGSTOP as a hack to build a return
value.

Reviewed by:	jhb
MFC after:	1 week
This commit is contained in:
David Xu 2006-02-15 23:52:01 +00:00
parent e70fcb23b6
commit 94f0972bec
8 changed files with 99 additions and 114 deletions

View File

@ -166,7 +166,7 @@ cv_wait_sig(struct cv *cvp, struct mtx *mp)
{
struct thread *td;
struct proc *p;
int rval, sig;
int rval;
WITNESS_SAVE_DECL(mp);
td = curthread;
@ -210,10 +210,7 @@ cv_wait_sig(struct cv *cvp, struct mtx *mp)
sleepq_add(cvp, mp, cvp->cv_description, SLEEPQ_CONDVAR |
SLEEPQ_INTERRUPTIBLE);
sig = sleepq_catch_signals(cvp);
rval = sleepq_wait_sig(cvp);
if (rval == 0)
rval = sleepq_calc_signal_retval(sig);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
@ -292,7 +289,6 @@ cv_timedwait_sig(struct cv *cvp, struct mtx *mp, int timo)
struct thread *td;
struct proc *p;
int rval;
int sig;
WITNESS_SAVE_DECL(mp);
td = curthread;
@ -338,10 +334,7 @@ cv_timedwait_sig(struct cv *cvp, struct mtx *mp, int timo)
sleepq_add(cvp, mp, cvp->cv_description, SLEEPQ_CONDVAR |
SLEEPQ_INTERRUPTIBLE);
sleepq_set_timeout(cvp, timo);
sig = sleepq_catch_signals(cvp);
rval = sleepq_timedwait_sig(cvp, sig != 0);
if (rval == 0)
rval = sleepq_calc_signal_retval(sig);
rval = sleepq_timedwait_sig(cvp);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))

View File

@ -223,7 +223,7 @@ kse_thr_interrupt(struct thread *td, struct kse_thr_interrupt_args *uap)
else
td2->td_intrval = ERESTART;
if (TD_ON_SLEEPQ(td2) && (td2->td_flags & TDF_SINTR))
sleepq_abort(td2);
sleepq_abort(td2, td2->td_intrval);
mtx_unlock_spin(&sched_lock);
}
PROC_UNLOCK(p);

View File

@ -92,7 +92,7 @@ static char *expand_name(const char *, uid_t, pid_t);
static int killpg1(struct thread *td, int sig, int pgid, int all);
static int issignal(struct thread *p);
static int sigprop(int sig);
static void tdsigwakeup(struct thread *, int, sig_t);
static void tdsigwakeup(struct thread *, int, sig_t, int);
static void sig_suspend_threads(struct thread *, struct proc *, int);
static int filt_sigattach(struct knote *kn);
static void filt_sigdetach(struct knote *kn);
@ -2056,6 +2056,7 @@ do_tdsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi)
sigqueue_t *sigqueue;
int prop;
struct sigacts *ps;
int intrval;
int ret = 0;
PROC_LOCK_ASSERT(p, MA_OWNED);
@ -2116,6 +2117,10 @@ do_tdsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi)
action = SIG_CATCH;
else
action = SIG_DFL;
if (SIGISMEMBER(ps->ps_sigintr, sig))
intrval = EINTR;
else
intrval = ERESTART;
mtx_unlock(&ps->ps_mtx);
if (prop & SA_CONT)
@ -2260,7 +2265,7 @@ do_tdsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi)
*/
mtx_lock_spin(&sched_lock);
if (TD_ON_SLEEPQ(td) && (td->td_flags & TDF_SINTR))
sleepq_abort(td);
sleepq_abort(td, intrval);
mtx_unlock_spin(&sched_lock);
goto out;
/*
@ -2270,7 +2275,7 @@ do_tdsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi)
} else if (p->p_state == PRS_NORMAL) {
if (p->p_flag & P_TRACED || action == SIG_CATCH) {
mtx_lock_spin(&sched_lock);
tdsigwakeup(td, sig, action);
tdsigwakeup(td, sig, action, intrval);
mtx_unlock_spin(&sched_lock);
goto out;
}
@ -2315,7 +2320,7 @@ do_tdsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi)
runfast:
mtx_lock_spin(&sched_lock);
tdsigwakeup(td, sig, action);
tdsigwakeup(td, sig, action, intrval);
thread_unsuspend(p);
mtx_unlock_spin(&sched_lock);
out:
@ -2330,7 +2335,7 @@ out:
* out of any sleep it may be in etc.
*/
static void
tdsigwakeup(struct thread *td, int sig, sig_t action)
tdsigwakeup(struct thread *td, int sig, sig_t action, int intrval)
{
struct proc *p = td->td_proc;
register int prop;
@ -2382,7 +2387,7 @@ tdsigwakeup(struct thread *td, int sig, sig_t action)
if (td->td_priority > PUSER)
sched_prio(td, PUSER);
sleepq_abort(td);
sleepq_abort(td, intrval);
} else {
/*
* Other states do nothing with the signal immediately,

View File

@ -124,7 +124,7 @@ msleep(ident, mtx, priority, wmesg, timo)
{
struct thread *td;
struct proc *p;
int catch, rval, sig, flags;
int catch, rval, flags;
WITNESS_SAVE_DECL(mtx);
td = curthread;
@ -205,10 +205,6 @@ msleep(ident, mtx, priority, wmesg, timo)
sleepq_add(ident, mtx, wmesg, flags);
if (timo)
sleepq_set_timeout(ident, timo);
if (catch) {
sig = sleepq_catch_signals(ident);
} else
sig = 0;
/*
* Adjust this thread's priority.
@ -218,7 +214,7 @@ msleep(ident, mtx, priority, wmesg, timo)
mtx_unlock_spin(&sched_lock);
if (timo && catch)
rval = sleepq_timedwait_sig(ident, sig != 0);
rval = sleepq_timedwait_sig(ident);
else if (timo)
rval = sleepq_timedwait(ident);
else if (catch)
@ -227,8 +223,6 @@ msleep(ident, mtx, priority, wmesg, timo)
sleepq_wait(ident);
rval = 0;
}
if (rval == 0 && catch)
rval = sleepq_calc_signal_retval(sig);
#ifdef KTRACE
if (KTRPOINT(td, KTR_CSW))
ktrcsw(0, 0);

View File

@ -762,7 +762,7 @@ thread_single(int mode)
thread_unsuspend_one(td2);
if (TD_ON_SLEEPQ(td2) &&
(td2->td_flags & TDF_SINTR))
sleepq_abort(td2);
sleepq_abort(td2, EINTR);
break;
case SINGLE_BOUNDARY:
if (TD_IS_SUSPENDED(td2) &&
@ -770,7 +770,7 @@ thread_single(int mode)
thread_unsuspend_one(td2);
if (TD_ON_SLEEPQ(td2) &&
(td2->td_flags & TDF_SINTR))
sleepq_abort(td2);
sleepq_abort(td2, ERESTART);
break;
default:
if (TD_IS_SUSPENDED(td2))
@ -894,12 +894,12 @@ thread_suspend_check(int return_instead)
return (0); /* Exempt from stopping. */
}
if ((p->p_flag & P_SINGLE_EXIT) && return_instead)
return (1);
return (EINTR);
/* Should we goto user boundary if we didn't come from there? */
if (P_SHOULDSTOP(p) == P_STOPPED_SINGLE &&
(p->p_flag & P_SINGLE_BOUNDARY) && return_instead)
return (1);
return (ERESTART);
/* If thread will exit, flush its pending signals */
if ((p->p_flag & P_SINGLE_EXIT) && (p->p_singlethread != td))

View File

@ -318,8 +318,10 @@ sleepq_add(void *wchan, struct mtx *lock, const char *wmesg, int flags)
mtx_lock_spin(&sched_lock);
td->td_wchan = wchan;
td->td_wmesg = wmesg;
if (flags & SLEEPQ_INTERRUPTIBLE)
if (flags & SLEEPQ_INTERRUPTIBLE) {
td->td_flags |= TDF_SINTR;
td->td_flags &= ~TDF_SLEEPABORT;
}
mtx_unlock_spin(&sched_lock);
}
@ -345,55 +347,64 @@ sleepq_set_timeout(void *wchan, int timo)
/*
* Marks the pending sleep of the current thread as interruptible and
* makes an initial check for pending signals before putting a thread
* to sleep.
* to sleep. Return with sleep queue and scheduler lock held.
*/
int
static int
sleepq_catch_signals(void *wchan)
{
struct sleepqueue_chain *sc;
struct sleepqueue *sq;
struct thread *td;
struct proc *p;
int sig;
struct sigacts *ps;
int sig, ret;
td = curthread;
p = td->td_proc;
p = curproc;
sc = SC_LOOKUP(wchan);
mtx_assert(&sc->sc_lock, MA_OWNED);
MPASS(td->td_sleepqueue == NULL);
MPASS(wchan != NULL);
CTR3(KTR_PROC, "sleepq catching signals: thread %p (pid %ld, %s)",
(void *)td, (long)p->p_pid, p->p_comm);
(void *)td, (long)p->p_pid, p->p_comm);
/* Mark thread as being in an interruptible sleep. */
MPASS(td->td_flags & TDF_SINTR);
MPASS(TD_ON_SLEEPQ(td));
sleepq_release(wchan);
mtx_unlock_spin(&sc->sc_lock);
/* See if there are any pending signals for this thread. */
PROC_LOCK(p);
mtx_lock(&p->p_sigacts->ps_mtx);
ps = p->p_sigacts;
mtx_lock(&ps->ps_mtx);
sig = cursig(td);
mtx_unlock(&p->p_sigacts->ps_mtx);
if (sig == 0 && thread_suspend_check(1))
sig = SIGSTOP;
PROC_UNLOCK(p);
if (sig == 0) {
mtx_unlock(&ps->ps_mtx);
ret = thread_suspend_check(1);
MPASS(ret == 0 || ret == EINTR || ret == ERESTART);
} else {
if (SIGISMEMBER(ps->ps_sigintr, sig))
ret = EINTR;
else
ret = ERESTART;
mtx_unlock(&ps->ps_mtx);
}
/*
* If there were pending signals and this thread is still on
* the sleep queue, remove it from the sleep queue. If the
* thread was removed from the sleep queue while we were blocked
* above, then clear TDF_SINTR before returning.
*/
sleepq_lock(wchan);
sq = sleepq_lookup(wchan);
mtx_lock_spin(&sched_lock);
if (TD_ON_SLEEPQ(td) && sig != 0)
sleepq_resume_thread(sq, td, -1);
else if (!TD_ON_SLEEPQ(td) && sig == 0)
if (ret) {
PROC_UNLOCK(p);
/*
* If there were pending signals and this thread is still on
* the sleep queue, remove it from the sleep queue.
*/
mtx_lock_spin(&sc->sc_lock);
sq = sleepq_lookup(wchan);
mtx_lock_spin(&sched_lock);
if (TD_ON_SLEEPQ(td))
sleepq_resume_thread(sq, td, -1);
td->td_flags &= ~TDF_SINTR;
mtx_unlock_spin(&sched_lock);
return (sig);
} else {
mtx_lock_spin(&sc->sc_lock);
mtx_lock_spin(&sched_lock);
PROC_UNLOCK(p);
}
return (ret);
}
/*
@ -409,6 +420,7 @@ sleepq_switch(void *wchan)
td = curthread;
sc = SC_LOOKUP(wchan);
mtx_assert(&sc->sc_lock, MA_OWNED);
mtx_assert(&sched_lock, MA_OWNED);
/*
* If we have a sleep queue, then we've already been woken up, so
@ -417,16 +429,13 @@ sleepq_switch(void *wchan)
if (td->td_sleepqueue != NULL) {
MPASS(!TD_ON_SLEEPQ(td));
mtx_unlock_spin(&sc->sc_lock);
mtx_lock_spin(&sched_lock);
return;
}
/*
* Otherwise, actually go to sleep.
*/
mtx_lock_spin(&sched_lock);
mtx_unlock_spin(&sc->sc_lock);
sched_sleep(td);
TD_SET_SLEEPING(td);
mi_switch(SW_VOL, NULL);
@ -485,54 +494,21 @@ sleepq_check_signals(void)
mtx_assert(&sched_lock, MA_OWNED);
td = curthread;
/*
* If TDF_SINTR is clear, then we were awakened while executing
* sleepq_catch_signals().
*/
if (!(td->td_flags & TDF_SINTR))
return (0);
/* We are no longer in an interruptible sleep. */
td->td_flags &= ~TDF_SINTR;
if (td->td_flags & TDF_SINTR)
td->td_flags &= ~TDF_SINTR;
if (td->td_flags & TDF_SLEEPABORT) {
td->td_flags &= ~TDF_SLEEPABORT;
return (td->td_intrval);
}
if (td->td_flags & TDF_INTERRUPT)
return (td->td_intrval);
return (0);
}
/*
* If we were in an interruptible sleep and we weren't interrupted and
* didn't timeout, check to see if there are any pending signals and
* which return value we should use if so. The return value from an
* earlier call to sleepq_catch_signals() should be passed in as the
* argument.
*/
int
sleepq_calc_signal_retval(int sig)
{
struct thread *td;
struct proc *p;
int rval;
td = curthread;
p = td->td_proc;
PROC_LOCK(p);
mtx_lock(&p->p_sigacts->ps_mtx);
/* XXX: Should we always be calling cursig()? */
if (sig == 0)
sig = cursig(td);
if (sig != 0) {
if (SIGISMEMBER(p->p_sigacts->ps_sigintr, sig))
rval = EINTR;
else
rval = ERESTART;
} else
rval = 0;
mtx_unlock(&p->p_sigacts->ps_mtx);
PROC_UNLOCK(p);
return (rval);
}
/*
* Block the current thread until it is awakened from its sleep queue.
*/
@ -541,6 +517,7 @@ sleepq_wait(void *wchan)
{
MPASS(!(curthread->td_flags & TDF_SINTR));
mtx_lock_spin(&sched_lock);
sleepq_switch(wchan);
mtx_unlock_spin(&sched_lock);
}
@ -552,11 +529,18 @@ sleepq_wait(void *wchan)
int
sleepq_wait_sig(void *wchan)
{
int rcatch;
int rval;
sleepq_switch(wchan);
rcatch = sleepq_catch_signals(wchan);
if (rcatch == 0)
sleepq_switch(wchan);
else
sleepq_release(wchan);
rval = sleepq_check_signals();
mtx_unlock_spin(&sched_lock);
if (rcatch)
return (rcatch);
return (rval);
}
@ -570,6 +554,7 @@ sleepq_timedwait(void *wchan)
int rval;
MPASS(!(curthread->td_flags & TDF_SINTR));
mtx_lock_spin(&sched_lock);
sleepq_switch(wchan);
rval = sleepq_check_timeout();
mtx_unlock_spin(&sched_lock);
@ -581,18 +566,23 @@ sleepq_timedwait(void *wchan)
* it is interrupted by a signal, or it times out waiting to be awakened.
*/
int
sleepq_timedwait_sig(void *wchan, int signal_caught)
sleepq_timedwait_sig(void *wchan)
{
int rvalt, rvals;
int rcatch, rvalt, rvals;
sleepq_switch(wchan);
rcatch = sleepq_catch_signals(wchan);
if (rcatch == 0)
sleepq_switch(wchan);
else
sleepq_release(wchan);
rvalt = sleepq_check_timeout();
rvals = sleepq_check_signals();
mtx_unlock_spin(&sched_lock);
if (signal_caught || rvalt == 0)
if (rcatch)
return (rcatch);
if (rvals)
return (rvals);
else
return (rvalt);
return (rvalt);
}
/*
@ -825,13 +815,14 @@ sleepq_remove(struct thread *td, void *wchan)
* Also, whatever the signal code does...
*/
void
sleepq_abort(struct thread *td)
sleepq_abort(struct thread *td, int intrval)
{
void *wchan;
mtx_assert(&sched_lock, MA_OWNED);
MPASS(TD_ON_SLEEPQ(td));
MPASS(td->td_flags & TDF_SINTR);
MPASS(intrval == EINTR || intrval == ERESTART);
/*
* If the TDF_TIMEOUT flag is set, just leave. A
@ -843,6 +834,10 @@ sleepq_abort(struct thread *td)
CTR3(KTR_PROC, "sleepq_abort: thread %p (pid %ld, %s)",
(void *)td, (long)td->td_proc->p_pid, (void *)td->td_proc->p_comm);
wchan = td->td_wchan;
if (wchan != NULL) {
td->td_intrval = intrval;
td->td_flags |= TDF_SLEEPABORT;
}
mtx_unlock_spin(&sched_lock);
sleepq_remove(td, wchan);
mtx_lock_spin(&sched_lock);

View File

@ -343,7 +343,7 @@ struct thread {
#define TDF_TIMEOUT 0x00000010 /* Timing out during sleep. */
#define TDF_IDLETD 0x00000020 /* This is a per-CPU idle thread. */
#define TDF_SELECT 0x00000040 /* Selecting; wakeup/waiting danger. */
#define TDF_UNUSED7 0x00000080 /* --available -- */
#define TDF_SLEEPABORT 0x00000080 /* sleepq_abort was called. */
#define TDF_TSNOBLOCK 0x00000100 /* Don't block on a turnstile due to race. */
#define TDF_UNUSED9 0x00000200 /* --available -- */
#define TDF_BOUNDARY 0x00000400 /* Thread suspended at user boundary */

View File

@ -87,12 +87,10 @@ struct thread;
#define SLEEPQ_INTERRUPTIBLE 0x100 /* Sleep is interruptible. */
void init_sleepqueues(void);
void sleepq_abort(struct thread *td);
void sleepq_abort(struct thread *td, int intrval);
void sleepq_add(void *, struct mtx *, const char *, int);
struct sleepqueue *sleepq_alloc(void);
void sleepq_broadcast(void *, int, int);
int sleepq_calc_signal_retval(int sig);
int sleepq_catch_signals(void *wchan);
void sleepq_free(struct sleepqueue *);
void sleepq_lock(void *);
struct sleepqueue *sleepq_lookup(void *);
@ -101,7 +99,7 @@ void sleepq_remove(struct thread *, void *);
void sleepq_signal(void *, int, int);
void sleepq_set_timeout(void *wchan, int timo);
int sleepq_timedwait(void *wchan);
int sleepq_timedwait_sig(void *wchan, int signal_caught);
int sleepq_timedwait_sig(void *wchan);
void sleepq_wait(void *);
int sleepq_wait_sig(void *wchan);