Page MenuHomeFreeBSD

Fix an occasional hang in smp_after_idle_runnable
AbandonedPublic

Authored by mhorne on Sep 20 2020, 10:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 14, 2:22 PM
Unknown Object (File)
Thu, Mar 28, 9:57 AM
Unknown Object (File)
Dec 20 2023, 1:35 AM
Unknown Object (File)
Nov 8 2023, 9:18 PM
Unknown Object (File)
Nov 1 2023, 6:33 PM
Unknown Object (File)
Oct 9 2023, 12:45 AM
Unknown Object (File)
Oct 7 2023, 8:10 PM
Unknown Object (File)
Sep 30 2023, 6:37 PM
Subscribers
None

Details

Reviewers
jhb
jrtc27
markj
Summary

Okay, here's a fun one.

In smp_after_idle_runnable() we execute the following loop to ensure
APs enter the scheduler before freeing their boot stack:

pc = pcpu_find(cpu);
while (atomic_load_ptr(&pc->pc_curpcb) == NULL)
    cpu_spinwait();
kmem_free((vm_offset_t)bootstacks[cpu], PAGE_SIZE);

Strangely, the resulting assembly for this loop is:

ffffffc00044dbe6:   ld      a0,32(a0)
ffffffc00044dbe8:   bnez    a0,ffffffc00044dbaa <smp_after_idle_runnable+0x26>
ffffffc00044dbea:   j       ffffffc00044dbea <smp_after_idle_runnable+0x66>

Which will spin forever if pc->pc_curpcb == NULL on the first check.

This usually doesn't happen as APs enter the scheduler quickly, relative
to the BSP entering smp_after_idle_runnable. However, I've noticed that
when booting with loader(8) and -smp 2, the secondary processor will
sometimes get interrupted to handle disk I/O before it is finished in
init_secondary(), which increases the window for this to occur.

Our definition of cpu_spinwait() on RISC-V is empty, so as a
workaround defining it as a single nop instruction seems to fix the
issue.

I have two follow-up questions to this:

Is this a compiler bug, or is this empty while loop abusing some
undefined behaviour? I'll note that gcc doesn't emit the same sequence,
but so far I've had trouble replicating this as a smaller test case.

Second, would it be desirable to ensure that APs don't receive external
interrupts until they have finished init_secondary()? I can't name
an explicit harm from allowing this, but it definitely caused me some
confusion while debugging. The reason it is currently possible is that
intr_irq_shuffle() executes at SI_ORDER_SECOND, while
smp_after_idle_runnable() executes at SI_ORDER_ANY.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 33676
Build 30915: arc lint + arc unit