kernel/timers: fix priority-inversion livelock in timer_hs/cancel_timer_hs #564
Burer
Doczom
Egor00f
rgimad
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: KolibriOS/kolibrios#564
Reference in New Issue
Block a user
I wrote an autotest that runs Netsurf a lot of times. While running this
test I always faced the hang issue. First thought was about network
and sockets but QEMU debug through telnet revealed the issue in timers.
Now there is no hang after this fix.
timer_hs and cancel_timer_hs hold the global timer-list lock
(lock_timer_list/unlock_timer_list) across a short critical section
of plain list-pointer stores, without disabling interrupts. If the
owning thread is preempted inside that window, it can never be
scheduled again while a higher-priority thread is runnable: the
scheduler is strictly priority-based and osloop's check_timers (top
priority) busy-waits for the same lock via change_task, which never
descends to a lower-priority ring while its own ring has a runnable
thread. The result is a permanent, whole-system livelock -- not a
one-off race, but a deterministic outcome whenever the preemption
lands inside the section.
Reproduced by driving thousands of blocking TCP connects (each one
calls timer_hs for its connect timeout) through a browser under QEMU,
where interrupts tend to land on translation-block boundaries right
after the lock's cmpxchg. Confirmed via the QEMU monitor: osloop
spinning forever in lock_timer_list while the lock owner sat, fully
preempted, one instruction into timer_hs's critical section.
Fix: wrap the lock/insert-or-remove/unlock sequence in
timer_hs and cancel_timer_hs with pushfd/cli ... popfd, making it
atomic with respect to preemption. The wait loop inside
lock_timer_list is unaffected -- change_task manages IF on its own,
so spinning there with interrupts off cannot itself hang anything.
Good work!
There is one small improvement you can do regarding future-proofing.
The current fix will not hold when SMP comes into play. Therefore, it would be nicer if you use the macros from macros.inc to realize the pushf/cli popf.
They are placeholders for when SMP comes into play.
I dont remember the names by heart, so let me know if you dont find what I mean :)
Sorry, macros I remembered are only in drivers/macros.inc
Lets ignore this future issue for now then.