Skip to content

RT: re-read the system state while waiting for it - #114

Open
andyp1per wants to merge 1 commit into
ArduPilot:masterfrom
andyp1per:pr-wait-system-state
Open

andyp1per wants to merge 1 commit into
ArduPilot:masterfrom
andyp1per:pr-wait-system-state

Conversation

@andyp1per

Copy link
Copy Markdown
Contributor

chSysWaitSystemState() polls ch_system.state, which another core writes, but the field is not volatile, so the compiler reads it once and then spins on the cached value. A core that calls it before the other core's chSysInit() has finished never leaves. This reads the state through a volatile pointer.

Seen in an RP2350 build (gcc 10-2020-q4), where core1 calls it from c1_main():

ldrb r3, [r3, #0]   ; state read once, before the loop
cmp  r3, r0         ; the loop compares the cached copy
bne  <cmp>

Found on an RPI_UAVFC after cutting core1's process stack from 16 KB to 1 KB: the board stopped booting. Read over SWD without halting, core1's PC stayed in that loop while ch_system.state in memory was already ch_sys_running. The larger stack had been hiding the race, because core1's CRT0 spent long enough filling it that core0 finished first. With this change the same build boots and core1 runs normally.

demos/RP/RT-RP2040-PICO/c1_main.c calls it the same way, so any SMP port whose second core reaches it early enough can hit this. The same fix is also carried in #113.

chSysWaitSystemState() polls ch_system.state, which another core writes,
but the field is not volatile. The compiler loads it once before the loop
and then compares the cached value forever, so a core that arrives before
chSysInit() has finished on the other core never leaves.

Found on RP2350, where core1 calls it from c1_main(): with its process
stack cut from 16 KB to 1 KB, core1 no longer spent long enough in the
CRT0 stack fill to arrive after core0, and hung with the kernel state
already running.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant