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

Calling vTaskSuspend() on both cores can result in lost ticks (IDF-183) #1952

Closed
projectgus opened this issue May 14, 2018 · 4 comments
Closed

Comments

@projectgus
Copy link
Contributor

projectgus commented May 14, 2018

Originally reported by @ourairquality here:
#1400 (comment)

I'm able to reproduce the issue as follows:

  • Both cores call vTaskSuspendAll() (in either order)
  • More than one tick interrupt passes (causing uxPendedTicks to be non-zero)
  • Core 1 calls xTaskResumeAll() before Core 0 (causing uxPendedTicks to be zeroed without the tick count incrementing)

I believe all three conditions are required for the bug.

The tick count is lower than it should be, compared to the passage of time. This causes timeouts to occur later than they should, and using the tick count for wall time measurements will be inaccurate.

@ourairquality, please let me know if this matches the circumstances you're seeing.

AFAIK it's not very common to disable scheduler on both cores for extended periods (rather than using critical sections), which I think is probably why this hasn't been noticed until now. However we will fix obviously fix it!

@projectgus projectgus changed the title Calling vTaskSuspend() on both cores can result in lost ticks [TW#22508] Calling vTaskSuspend() on both cores can result in lost ticks May 14, 2018
@ourairquality
Copy link

Thank you. It might be far more common as it appears that accessing the flash, via spi_flash_read() etc results in both cores calling vTaskSuspendAll(). It probably requires a tick interrupt to occur on Core 0, but that seems likely. It is then a race to see which core calls vTaskSuspendAll() first. A workaround for that path might be possible in spi_flash_enable_interrupts_caches_and_other_cpu() - have Core 1 spin until Core 0 has resumed, but that would not address the issue in general.

@projectgus
Copy link
Contributor Author

Yes, you're right about spi_flash access. Good point, thanks.

@X-Ryl669
Copy link
Contributor

Could the problem be solved by having each core deal with its own uxPendingTick (without interfering with the other) ?

@projectgus projectgus changed the title [TW#22508] Calling vTaskSuspend() on both cores can result in lost ticks Calling vTaskSuspend() on both cores can result in lost ticks (IDF-183) Mar 15, 2019
@igrr igrr closed this as completed in 8e434c1 May 15, 2019
@ourairquality
Copy link

Nice to see a fix, thanks. Looking at the patch it appears that vApplicationTickHook() is not called now when processing the pended ticks, that code has been removed. Might that cause a loss for an app that had a hook to be called once for each tick, that counted them. I don't use that hook, just questioning if calling that hook could be usefully retained. If would appear that keeping that code could result in that hook being called from core 1, rather than core 0 in the common paths, and was that a problem and the reason this code was removed?

igrr pushed a commit that referenced this issue Jun 30, 2019
xTaskIncrementTick have to unwind uxPendedTicks on CPU1 and CPU0.

Use case: If an erase operation was run on the CPU1 then it leads
to starving other tasks which waiting time. Waited tasks just skipped.

Closes: #1952

Closes: IDF-183
igrr pushed a commit that referenced this issue Jun 30, 2019
xTaskIncrementTick have to unwind uxPendedTicks on CPU1 and CPU0.

Use case: If an erase operation was run on the CPU1 then it leads
to starving other tasks which waiting time. Waited tasks just skipped.

Closes: #1952

Closes: IDF-183
trombik pushed a commit to trombik/esp-idf that referenced this issue Aug 9, 2019
xTaskIncrementTick have to unwind uxPendedTicks on CPU1 and CPU0.

Use case: If an erase operation was run on the CPU1 then it leads
to starving other tasks which waiting time. Waited tasks just skipped.

Closes: espressif#1952

Closes: IDF-183
igrr pushed a commit that referenced this issue Aug 20, 2019
xTaskIncrementTick have to unwind uxPendedTicks on CPU1 and CPU0.

Use case: If an erase operation was run on the CPU1 then it leads
to starving other tasks which waiting time. Waited tasks just skipped.

Closes: #1952

Closes: IDF-183
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants