-
Notifications
You must be signed in to change notification settings - Fork 2.1k
core/mutex: use thread_yield_higher() in mutex_unlock() #20890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Using `sched_switch()` in `mutex_unlock()` can result in crashes when `mutex_unlock()` is called from IRQ context. This however is a common pattern in RIOT to wake up a thread from IRQ. The reason for the crash is that `sched_switch()` assumes `thread_get_active()` to always return a non-`NULL` value. But when thread-less idle is used, no thread may be active after the last runnable thread exited. Using `thread_yield_higher()` instead solves the issue, as `thread_yield_higher()` is safe to call from IRQ context without an active thread. This fixes RIOT-OS#20812
With this patch master
this PR
|
That is expected: The use of |
IMHO that's even wrong - if the high prio thread is running, calling Only yielding to higher priority threads is much more sound. |
Neither The hot path is no waiter with a mutex anyway, and that is actually optimised anyway (with no call to either). So this would be net positive from a performance PoV IMO. |
Contribution description
Using
sched_switch()
inmutex_unlock()
can result in crashes whenmutex_unlock()
is called from IRQ context. This however is a common pattern in RIOT to wake up a thread from IRQ. The reason for the crash is thatsched_switch()
assumesthread_get_active()
to always return a non-NULL
value. But when thread-less idle is used, no thread may be active after the last runnable thread exited. Usingthread_yield_higher()
instead solves the issue, asthread_yield_higher()
is safe to call from IRQ context without an active thread.Testing procedure
As described in #20812 (also add the
assert(active_thread != NULL);
tosched_switch()
to not rely on undefined behavior triggering the crash, but to crash reliably on invalid state).In addition to fixing the crash, it should not introduce regressions. I'm not sure how good the test coverage for mutex is, but at least the following passed:
for test in tests/core/mutex_*; do make BOARD=nucleo-f767zi -C $test flash test; done
Issues/PRs references
This fixes #20812
Alternative to #20878