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

WIP: remove signal_fn in executor. #430

Closed
wants to merge 2 commits into from
Closed

WIP: remove signal_fn in executor. #430

wants to merge 2 commits into from

Conversation

Dirbaio
Copy link
Member

@Dirbaio Dirbaio commented Oct 16, 2021

The dynamic call into signal_fn here is rather slow (20-30 cycles). It would be cool to get rid of it.

However, it is needed to allow having multiple executors with different signal_fn's at the same time. For example, one WFE executor and one interrupt executor. It is not possible to use generics instead, as they would have to infect Executor, TaskStorage, TaskHeader, and we would still need a dynamic call when waking because now there can be multiple TaskHeader types. Not cool.

Solution: drop the possibility of mixing WFE and interrupt executors. This is not that big of a downside: if you want just one executor, use WFE. If you want multiple, just use all interrupt executors. YOu'll need to use up one extra hardware priority level, but these are usually plentiful.

(possibility of mixing could be added back by having ExecutorWaker store which kind it is and do an if. Not zero cost but probably still faster than a dyn call)

TODO fix stm32, rp.
TODO add cargo features.

@Dirbaio Dirbaio marked this pull request as draft October 16, 2021 02:09
@lulf
Copy link
Member

lulf commented Oct 18, 2021

I think it's reasonable with the extra constraint and workaround until there is an actual use case for having different signal_fns for different executors.

@Dirbaio
Copy link
Member Author

Dirbaio commented Oct 19, 2021

One disadvantage of only using InterruptExecutor is you're no longer running anything in thread mode, so ThreadModeMutex becomes useless. embassy-net has an internal ThreadModeMutex, but removing it is probably a good idea either way. Just have the user pass a &'static Stack around. nrf-softdevice works like that and it's not the end of the world.

We could have ThreadModeMutex-equivalents for interrupts. The downside is "Check current prio equals X" is a bit slower (you have to get the current irqn from IPSR, then look up its prio in NVIC.IPRx). "Check current irqn is X" is unsound because you can change the prio for that irq.

@osobiehl
Copy link

osobiehl commented May 3, 2022

Hi, I'm not sure where to write this but I wanted your input on the use of the sev() instruction as the signal_fn in the thread mode executor. For context I am trying to get a quick port of Embassy working on a RISCV ESP32C3 board
@Dirbaio
From my understanding

  • The thread mode executor is named as such because it is like a 'main' thread blocking all progress until an interrupt occurs after it polls tasks
  • signalling to the thread mode executor is done via its signal_fn callback, which just triggers a signal event instruction cortex_m::asm::sev()
  • HOWEVER: thread mode executors do not carry out polling while they are in an interrupt handler. Some parts (such as setting a new alarm for the timerqueue) occur in an interrupt-free critical section, but they will only work after being woken up by an interrupt
  • once an interrupt is done polling, it goes back to a wait for event wfe() instruction

However, suppose that after polling new tasks in the runqueue, the executor schedules a new alarm for a timestamp that has already passed. Depending on how the time driver is implemented, this would

  • automatically trigger alarm callback to let the executor know there is work to do, this would happen synchronously, meaning that the executor would not be in a sleeping state, so it would have no event to trigger it again!

or

  • an interrupt would be triggered after leaving the critical section responsible for scheduling the alarm. However, there is no guarantee that the interrupt will not trigger immediately after we leave the critical section and before the next wfe() instruction is executed, meaning that we miss a wakeup and the board stays waiting for an event indefinitely

Since I don't own any cortex_m board, I am unable to test if this is generally an issue for existing boards or if it is mentioned somewhere as the limitation of the thread mode executor. Sorry if I'm being stupid!

@Dirbaio
Copy link
Member Author

Dirbaio commented May 3, 2022

@osobiehl

WFE/SEV work like this:

  • sev sets an "event" flag register
  • wfe waits until the "event" flag is set, clears it, and returns. And, more importantly: if the flag was already set, it clears it and returns immediately. It doesn't wait.

So it's OK if the sev gets executed at a time while no wfe is running, the event gets "stored for later" in the flag, and when the wfe runs later, it still "sees" it.

I don't know if there's an equivalent in RISCV, If not, it could be done with a global AtomicBool to store the "have work to do" flag.

BTW you might want to get in touch with @MabezDev and the other foks from Espressif, they're also interested in this, see #745 . Also, come join the chat! :D

@Dirbaio
Copy link
Member Author

Dirbaio commented Sep 26, 2022

Probably not gonna happen anytime soon.

@Dirbaio Dirbaio closed this Sep 26, 2022
@Dirbaio Dirbaio deleted the fun branch March 2, 2023 00:23
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

Successfully merging this pull request may close these issues.

3 participants