-
Notifications
You must be signed in to change notification settings - Fork 3k
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
ST boards: Fix sleep tracing #13034
ST boards: Fix sleep tracing #13034
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we remove usage of singleton_mutex_id
in this file, then does it still need to be in mbed rtos start ? it is defined there and initialized but not used. Only used here in this file.
Good to have these issues fixed!
I'm a bit wary of this - But what is the actual problem with the original code? I still don't understand the failure mode from our discussion. Is |
Another possibility along the same lines - how about if the crashy thing we put into the mutex error callback chose not to crash if the kernel isn't running? That would have the general effect of Maybe that would cover up too much. But the main thing we were trying to catch with the crash was the "call from IRQ" mistake - that's definitely a bogus thing to do. But if the kernel is not running, then a failed mutex lock attempt is not really harmful. (Assuming they unlock before the kernel is started again). |
Yes, it is. Okay, then setting it at the start of We're already assuming single-threaded at least until that call is made - but that could spawn new threads in object constructors. |
@hugueskamba, thank you for your changes. |
e79ae2f
to
37d5713
Compare
Thanks. Done. |
37d5713
to
6d4a0cd
Compare
6d4a0cd
to
14bde89
Compare
int main(void); | ||
static void mbed_cpy_nvic(void); | ||
void mbed_rtos_init_singleton_mutex(void); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be properly publicly declared in mbed_boot.h. mbed_rtx_rtos.c doesn't seem to have its own header, but it includes mbed_boot.h, and that's where mbed_rtos_start
is declared.
(Although maybe everything else there is public. Stick a @internal or whatever Doxygen uses on it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Everything is already internal documentation.
Prevent singleton lock if the RTOS is not yet ready. lp_ticker is used during the RTOS initialization process. ST lp_ticker implementation calls sleep functions which in turn attempts to print to the console when sleep tracing is enabled. Console initialization attempts to lock the singleton mutex.
14bde89
to
5d94fd4
Compare
Allow singleton_lock and singleton_unlock to be called before the RTOS has been started by checking for a valid mutex before locking and unlocking it.
CI started |
Test run: SUCCESSSummary: 6 of 6 test jobs passed |
This PR does not contain release version label after merging. |
Summary of changes
Prevent singleton lock if the RTOS is not yet ready.
lp_ticker is used during the RTOS initialization process.
ST lp_ticker implementation calls sleep functions
which in turn attempts to print to the console when sleep tracing
is enabled. Console initialization attempts to lock the singleton mutex.
fixes #11497
fixes #12593
To test:
mbed-os-example-blinky
.mbed compile -t <TOOLCHAIN> -m <ST_TARGET> -f --sterm -DMBED_SLEEP_TRACING_ENABLED
.Impact of changes
Sleep tracing can be enabled with ST boards.
Migration actions required
N/A
Documentation
N/A
Pull request type
Test results
Reviewers
@kjbracey-arm @c1728p9 @korjaa @LMESTM @evedon