tests/periph_timer_timeout0: Regression test for timer callbacks setting new timers #10019

jnohlgard · 2018-09-24T07:06:39Z

Contribution description

This test is designed to catch an implementation bug where a timer
callback is called directly from inside timer_set if the given
timeout=0, leading to a stack overflow if timer_set is called from
within the callback of the same timer.

The bug is present on the current revision of the Kinetis LPTMR periph
driver. A bug fix is provided in #10020.

Testing procedure

build and flash tests/periph_timer_timeout0, run make test

Issues/PRs references

#10020 for the bug fix

stale · 2020-04-10T12:25:12Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

MrKevinWeiss · 2020-04-14T13:08:34Z

This is useful just needs some review time.

MrKevinWeiss · 2022-08-30T18:31:11Z

I rebased and reran but it seems like some things are not so happy

Teufelchen1 · 2024-01-25T14:03:41Z

Rebased and moved in the new test folder. Test passed on my nrf52dk.

riot-ci · 2024-01-25T14:04:11Z

Murdock results

❌ FAILED

8f00189 tests: Integrate zero timeout test into timer test

Success	Failures	Total	Runtime
57	1	59	01m:48s

Test failures (1)

Application	Target	Toolchain	Runtime (s)	Worker
tests/periph/timer	samr21-xpro	gnu	24.15	pi-ef3229ad

Artifacts

Teufelchen1 · 2024-03-18T16:07:33Z

The newly introduced tests fails on the samr21-xpro. I think the timer on that board might have a bug.

maribu · 2024-03-18T17:06:02Z

I have that board at hands. I do like the test, but I dislike that it creates a new app rather than extending the existing tests/periph/timer app with a new test case. The app isn't that large and will still fit RAM and ROM of all relevant boards.

Consolidating the number of apps a bit reduces CI time and increase the life of boards used for testing by reducing the number of flash cycles per full test run.

Teufelchen1 · 2024-03-19T16:10:03Z

@maribu like this? (And still fails on the samr21xpro)

Teufelchen1 · 2024-04-04T16:48:31Z

There have been some recent CI failures for the tests/sys/shell. I think that is related as the failure occurs on the samr21-xpro when compiled using LLVM inside a periodic timer test.

E.g. see this.


-- running on worker pi-68cf63c4 thread 1, build number 3102.
-- executing tests for tests/sys/shell on samr21-xpro (compiled with llvm toolchain):
- executing hook run_test_pre
- resetting USB power...
- waiting for tty to re-attach...
- hook run_test_pre finished
make: Entering directory '/tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/tests/sys/shell'
/bin/sh: 1: arm-none-eabi-gcc: not found
/tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/dist/tools/edbg/edbg.sh flash /tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/build/tests_shell.bin
### Flashing Target ###
Debugger: ATMEL EDBG CMSIS-DAP ATML2127031800002288 02.01.0157 (S)
Clock frequency: 16.0 MHz
Target: SAM R21G18 (Rev B)
Verification...... done.
Done flashing
make: Nothing to be done for 'termdeps'.
make: Leaving directory '/tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/tests/sys/shell'
make: Entering directory '/tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/tests/sys/shell'
/bin/sh: 1: arm-none-eabi-gcc: not found


/bin/sh: 1: arm-none-eabi-gcc: not found
socat - open:/dev/ttyACM0,b115200,echo=0,raw,cs8,parenb=0,cstopb=0 


> 

> bufsize
bufsize

60
> ____________________________________________________verylong
____________________________________________________verylong

shell: maximum line length exceeded
> garbage1234��
garbage1234

> 

> ��
shell exited
> 

> ��
periodic 5
> 

> periodic 5

> test
Timeout in expect script at "child.expect_exact('test')" (tests/sys/shell/tests/01-run.py:214)

make: *** [/tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/makefiles/tests/tests.inc.mk:26: test] Error 1
make: Leaving directory '/tmp/dwq.0.8417650337034919/b0edffdfccc3b6b961cda4d7e8c9893f/tests/sys/shell'

…ing new timers This test is designed to catch an implementation bug where a timer callback is called directly from inside timer_set if the given timeout=0, leading to a stack overflow if timer_set is called from within the callback of the same timer. The bug is present on the current revision of the Kinetis LPTMR periph driver. A bug fix is provided in a separate commit.

mguetschow · 2024-06-11T09:26:46Z

tests/periph/timer/main.c

@@ -207,6 +239,53 @@ static int test_timer(unsigned num, uint32_t timer_freq)
    return 1;
 }

+


Suggested change

mguetschow · 2024-06-11T09:32:28Z

tests/periph/timer/main.c

+        timer_set(ctx->dev, chan, 20000u);
+        timer_set(ctx->dev, chan, 0);
+    }
+    mutex_unlock(&ctx->mtx);


Can someone explain to me the purpose of the mutex? It is locked in test_timer_timeout and unlocked in the timer callback?

That's a common pattern in RIOT for a thread to wait for an IRQ. Unlike a POSIX mutex, a RIOT mutex can be unlocked from a different context (and even from IRQ context).

The idea is that locking an already locked mutex will block. Unlocking that mutex from the ISR will unblock the thread.

mguetschow · 2024-06-11T09:32:55Z

tests/periph/timer/main.c

+    /* Wait until we have executed the zero timeout callback enough times */
+    while (ctx.counter < TEST_ITERATIONS) {
+        mutex_lock(&ctx.mtx);
+        ++switches;


switches is unused from here on?

jnohlgard added this to the Release 2018.10 milestone Sep 24, 2018

jnohlgard mentioned this pull request Sep 24, 2018

kinetis: lptmr reload instead of spinning #10020

Merged

jnohlgard force-pushed the pr/test-periph-timer-timeout0 branch from a974b23 to 0dc18fd Compare September 24, 2018 07:23

jia200x modified the milestones: Release 2018.10, Release 2019.01 Oct 25, 2018

aabadie modified the milestones: Release 2019.01, Release 2019.04 Jan 25, 2019

PeterKietzmann requested a review from MichelRottleuthner March 22, 2019 12:57

miri64 modified the milestones: Release 2019.04, Release 2019.07 May 7, 2019

MrKevinWeiss self-assigned this May 28, 2019

MrKevinWeiss modified the milestones: Release 2019.07, Release 2019.10 Jul 16, 2019

kb2ma removed this from the Release 2019.10 milestone Oct 8, 2019

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Apr 10, 2020

MrKevinWeiss added the State: don't stale State: Tell state-bot to ignore this issue label Apr 14, 2020

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Apr 14, 2020

MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

Teufelchen1 force-pushed the pr/test-periph-timer-timeout0 branch from 0dc18fd to 6cdf4c9 Compare January 25, 2024 14:01

jnohlgard requested review from leandrolanzieri and aabadie as code owners January 25, 2024 14:01

github-actions bot added Area: doc Area: Documentation and removed Area: timers Area: timer subsystems labels Jan 25, 2024

github-actions bot removed the Area: doc Area: Documentation label Mar 19, 2024

Teufelchen1 mentioned this pull request Apr 4, 2024

periph/flashpage: remove deprecated flashpage_*_free functions #18093

Merged

Joakim Nohlgård and others added 2 commits May 7, 2024 15:59

tests: Integrate zero timeout test into timer test

8f00189

Teufelchen1 force-pushed the pr/test-periph-timer-timeout0 branch from 23de238 to 8f00189 Compare May 7, 2024 13:59

mguetschow added this to the Release 2024.07 milestone Jun 11, 2024

mguetschow reviewed Jun 11, 2024

View reviewed changes

mguetschow removed this from the Release 2024.07 milestone Jul 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/periph_timer_timeout0: Regression test for timer callbacks setting new timers #10019

tests/periph_timer_timeout0: Regression test for timer callbacks setting new timers #10019

jnohlgard commented Sep 24, 2018 •

edited

Loading

stale bot commented Apr 10, 2020

MrKevinWeiss commented Apr 14, 2020

MrKevinWeiss commented Aug 30, 2022

Teufelchen1 commented Jan 25, 2024

riot-ci commented Jan 25, 2024 •

edited

Loading

Teufelchen1 commented Mar 18, 2024

maribu commented Mar 18, 2024

Teufelchen1 commented Mar 19, 2024 •

edited

Loading

Teufelchen1 commented Apr 4, 2024

mguetschow Jun 11, 2024

mguetschow Jun 11, 2024

maribu Jun 11, 2024

mguetschow Jun 11, 2024

		@@ -207,6 +239,53 @@ static int test_timer(unsigned num, uint32_t timer_freq)
		return 1;
		}

tests/periph_timer_timeout0: Regression test for timer callbacks setting new timers #10019

Are you sure you want to change the base?

tests/periph_timer_timeout0: Regression test for timer callbacks setting new timers #10019

Conversation

jnohlgard commented Sep 24, 2018 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

stale bot commented Apr 10, 2020

MrKevinWeiss commented Apr 14, 2020

MrKevinWeiss commented Aug 30, 2022

Teufelchen1 commented Jan 25, 2024

riot-ci commented Jan 25, 2024 • edited Loading

Murdock results

Artifacts

Teufelchen1 commented Mar 18, 2024

maribu commented Mar 18, 2024

Teufelchen1 commented Mar 19, 2024 • edited Loading

Teufelchen1 commented Apr 4, 2024

mguetschow Jun 11, 2024

Choose a reason for hiding this comment

mguetschow Jun 11, 2024

Choose a reason for hiding this comment

maribu Jun 11, 2024

Choose a reason for hiding this comment

mguetschow Jun 11, 2024

Choose a reason for hiding this comment

jnohlgard commented Sep 24, 2018 •

edited

Loading

riot-ci commented Jan 25, 2024 •

edited

Loading

Teufelchen1 commented Mar 19, 2024 •

edited

Loading