debug_irq_disable: add module to debug time spent in irq_disable#18795
debug_irq_disable: add module to debug time spent in irq_disable#18795benpicco merged 3 commits intoRIOT-OS:masterfrom
Conversation
github-actions
bot
added
Area: core
benpicco
force-pushed
the
irq-track
branch
3 times, most recently
from
d92ac7a to
e7beb61
Compare
| */ | ||
| static inline void _irq_debug_start_count(void) | ||
| { | ||
| SysTick->VAL = 0; |
There was a problem hiding this comment.
wouldn't this work better if we keep the systick running and store the lock time
and than return reload_mask & ( unlock_time -locktime)
There was a problem hiding this comment.
Why do you think that's better? We'd need an extra variable and start / stop would no longer by symmetric, we'd need an extra function to start the SysTick.
There was a problem hiding this comment.
The way this PR does it is: ether use systick for this xor for something else (e.g.: your countdowntimer)
The way would allow you to have any Initialisation on that systick (counting down or systicking around) and do this and all that needs is static uint32_t
i am not sure if it also would be quicker since these are 3 memory write accesses and mine has 1 read and 1 write for start and this has 1 read and 1 write for stop vs mine has 2 reads.
There was a problem hiding this comment.
The Countdown timer wouldn't run the SysTick continuously either, only when there is a countdown scheduled.
It would be incompatible with this either way, if merged.
And I'm not that concerned about a few instructions here - the UART printouts will ensure that this is purely a debug feature and can't be used for anything performance critical 😉
jue89
left a comment
jue89
left a comment
There was a problem hiding this comment.
Nice PR! This seems to be helpful for optimizing IRQ latencies.
benpicco
requested review from
MrKevinWeiss,
jia200x and
leandrolanzieri
as code owners
jue89
added
the
CI: ready for build
Murdock results✔️ PASSED 59a3e61 cpu/cortexm_common: measure time spent with IRQ disabled
ArtifactsThis only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now. |
jue89
left a comment
jue89
left a comment
There was a problem hiding this comment.
ACK.
This module doesn't interfere if it's not explicitly selected.
Diff of FW bins compiled from tests/irq_disable_restore with and without this patch - debug_irq_disable not selected:
--- tests_irq_disable_restore-52aa12bd.bin.hex 2022-11-25 18:22:59.561436017 +0100
+++ tests_irq_disable_restore-59a3e613.bin.hex 2022-11-25 18:23:26.585889500 +0100
@@ -765,8 +765,8 @@
00002fc0: 1513 1007 1a0c 1206 0b05 0a09 6d61 696e ............main
00002fd0: 2829 3a20 5468 6973 2069 7320 5249 4f54 (): This is RIOT
00002fe0: 2120 2856 6572 7369 6f6e 3a20 3230 3233 ! (Version: 2023
-00002ff0: 2e30 312d 6465 7665 6c2d 3131 352d 6735 .01-devel-115-g5
-00003000: 3261 6131 2d48 4541 4429 006d 6169 6e00 2aa1-HEAD).main.
+00002ff0: 2e30 312d 6465 7665 6c2d 3131 382d 6735 .01-devel-118-g5
+00003000: 3961 3365 2d48 4541 4429 006d 6169 6e00 9a3e-HEAD).main.
00003010: 2a2a 2a20 5249 4f54 206b 6572 6e65 6c20 *** RIOT kernel
00003020: 7061 6e69 633a 0a25 730a 0a00 2a2a 2a20 panic:.%s...***
00003030: 6861 6c74 6564 2e0a 0073 6368 6564 756c halted...schedulIt's a useful tool to get a feeling how long IRQs are disabled and support debugging of high IRQ latencies.
5 participants
Contribution description
When writing code that needs to fulfill some kind of real-time grantees, it is often useful to know how much time was spent with interrupts disabled.
This adds a simple debug module that wraps
irq_restore()and uses SysTick to measure the time spent with IRQs disabled on Cortex-M.Testing procedure
Just add
to your application.
You might also want to add
Issues/PRs references