embassy-rs / embassy

Modern embedded framework, using Rust and async.
https://embassy.dev
Apache License 2.0
5.22k stars 720 forks source link

Executor is not fair if spi dma transfer are used #1061

Open MauroMombelli opened 1 year ago

MauroMombelli commented 1 year ago

Minimal example to reproduce the issue follow.

EXPECTED: START A and START B are both printed (order can be inverted) A and B are printed, in different order/frequency

OBSERVED: START A and A are NEVER printed. START B and B are always printed.

FUN FACT: Adding a third task, is also never run. "spi.transfer().await;" seems to monopolize the executor Adding a little delay with "Timer::after(Duration::from_millis(100)).await;" to the spi's task will let START A and A get printed. Also works if adding yield_now().await;

CODE:

#![no_std]
#![no_main]
#![feature(type_alias_impl_trait)]

use defmt::*;
use embassy_executor::Spawner;
use embassy_futures::yield_now;
use embassy_stm32::{spi::{Config, Spi}, peripherals::{SPI1, DMA1_CH3, DMA1_CH2}};

use {defmt_rtt as _, panic_probe as _};

// Declare async tasks
#[embassy_executor::task]
async fn a() {
    info!("START A");

    loop {
        info!("A");
        yield_now().await;
    }
}

#[embassy_executor::task]
async fn spi_poll(mut spi: Spi<'static, SPI1, DMA1_CH3, DMA1_CH2>) {
    info!("START B");

    loop {
        info!("B");
        let write: [u8; 7] = [0; 7];
        let mut read: [u8; 7] = [0; 7];
        let _ = spi.transfer(&mut read, &write).await;
    }
}

#[embassy_executor::main]
async fn main(spawner: Spawner) {

    let p = embassy_stm32::init(Default::default());

    info!("Hello World!");

    spawner.spawn(a()).unwrap();

    let spi = Spi::new(
        p.SPI1,
        p.PA5,
        p.PA7,
        p.PA6,
        p.DMA1_CH3,
        p.DMA1_CH2,
        embassy_stm32::time::Hertz(1_000_000),
        Config::default(),
    );

    spawner.spawn(spi_poll(spi)).unwrap();
}
yodaldevoid commented 1 year ago

I have also run into this issue, but when using embassy_rp instead. Adding a yield_now to my SPI task also addressed the problem.

nikvoid commented 1 year ago

This is happening because without yield_now().await you just do not have any await-points in a task, so it blocks entire executor in this endless loop

    loop {
        info!("A");
        yield_now().await; // await point, here executor can switch tasks
    }
lestofante commented 1 year ago

@nikvoid read the issue again, you are completely missing what is going on here

nikvoid commented 1 year ago

Oh, get it wrong, sorry