Date: Sun, 17 Sep 2017 03:09:57 +0000 From: bugzilla-noreply@freebsd.org To: freebsd-bugs@FreeBSD.org Subject: [Bug 222234] head -r323246 aarch64 (Pine64+ 2GB) boot time context, sometimes: acquiring blockable sleep lock with spinlock or critical section held Message-ID: <bug-222234-8-I8laftDYKn@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-222234-8@https.bugs.freebsd.org/bugzilla/> References: <bug-222234-8@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D222234 --- Comment #3 from Mark Millard <markmi@dsl-only.net> --- (In reply to Mark Millard from comment #2) The following notes are the observations that eventually got me to the point of checking out if the sev instruction was sufficient on its own. It may or may not point out other potential issues. The context here is a debug-kernel used to investigate the occasional boot failures for that context. . . When verbose boot messages are enabled there is an earlier contrast between when booting works overall vs. when it later fails: When it works: subsystem f000000 release_aps(0)... Release APs done. When it fails:=20 subsystem f000000 release_aps(0)... Release APs APs not started done. And it well explains why ->pc_curthread ends up NULL for secondaries (in particular cpu =3D=3D 1), init_secondary had never executed the assignments show below:=20 while (!aps_ready) __asm __volatile("wfe"); /* Initialize curthread */ KASSERT(PCPU_GET(idlethread) !=3D NULL, ("no idle thread")); pcpup->pc_curthread =3D pcpup->pc_idlethread; pcpup->pc_curpcb =3D pcpup->pc_idlethread->td_pcb; The subsystem messages are from: static void release_aps(void *dummy __unused) {=20=20=20=20=20=20=20 int i; /* Only release CPUs if they exist */ if (mp_ncpus =3D=3D 1) return; intr_pic_ipi_setup(IPI_AST, "ast", ipi_ast, NULL); intr_pic_ipi_setup(IPI_PREEMPT, "preempt", ipi_preempt, NULL); intr_pic_ipi_setup(IPI_RENDEZVOUS, "rendezvous", ipi_rendezvous, NULL); intr_pic_ipi_setup(IPI_STOP, "stop", ipi_stop, NULL); intr_pic_ipi_setup(IPI_STOP_HARD, "stop hard", ipi_stop, NULL); intr_pic_ipi_setup(IPI_HARDCLOCK, "hardclock", ipi_hardclock, NULL); atomic_store_rel_int(&aps_ready, 1); /* Wake up the other CPUs */ __asm __volatile("sev"); printf("Release APs\n"); for (i =3D 0; i < 2000; i++) { if (smp_started) return; DELAY(1000); } printf("APs not started\n"); }=20=20=20=20=20=20=20 SYSINIT(start_aps, SI_SUB_SMP, SI_ORDER_FIRST, release_aps, NULL); init_secondary has an example or two of not using atomic_load_acq_int when atomic_store_rel_int is in use. One is: while (!aps_ready) __asm __volatile("wfe"); /* Initialize curthread */ KASSERT(PCPU_GET(idlethread) !=3D NULL, ("no idle thread")); pcpup->pc_curthread =3D pcpup->pc_idlethread; pcpup->pc_curpcb =3D pcpup->pc_idlethread->td_pcb; where aps_ready was declared via: /* Set to 1 once we're ready to let the APs out of the pen. */ volatile int aps_ready =3D 0; where release_aps has the use of atomic_store_rel_int: atomic_store_rel_int(&aps_ready, 1); /* Wake up the other CPUs */ __asm __volatile("sev"); There is also in init_secondary: atomic_add_rel_32(&smp_cpus, 1); if (smp_cpus =3D=3D mp_ncpus) { /* enable IPI's, tlb shootdown, freezes etc */ atomic_store_rel_int(&smp_started, 1); } where smp_cpus is accessed without being explicitly atomic. mp_ncpus seems to have no atomic use at all. Where: /usr/src/sys/sys/smp.h:extern int smp_cpus; /usr/src/sys/kern/subr_smp.c:int smp_cpus =3D 1; /* how many cpu's running= */ So no "volatile", unlike the earlier example. /usr/src/sys/kern/kern_umtx.c: if (smp_cpus > 1) { /usr/src/sys/kern/subr_smp.c:SYSCTL_INT(_kern_smp, OID_AUTO, cpus, CTLFLAG_RD|CTLFLAG_CAPRD, &smp_cpus, 0, /usr/src/sys/sys/smp.h:extern int mp_ncpus; /usr/src/sys/kern/subr_smp.c:int mp_ncpus; The smp_started is not explicitly accessed as atomic in release_aps but in init_secondary has its update to 1 via: mtx_lock_spin(&ap_boot_mtx); atomic_add_rel_32(&smp_cpus, 1); if (smp_cpus =3D=3D mp_ncpus) { /* enable IPI's, tlb shootdown, freezes etc */ atomic_store_rel_int(&smp_started, 1); } mtx_unlock_spin(&ap_boot_mtx); where: /usr/src/sys/sys/smp.h:extern volatile int smp_started; /usr/src/sys/kern/subr_smp.c:volatile int smp_started; ("volatile" again for this context.) I'll also note that for the sparc64 architecture there is some code like: if (__predict_false(atomic_load_acq_int(&smp_started) =3D=3D 0)) that is explicitly matched to the atomic_store_rel_int in its mp_machdep.c . I do not have enough background aarch64 knowledge to know if it is provable that atomic_load_acq_int is not needed in some of these cases. But getting "APs not started" at least sometimes suggests an intermittent failure of the code as it is. Another difference is lack of explicit initialization of smp_started but explicit initialization of aps_ready and smp_cpus . I have no clue if the boot sequence is supposed to handle "APs not started" by reverting to not being a symmetric multiprocessing boot or some other specific way instead of trying to avoiding use of what was not initialized by: pcpup->pc_curthread =3D pcpup->pc_idlethread; pcpup->pc_curpcb =3D pcpup->pc_idlethread->td_pcb; in init_secondary. [The following notes are from later. . .] Example mp_ncpus and smp_cpus figures from a failed Pine64+ 2GB boot: db> print/x *smp_cpus 1 db> print/x *mp_ncpus 138800000004 But that should be a 4 byte width. Showing some context for reference: db> x/bx mp_ncpus-4,4=20 rebooting: 0 0 0 0 db> x/bx mp_ncpus,4 mp_ncpus: 4 0 0 0 db> x/bx mp_ncpus+4,4=20 scsi_delay: 88 13 0 0 For completeness: db> x/bx smp_cpus-4,4 sysctl___kern_smp_disabled+0x5c: 0 0 0 0 db> x/bx smp_cpus,4 smp_cpus: 1 0 0 0 db> x/bx smp_cpus+4,4=20 smp_cpus+0x4: 0 0 0 0 So smp_cpus was not incremented in memory. This goes along with no occurances of: pcpup->pc_curthread =3D pcpup->pc_idlethread; pcpup->pc_curpcb =3D pcpup->pc_idlethread->td_pcb; updates happening in init_secondary: /* Spin until the BSP releases the APs */ while (!aps_ready) __asm __volatile("wfe"); /* Initialize curthread */ KASSERT(PCPU_GET(idlethread) !=3D NULL, ("no idle thread")); pcpup->pc_curthread =3D pcpup->pc_idlethread; pcpup->pc_curpcb =3D pcpup->pc_idlethread->td_pcb; . . . mtx_lock_spin(&ap_boot_mtx); atomic_add_rel_32(&smp_cpus, 1); if (smp_cpus =3D=3D mp_ncpus) { /* enable IPI's, tlb shootdown, freezes etc */ atomic_store_rel_int(&smp_started, 1); } mtx_unlock_spin(&ap_boot_mtx); Which seems to imply that the aps_ready update: atomic_store_rel_int(&aps_ready, 1); /* Wake up the other CPUs */ __asm __volatile("sev"); in release_aps was not seen in the: while (!aps_ready) __asm __volatile("wfe"); in init_secondary. --=20 You are receiving this mail because: You are the assignee for the bug.=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-222234-8-I8laftDYKn>