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

riot-rs-threads: WFI not working as expected on riscv/ esp #310

Closed
elenaf9 opened this issue Jun 3, 2024 · 5 comments
Closed

riot-rs-threads: WFI not working as expected on riscv/ esp #310

elenaf9 opened this issue Jun 3, 2024 · 5 comments
Labels

Comments

@elenaf9
Copy link
Collaborator

elenaf9 commented Jun 3, 2024

While testing #304 I noticed that the WFI (wait for interrupt) in riot_rs_threads::arch::riscv::sched (when the runqueue is empty) doesn't work as expected, at least not when using it together with embassy timers.

The expected behavior would be that when the scheduler wfi, an embassy timer that expires would cause an interrupt and the scheduler would wake up again.

It works for the cortex_m impl as expected, so it's most likely that:
a) we are using the WFI mechanism for riscv wrong and/or a missing something or
b) there is some issue related to the embassy timer implementation for esp

I'll look into it.

elenaf9 added a commit to elenaf9/RIOT-rs that referenced this issue Jun 4, 2024
Fixes future-proof-iot#310 for the system timer interrupt here.
@elenaf9
Copy link
Collaborator Author

elenaf9 commented Jun 5, 2024

It works for the cortex_m impl as expected, so it's most likely that:

This is actually not correct. I ran into the same issue for the nrf52840dk when using the executor-thread from #304 and running this simple tasks:

// Will run in a thread when the `executor-thread` feature is enabled
#[riot_rs::task(autostart)]
async fn task() {
    riot_rs::debug::println!("[task] Hello");
    embassy_time::Timer::after(embassy_time::Duration::from_secs(5)).await;
    riot_rs::debug::println!("[task] Hello again");
}

The execution stalls after the timer.

Interestingly, the issue here is different than for esp.

For esp, I noticed the issue when testing the esp-wifi thread (#300), because the systimer didn't trigger.
The WFI seems to block it. Increasing the priority of the systimer0 for the esp-wifi thread fixes it.
The above code sample on the other hand works for esp. Maybe because embassy internally already uses higher priorities for its timer interrupts?

For cortex-m on the other hand, the timer triggers but then the PendSV exception that should wake up the thread doesn't go through. Also, this only seems to be an issue when testing on nrf52840dk; on the rp2040 the PendSV exception goes through.

@kaspar030 any idea what the issue for cortex-m could be? I will do further debugging, but maybe you already know what the problem is?

@kaspar030
Copy link
Collaborator

For cortex-m on the other hand, the timer triggers but then the PendSV exception that should wake up the thread doesn't go through. Also, this only seems to be an issue when testing on nrf52840dk; on the rp2040 the PendSV exception goes through.

I can reproduce this on nrf52840dk. Just adding a println!() in there makes the thread wake up again. weird.

@kaspar030
Copy link
Collaborator

Adding a fence in the cortex-m sched() fixes the issue on nrf52840dk. @elenaf9 can you check if adding such a barrier fixes the risc-v case?
btw, on cortex-m, the pendsv is intentionally set to have the lowest priority (0xff), so other ISRs can interrupt. is sth similar configured on riscv?

@kaspar030
Copy link
Collaborator

For esp, I noticed the issue when testing the esp-wifi thread (#300), because the systimer didn't trigger.
The WFI seems to block it. Increasing the priority of the systimer0 for the esp-wifi thread fixes it.

ah, re-read and saw this now. The interrupt that does the "wfi" dance needs to be preemptible, so probably set to the lowest priority possible. (does that even work on risc-v?)

@elenaf9
Copy link
Collaborator Author

elenaf9 commented Jun 6, 2024

For esp, I noticed the issue when testing the esp-wifi thread (#300), because the systimer didn't trigger.
The WFI seems to block it. Increasing the priority of the systimer0 for the esp-wifi thread fixes it.

ah, re-read and saw this now. The interrupt that does the "wfi" dance needs to be preemptible, so probably set to the lowest priority possible. (does that even work on risc-v?)

Yes, I have set the FROM_CPU_INTR1 interrupt (that triggers the scheduler) at lowest prio.
The problem was that the systimer interrupt in esp-wifi was also set to lowest prio. So I guess for esp it's not a bug, but rather me not thinking the WFI mechanism through :)
If the interrupts in embassy are all set to a higher prio, this won't be a problem.

I'd close this issue as soon as #312 is merged, and reopen/ reiterate if we run into an scenario where we wait for an (embassy?) interrupt that doesn't have a high enough prio.

@elenaf9 elenaf9 closed this as completed Jun 10, 2024
elenaf9 added a commit to elenaf9/RIOT-rs that referenced this issue Jun 19, 2024
elenaf9 added a commit to elenaf9/RIOT-rs that referenced this issue Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants