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

RTOS: Add tickles sleep implementation (enabled for FRDM boards) #2547

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@bulislaw
Member

bulislaw commented Aug 25, 2016

Implementation based on lp_timer, therefore it'll only be used on
platforms defining LOWPOWERTIMER. I've introduced additional flag,
TICKLESS_LP, that needs to be defined to enable tickless mode. For
now it's only enabled for K64F and K22F. Enabling it for other targets
requires more extended, per platform, testing to be performed first.

@0xc0170 Could you have a look please

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Aug 25, 2016

Member

/morph test

Member

bulislaw commented Aug 25, 2016

/morph test

@mbed-bot

This comment has been minimized.

Show comment
Hide comment
@mbed-bot

mbed-bot Aug 25, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 716

Test failed!

mbed-bot commented Aug 25, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 716

Test failed!

@bridadan

This comment has been minimized.

Show comment
Hide comment
@bridadan

bridadan Aug 25, 2016

Contributor

@bulislaw Looks like this caused a regression on all FRDM boards, please take a look at the results. For now, please hold off on merging this.

Contributor

bridadan commented Aug 25, 2016

@bulislaw Looks like this caused a regression on all FRDM boards, please take a look at the results. For now, please hold off on merging this.

@sg- sg- added the do not merge label Aug 25, 2016

@sg-

This comment has been minimized.

Show comment
Hide comment
@sg-

sg- Aug 25, 2016

Member

What does TICKLESS_LP require (or implement) that LOWPOWERTIMER doesn't?

Member

sg- commented Aug 25, 2016

What does TICKLESS_LP require (or implement) that LOWPOWERTIMER doesn't?

@sg- sg- added the needs: work label Aug 25, 2016

@0xc0170

This comment has been minimized.

Show comment
Hide comment
@0xc0170

0xc0170 Aug 26, 2016

Member

What does TICKLESS_LP require (or implement) that LOWPOWERTIMER doesn't?

I believe if a platform has low power and sleep, it should implicitly be supported for tickless mode?

Tests timeouts, for these 2 platforms that provide lp ticker and enable tickless mode

Member

0xc0170 commented Aug 26, 2016

What does TICKLESS_LP require (or implement) that LOWPOWERTIMER doesn't?

I believe if a platform has low power and sleep, it should implicitly be supported for tickless mode?

Tests timeouts, for these 2 platforms that provide lp ticker and enable tickless mode

@sg-

This comment has been minimized.

Show comment
Hide comment
@sg-

sg- Aug 26, 2016

Member

I believe if a platform has low power and sleep, it should implicitly be supported for tickless mode?

Yes

Member

sg- commented Aug 26, 2016

I believe if a platform has low power and sleep, it should implicitly be supported for tickless mode?

Yes

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Aug 30, 2016

Member

I'm stuck, I've tried to repro the failures locally using both GCC and ARM without any luck, I was trying to run only failing tests, larger blocks and whole suite multiple times on both boards but nothing. Also tried to power down the board before the tests. In the last act of desperation I've tried to use exactly the same version of GCC the CI is using, but it also pass. Any ideas what could be going on?

Member

bulislaw commented Aug 30, 2016

I'm stuck, I've tried to repro the failures locally using both GCC and ARM without any luck, I was trying to run only failing tests, larger blocks and whole suite multiple times on both boards but nothing. Also tried to power down the board before the tests. In the last act of desperation I've tried to use exactly the same version of GCC the CI is using, but it also pass. Any ideas what could be going on?

@0xc0170

This comment has been minimized.

Show comment
Hide comment
@0xc0170

0xc0170 Aug 30, 2016

Member

I'll have a look today, the failure is for all 3 toolchains almost identical tests (isr and mutex)

Member

0xc0170 commented Aug 30, 2016

I'll have a look today, the failure is for all 3 toolchains almost identical tests (isr and mutex)

@bridadan

This comment has been minimized.

Show comment
Hide comment
@bridadan

bridadan Aug 30, 2016

Contributor

Did you merge with master/rebase first? The CI first merges with master. In this particular case, the CI merged your commit ba5c72e4ae4864e23ffde452d19c054b37749ffe into commit 6a1208a from the master branch.

Contributor

bridadan commented Aug 30, 2016

Did you merge with master/rebase first? The CI first merges with master. In this particular case, the CI merged your commit ba5c72e4ae4864e23ffde452d19c054b37749ffe into commit 6a1208a from the master branch.

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Aug 30, 2016

Member

Yeah I did a rebase (git pull --rebase upstream master) before pushing to the branch.

Member

bulislaw commented Aug 30, 2016

Yeah I did a rebase (git pull --rebase upstream master) before pushing to the branch.

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Aug 31, 2016

Member

I've rebased the code and enabled it for all platforms supporting LOWPOWERTIMER and SLEEP. I couldn't test it on NRF51_DK, I have the board, but it won't run tests I'm getting timeouts without anything actually running. I'll kick the test run here.

Member

bulislaw commented Aug 31, 2016

I've rebased the code and enabled it for all platforms supporting LOWPOWERTIMER and SLEEP. I couldn't test it on NRF51_DK, I have the board, but it won't run tests I'm getting timeouts without anything actually running. I'll kick the test run here.

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Aug 31, 2016

Member

/morph test

Member

bulislaw commented Aug 31, 2016

/morph test

@bridadan

This comment has been minimized.

Show comment
Hide comment
@bridadan

bridadan Aug 31, 2016

Contributor

@bulislaw I'm going to have to restart this build unfortunately. Sorry about that!

Contributor

bridadan commented Aug 31, 2016

@bulislaw I'm going to have to restart this build unfortunately. Sorry about that!

@mbed-bot

This comment has been minimized.

Show comment
Hide comment
@mbed-bot

mbed-bot Aug 31, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 757

Build failed!

mbed-bot commented Aug 31, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 757

Build failed!

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Aug 31, 2016

Member

@bridadan please remember to restart the job when you're ready.

Member

bulislaw commented Aug 31, 2016

@bridadan please remember to restart the job when you're ready.

@bridadan

This comment has been minimized.

Show comment
Hide comment
@bridadan

bridadan Aug 31, 2016

Contributor

/morph test

Contributor

bridadan commented Aug 31, 2016

/morph test

@mbed-bot

This comment has been minimized.

Show comment
Hide comment
@mbed-bot

mbed-bot Aug 31, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 762

Test failed!

mbed-bot commented Aug 31, 2016

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 762

Test failed!

@bridadan

This comment has been minimized.

Show comment
Hide comment
@bridadan

bridadan Aug 31, 2016

Contributor

@bulislaw I'm having trouble reproducing the failure as well, both locally and on the CI machine. It seems to occur only with the test system is under heavy load (aka when its running all the tests on all the platforms). It's strange that I don't see these particular failures on other PRs though. Is it possible that this PR would be affecting the speed at which the tests run? That of course shouldn't affect the testing tools, but at this point I'm not willing to rule it out.

cc @mazimkhan

Contributor

bridadan commented Aug 31, 2016

@bulislaw I'm having trouble reproducing the failure as well, both locally and on the CI machine. It seems to occur only with the test system is under heavy load (aka when its running all the tests on all the platforms). It's strange that I don't see these particular failures on other PRs though. Is it possible that this PR would be affecting the speed at which the tests run? That of course shouldn't affect the testing tools, but at this point I'm not willing to rule it out.

cc @mazimkhan

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Sep 1, 2016

Member

@bridadan Well I don't know exactly how greentea works, but I would imagine the workload on the host wouldn't influence the execution on the target (I hope). I don't think speed should affect it at all, (ignoring NRF for now as I haven't tested them, I'm having problems running tests on them locally) the 'mutex' tests look like they are not really executed or freeze and the 'isr' seem to stuck after first iteration. Assuming they actually freeze, never wake up from the tickless sleep, but not sure how timing could affect that here.

Member

bulislaw commented Sep 1, 2016

@bridadan Well I don't know exactly how greentea works, but I would imagine the workload on the host wouldn't influence the execution on the target (I hope). I don't think speed should affect it at all, (ignoring NRF for now as I haven't tested them, I'm having problems running tests on them locally) the 'mutex' tests look like they are not really executed or freeze and the 'isr' seem to stuck after first iteration. Assuming they actually freeze, never wake up from the tickless sleep, but not sure how timing could affect that here.

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Sep 1, 2016

Member

And the NRF* boards seems to have lp_ticker implementation broken, the callback is not being called.

Member

bulislaw commented Sep 1, 2016

And the NRF* boards seems to have lp_ticker implementation broken, the callback is not being called.

RTOS: Add tickles sleep implementation (enabled for FRDM boards)
It's enabled by default for all platforms defining DEVICE_SLEEP and
DEVICE_LOWPOWERTIMER.
@0xc0170

This comment has been minimized.

Show comment
Hide comment
@0xc0170

0xc0170 Sep 2, 2016

Member

I created feature_rtos_tickless branch, you can send this patch against that branch.

Member

0xc0170 commented Sep 2, 2016

I created feature_rtos_tickless branch, you can send this patch against that branch.

@sg-

This comment has been minimized.

Show comment
Hide comment
@sg-

sg- Sep 8, 2016

Member

@bulislaw can this be closed given the new branch?

Member

sg- commented Sep 8, 2016

@bulislaw can this be closed given the new branch?

@bulislaw

This comment has been minimized.

Show comment
Hide comment
@bulislaw

bulislaw Sep 9, 2016

Member

Yes, will do it now.

Member

bulislaw commented Sep 9, 2016

Yes, will do it now.

@bulislaw bulislaw closed this Sep 9, 2016

@pan-

This comment has been minimized.

Show comment
Hide comment
@pan-

pan- Oct 17, 2016

Member

From what I see, the algorithm used for the tickless implementation is not correct. In that configuration, the device will remain asleep for the number of ticks obtained from os_suspend. It shouldn't because when an IRQ occur it can change the state of a task in the system (from blocked to pending).
Blocking the scheduling is not the correct behavior in this case.

Instead, after every wake up, the system should call os_resume to makes sure that if there is a pending task in the system, it is run right away. If there is no work left then the system will resume os_idle_demon (default_idle_hook in our case) and restart the sleep procedure.

An example is provided in the RTX documentation: http://elk.informatik.fh-augsburg.de/pub/stm32lab/libs/CMSIS-4.2/CMSIS_RTX/Doc/_timer_tick.html

Basically it is something like this:

while(true) { 
  uint32_t sleep_ticks = os_suspend();
  uint32_t slept_ticks = sleep_ticks;
  if (sleep_ticks) { 
     // Setup an alarm to wakeup in sleep_ticks
     setup_alarm_in(sleep_ticks);                  
     // read the time at which the device go to sleep 
     uint32_t sleep_time = lp_ticker_read(); 
     //go to sleep 
     sleep();                                                  

     // OK, the device has woken up 
     // clean up any state remaining from the alarm
     cleanup_alarm();                                    
     // compute the slept time, manage overflow, etc ... 
     slept_ticks = ticks_elapsed_from(sleep_time);
  }
  // resume the scheduling, indicating for how long the system was asleep.
  os_resume(slept_time);
}
Member

pan- commented Oct 17, 2016

From what I see, the algorithm used for the tickless implementation is not correct. In that configuration, the device will remain asleep for the number of ticks obtained from os_suspend. It shouldn't because when an IRQ occur it can change the state of a task in the system (from blocked to pending).
Blocking the scheduling is not the correct behavior in this case.

Instead, after every wake up, the system should call os_resume to makes sure that if there is a pending task in the system, it is run right away. If there is no work left then the system will resume os_idle_demon (default_idle_hook in our case) and restart the sleep procedure.

An example is provided in the RTX documentation: http://elk.informatik.fh-augsburg.de/pub/stm32lab/libs/CMSIS-4.2/CMSIS_RTX/Doc/_timer_tick.html

Basically it is something like this:

while(true) { 
  uint32_t sleep_ticks = os_suspend();
  uint32_t slept_ticks = sleep_ticks;
  if (sleep_ticks) { 
     // Setup an alarm to wakeup in sleep_ticks
     setup_alarm_in(sleep_ticks);                  
     // read the time at which the device go to sleep 
     uint32_t sleep_time = lp_ticker_read(); 
     //go to sleep 
     sleep();                                                  

     // OK, the device has woken up 
     // clean up any state remaining from the alarm
     cleanup_alarm();                                    
     // compute the slept time, manage overflow, etc ... 
     slept_ticks = ticks_elapsed_from(sleep_time);
  }
  // resume the scheduling, indicating for how long the system was asleep.
  os_resume(slept_time);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment