Skip to content
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

Sporadic scheduler: Fix time calculation and compile errors when assertions are enabled: #3111

Merged
merged 3 commits into from Mar 20, 2021

Conversation

patacongo
Copy link
Contributor

@patacongo patacongo commented Mar 19, 2021

Summary

Correct elapsed time calculation Elapsed time calculation must always be be the current time minus a time in the past. Not vice versa. Related Issue #2935

Fix missing semicolon at the end of a DEBUGASSERT statement:

sched/sched_sporadic.c: In function 'sporadic_budget_expire':
sched/sched_sporadic.c:512:15: error: expected ';' before 'period'
  512 |               period = (sporadic->repl_period >> 1) - unrealized;
      |               ^~~~~~
sched/sched_sporadic.c: In function 'nxsched_resume_sporadic':
sched/sched_sporadic.c:1078:19: error: expected ';' before 'period'
 1078 |                   period = (sporadic->repl_period >> 1) - unrealized;
      |                   ^~~~~~

Fix use of uninitialized variable in DEBUGASSERT statement:

sched/sched_sporadic.c:466:27: warning: 'sporadic' may be used uninitialized in this function [-Wmaybe-uninitialized]
  466 |                   sporadic->nrepls > 0);

Also fixes a couple of small typos.

Impact

There should be no unexpected side-effects of this changed.

Testing

Tested with the stm32f4discovery:sporadic configuration (see PR #3097)

@patacongo patacongo changed the title Sporadic scheduler: Fix compile errors when assertions are enabled: Sporadic scheduler: Fix time calculation and compile errors when assertions are enabled: Mar 19, 2021
Copy link
Contributor

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Fix missing semicolon at the end of a DEBUGASSERT statement:

sched/sched_sporadic.c: In function 'sporadic_budget_expire':
sched/sched_sporadic.c:512:15: error: expected ';' before 'period'
  512 |               period = (sporadic->repl_period >> 1) - unrealized;
      |               ^~~~~~
sched/sched_sporadic.c: In function 'nxsched_resume_sporadic':
sched/sched_sporadic.c:1078:19: error: expected ';' before 'period'
 1078 |                   period = (sporadic->repl_period >> 1) - unrealized;
      |                   ^~~~~~

Fix use of uninitialized variable in DEBUGASSERT statement:

sched/sched_sporadic.c:466:27: warning: 'sporadic' may be used uninitialized in this function [-Wmaybe-uninitialized]
  466 |                   sporadic->nrepls > 0);

Also fixes some typos.

There should be no unexpected side-effects of this changed.

Tested with the stm32f4discovery:sporadic configuration (see PR apache#3097
Elapsed time calculation must always be be the current time minus a time in the past.  Not vice versa.

Also corrects and improves some comments.
-CONFIG_USEC_PER_TICK=10000
+CONFIG_USEC_PER_TICK=1000

A system timer with a 10 MS period is not sufficient to run the dual thread sporadic scheduler test since the timings in that test are also around 10 MS.  Apparently there is a race condition when both sporadic thread's budgets complete on the same clock time.  This change does not eliminate the race, but reduces its effect greatly.
@xiaoxiang781216 xiaoxiang781216 merged commit 2775cad into apache:master Mar 20, 2021
@patacongo patacongo deleted the debugassert branch March 20, 2021 13:34
@btashton btashton added this to To-Add in Release Notes - 10.1.0 Apr 12, 2021
@jerpelea jerpelea moved this from To-Add to Minor in Release Notes - 10.1.0 Apr 12, 2021
@jerpelea jerpelea moved this from Minor to core in Release Notes - 10.1.0 Apr 14, 2021
@jerpelea jerpelea moved this from core to Added in Release Notes - 10.1.0 Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants