sched: separate out schedinit_ap()

schedinit_ap() sets up an AP for a later call to sched_throw(NULL).

Currently, ULE sets up some pcpu bits and fixes the idlethread lock with
a call to sched_throw(NULL); this results in a window where curthread is
setup in platforms' init_secondary(), but it has the wrong td_lock.
Typical platform AP startup procedure looks something like:

- Setup curthread
- ... other stuff, including cpu_initclocks_ap()
- Signal smp_started
- sched_throw(NULL) to enter the scheduler

cpu_initclocks_ap() may have callouts to process (e.g., nvme) and
attempt to sched_add() for this AP, but this attempt fails because
of the noted violated assumption leading to locking heartburn in
sched_setpreempt().

Interrupts are still disabled until cpu_throw() so we're not really at
risk of being preempted -- just let the scheduler in on it a little
earlier as part of setting up curthread.

Reviewed by:	alfredo, kib, markj
Triage help from:	andrew, markj
Smoke-tested by:	alfredo (ppc), kevans (arm64, x86), mhorne (arm)
Differential Revision:	https://reviews.freebsd.org/D32797
This commit is contained in:
Kyle Evans 2021-11-02 13:06:47 -05:00
parent ae49051c03
commit 589aed00e3
10 changed files with 43 additions and 7 deletions

View File

@ -182,6 +182,7 @@ init_secondary(int cpu)
pc->pc_curthread = pc->pc_idlethread;
pc->pc_curpcb = pc->pc_idlethread->td_pcb;
set_curthread(pc->pc_idlethread);
schedinit_ap();
#ifdef VFP
vfp_init();
#endif

View File

@ -255,6 +255,7 @@ init_secondary(uint64_t cpu)
/* Initialize curthread */
KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
pcpup->pc_curthread = pcpup->pc_idlethread;
schedinit_ap();
/* Initialize curpmap to match TTBR0's current setting. */
pmap0 = vmspace_pmap(&vmspace0);

View File

@ -678,6 +678,13 @@ schedinit(void)
mtx_init(&sched_lock, "sched lock", NULL, MTX_SPIN);
}
void
schedinit_ap(void)
{
/* Nothing needed. */
}
int
sched_runnable(void)
{

View File

@ -1743,6 +1743,26 @@ schedinit(void)
ts0->ts_cpu = curcpu; /* set valid CPU number */
}
/*
* schedinit_ap() is needed prior to calling sched_throw(NULL) to ensure that
* the pcpu requirements are met for any calls in the period between curthread
* initialization and sched_throw(). One can safely add threads to the queue
* before sched_throw(), for instance, as long as the thread lock is setup
* correctly.
*
* TDQ_SELF() relies on the below sched pcpu setting; it may be used only
* after schedinit_ap().
*/
void
schedinit_ap(void)
{
#ifdef SMP
PCPU_SET(sched, DPCPU_PTR(tdq));
#endif
PCPU_GET(idlethread)->td_lock = TDQ_LOCKPTR(TDQ_SELF());
}
/*
* This is only somewhat accurate since given many processes of the same
* priority they will switch when their slices run out, which will be
@ -2973,19 +2993,14 @@ sched_throw(struct thread *td)
struct thread *newtd;
struct tdq *tdq;
tdq = TDQ_SELF();
if (__predict_false(td == NULL)) {
#ifdef SMP
PCPU_SET(sched, DPCPU_PTR(tdq));
#endif
/* Correct spinlock nesting and acquire the correct lock. */
tdq = TDQ_SELF();
TDQ_LOCK(tdq);
/* Correct spinlock nesting. */
spinlock_exit();
PCPU_SET(switchtime, cpu_ticks());
PCPU_SET(switchticks, ticks);
PCPU_GET(idlethread)->td_lock = TDQ_LOCKPTR(tdq);
} else {
tdq = TDQ_SELF();
THREAD_LOCK_ASSERT(td, MA_OWNED);
THREAD_LOCKPTR_ASSERT(td, TDQ_LOCKPTR(tdq));
tdq_load_rem(tdq, td);

View File

@ -311,6 +311,7 @@ smp_init_secondary(u_int32_t cpuid)
/* Initialize curthread. */
KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
PCPU_SET(curthread, PCPU_GET(idlethread));
schedinit_ap();
mtx_lock_spin(&ap_boot_mtx);

View File

@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
#include <sys/bus.h>
#include <sys/pcpu.h>
#include <sys/proc.h>
#include <sys/sched.h>
#include <sys/smp.h>
#include <machine/bus.h>
@ -134,6 +135,7 @@ cpudep_ap_bootstrap(void)
#endif
pcpup->pc_curpcb = pcpup->pc_curthread->td_pcb;
sp = pcpup->pc_curpcb->pcb_sp;
schedinit_ap();
return (sp);
}

View File

@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
#include <sys/bus.h>
#include <sys/pcpu.h>
#include <sys/proc.h>
#include <sys/sched.h>
#include <sys/smp.h>
#include <machine/pcb.h>
@ -85,6 +86,7 @@ cpudep_ap_bootstrap()
#endif
pcpup->pc_curpcb = pcpup->pc_curthread->td_pcb;
sp = pcpup->pc_curpcb->pcb_sp;
schedinit_ap();
/* XXX shouldn't the pcb_sp be checked/forced for alignment here?? */

View File

@ -248,6 +248,7 @@ init_secondary(uint64_t hart)
/* Initialize curthread */
KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
pcpup->pc_curthread = pcpup->pc_idlethread;
schedinit_ap();
/*
* Identify current CPU. This is necessary to setup

View File

@ -226,6 +226,11 @@ SYSINIT(name, SI_SUB_LAST, SI_ORDER_MIDDLE, name ## _add_proc, NULL);
* Fixup scheduler state for proc0 and thread0
*/
void schedinit(void);
/*
* Fixup scheduler state for secondary APs
*/
void schedinit_ap(void);
#endif /* _KERNEL */
/* POSIX 1003.1b Process Scheduling */

View File

@ -1040,6 +1040,7 @@ init_secondary_tail(void)
/* Initialize curthread. */
KASSERT(PCPU_GET(idlethread) != NULL, ("no idle thread"));
PCPU_SET(curthread, PCPU_GET(idlethread));
schedinit_ap();
mtx_lock_spin(&ap_boot_mtx);