Rename tdq_ipipending and clear it in sched_switch().

This fixes a regression after r355311.  Specifically, sched_preempt()
may trigger a context switch by calling thread_lock(), since
thread_lock() calls critical_exit() in its slow path and the interrupted
thread may have already been marked for preemption.  This would happen
before tdq_ipipending is cleared, blocking further preemption IPIs.  The
CPU can be left in this state indefinitely if the interrupted thread
migrates.

Rename tdq_ipipending to tdq_owepreempt.  Any switch satisfies a remote
preemption request, so clear tdq_owepreempt in sched_switch() instead of
sched_preempt() to avoid subtle problems of the sort described above.

Reviewed by:	jeff, kib
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D22758
This commit is contained in:
Mark Johnston 2019-12-12 02:43:24 +00:00
parent c260030436
commit 7789ab32b3
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=355644

View File

@ -244,7 +244,7 @@ struct tdq {
volatile short tdq_switchcnt; /* Switches this tick. */
volatile short tdq_oldswitchcnt; /* Switches last tick. */
u_char tdq_lowpri; /* Lowest priority thread. */
u_char tdq_ipipending; /* IPI pending. */
u_char tdq_owepreempt; /* Remote preemption pending. */
u_char tdq_idx; /* Current insert index. */
u_char tdq_ridx; /* Current removal index. */
int tdq_id; /* cpuid. */
@ -1073,7 +1073,7 @@ tdq_notify(struct tdq *tdq, struct thread *td)
int pri;
int cpu;
if (tdq->tdq_ipipending)
if (tdq->tdq_owepreempt)
return;
cpu = td_get_sched(td)->ts_cpu;
pri = td->td_priority;
@ -1096,7 +1096,12 @@ tdq_notify(struct tdq *tdq, struct thread *td)
if (!tdq->tdq_cpu_idle || cpu_idle_wakeup(cpu))
return;
}
tdq->tdq_ipipending = 1;
/*
* The run queues have been updated, so any switch on the remote CPU
* will satisfy the preemption request.
*/
tdq->tdq_owepreempt = 1;
ipi_cpu(cpu, IPI_PREEMPT);
}
@ -2079,8 +2084,10 @@ sched_switch(struct thread *td, struct thread *newtd, int flags)
(flags & SW_PREEMPT) != 0;
td->td_flags &= ~(TDF_NEEDRESCHED | TDF_SLICEEND);
td->td_owepreempt = 0;
tdq->tdq_owepreempt = 0;
if (!TD_IS_IDLETHREAD(td))
tdq->tdq_switchcnt++;
/*
* The lock pointer in an idle thread should never change. Reset it
* to CAN_RUN as well.
@ -2386,7 +2393,6 @@ sched_preempt(struct thread *td)
thread_lock(td);
tdq = TDQ_SELF();
TDQ_LOCK_ASSERT(tdq, MA_OWNED);
tdq->tdq_ipipending = 0;
if (td->td_priority > tdq->tdq_lowpri) {
int flags;
@ -2397,6 +2403,8 @@ sched_preempt(struct thread *td)
mi_switch(flags | SWT_REMOTEWAKEIDLE, NULL);
else
mi_switch(flags | SWT_REMOTEPREEMPT, NULL);
} else {
tdq->tdq_owepreempt = 0;
}
thread_unlock(td);
}