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

AVR thread yield #5745

Closed
jthacker opened this issue Aug 12, 2016 · 4 comments
Closed

AVR thread yield #5745

jthacker opened this issue Aug 12, 2016 · 4 comments
Assignees
Labels
Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@jthacker
Copy link
Contributor

I believe the AVR thread yield is not implemented correctly. thread_arch_yield (aka thread_yield_higher) is currently implemented as a naked function that will begin execution of the new thread at the same level as the calling point. In the atmega version of timer.c, thread_yield_higher is called in an ISR, which can result in long running functions at this level possible halting the execution of all other threads. https://github.com/RIOT-OS/RIOT/blob/master/cpu/atmega_common/periph/timer.c#L193

I discovered this when working with xtimer. It would consistently run the spin loop version instead of sleeping the thread due to running at interrupt level. https://github.com/RIOT-OS/RIOT/blob/master/sys/xtimer/xtimer.c#L45

A possible method for eliminating this issue is to use an interrupt driven thread swap like on the other platforms. For example, on the cortex platform, an interrupt is triggered and will be serviced as normal, the resulting context swap will place the new thread at the normal execution level since it will only start execution after returning the thread swapping ISR returns. Unlike the cortex platform, AVRs do not have a dedicated software driven interrupt for context swaps.

I have implemented a method for doing this by using one of the external interrupt pins, while keeping it as an output and driving it through software. master...jthacker:avr_thread_arch_isr_stack_usage

CONTEXT_SWAP_INIT, CONTEXT_SWAP_INT_VECT and CONTEXT_SWAP_TRIGGER would need to implemented by each board since this requires sacrificing a physical pin on each board, and may be dependent on the layout. Obviously this is not ideal. For this reason I have opened this as an issue and not a pull request to see if firstly there is agreement with the issue at hand and to discuss possible alternative solutions.

@LudwigKnuepfer LudwigKnuepfer added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: AVR Platform: This PR/issue effects AVR-based platforms labels Aug 12, 2016
@kaspar030
Copy link
Contributor

Wow, this might explain a lot of trouble we've been having with AVR's interrupts behaving ... weird.

@kaspar030
Copy link
Contributor

Obviously this is not ideal.

Quick googling suggests that this might be the only way to have proper ISR handling. So a PR would be very welcome.

@kYc0o kYc0o added this to the Release 2016.10 milestone Aug 19, 2016
@kYc0o
Copy link
Contributor

kYc0o commented Aug 19, 2016

I agree with Kaspar, this could be the origin of several trouble we have especially with timers, maybe this could fix it.

@jthacker feel free to open a PR from your branch. Notice that not all external interrupts are implemented for this MCU family, so maybe we can complete the support and use a free pin for context switching.

jthacker added a commit to jthacker/RIOT that referenced this issue Aug 20, 2016
Fixes RIOT-OS#5745
For AVR based boards, three defines must be defined AVR_CONTEXT_SWAP_INIT,
AVR_CONTEXT_SWAP_INTERRUPT_VECT, and AVR_CONTEXT_SWAP_TRIGGER.
These defines are used to trigger a software interrupt used for context
switching.

When AVR_CONTEXT_SWAP_INTERRUPT_VECT is handled, the scheduler is run
and a context swap will happen if necessary, with the resulting thread
starting following the reti instruction. This results in threads running
at normal priority instead of at interrupt priority.

Atmega devices do provide a pure software interrupt. The method used
here for waspmote-pro and arduino-mega2560 is to use pin change
interrupts, set the pin to act as an output, and toggle the value to
simulate a software interrupt. The main limitation here is that a
physical pin is now occupied and must be defined for each board
supported by RIOT. On the plus side, it provides an easy method for
detecting context swaps with an oscilloscope.
jthacker added a commit to jthacker/RIOT that referenced this issue Sep 1, 2016
Fixes RIOT-OS#5745
For AVR based boards, three defines must be defined AVR_CONTEXT_SWAP_INIT,
AVR_CONTEXT_SWAP_INTERRUPT_VECT, and AVR_CONTEXT_SWAP_TRIGGER.
These defines are used to trigger a software interrupt used for context
switching.

When AVR_CONTEXT_SWAP_INTERRUPT_VECT is handled, the scheduler is run
and a context swap will happen if necessary, with the resulting thread
starting following the reti instruction. This results in threads running
at normal priority instead of at interrupt priority.

Atmega devices do provide a pure software interrupt. The method used
here for waspmote-pro and arduino-mega2560 is to use pin change
interrupts, set the pin to act as an output, and toggle the value to
simulate a software interrupt. The main limitation here is that a
physical pin is now occupied and must be defined for each board
supported by RIOT. On the plus side, it provides an easy method for
detecting context swaps with an oscilloscope.
jthacker added a commit to jthacker/RIOT that referenced this issue Sep 7, 2016
Fixes RIOT-OS#5745
For AVR based boards, three defines must be defined AVR_CONTEXT_SWAP_INIT,
AVR_CONTEXT_SWAP_INTERRUPT_VECT, and AVR_CONTEXT_SWAP_TRIGGER.
These defines are used to trigger a software interrupt used for context
switching.

When AVR_CONTEXT_SWAP_INTERRUPT_VECT is handled, the scheduler is run
and a context swap will happen if necessary, with the resulting thread
starting following the reti instruction. This results in threads running
at normal priority instead of at interrupt priority.

Atmega devices do provide a pure software interrupt. The method used
here for waspmote-pro and arduino-mega2560 is to use pin change
interrupts, set the pin to act as an output, and toggle the value to
simulate a software interrupt. The main limitation here is that a
physical pin is now occupied and must be defined for each board
supported by RIOT. On the plus side, it provides an easy method for
detecting context swaps with an oscilloscope.
@roberthartung
Copy link
Member

For me this breaks the default example I get a kernel panic because the msg_queue is not initialized

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

5 participants