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

Goodby Exclusive, Welcome Symmetrical Lock and [mut/&resource] optimization. #17

Closed
perlindgren opened this issue Sep 26, 2019 · 2 comments

Comments

@perlindgren
Copy link
Contributor

perlindgren commented Sep 26, 2019

Currently RTFM has the notion of asymmetric access to shared resources. Tasks with highest priority accessing shared:T gets a &mut T API, while other tasks (if any) gets a resource proxy, with a lock API (former claim). The examples/lock.rs from the current upstream.

#[rtfm::app(device = lm3s6965)]
const APP: () = {
    struct Resources {
        #[init(0)]
        shared: u32,
    }

    #[init]
    fn init(_: init::Context) {
        rtfm::pend(Interrupt::GPIOA);
    }

    // when omitted priority is assumed to be `1`
    #[task(binds = GPIOA, resources = [shared])]
    fn gpioa(mut c: gpioa::Context) {
        hprintln!("A").unwrap();

        // the lower priority task requires a critical section to access the data
        c.resources.shared.lock(|shared| {
            // data can only be modified within this critical section (closure)
            *shared += 1;

            // GPIOB will *not* run right now due to the critical section
            rtfm::pend(Interrupt::GPIOB);

            hprintln!("B - shared = {}", *shared).unwrap();

            // GPIOC does not contend for `shared` so it's allowed to run now
            rtfm::pend(Interrupt::GPIOC);
        });

        // critical section is over: GPIOB can now start

        hprintln!("E").unwrap();

        debug::exit(debug::EXIT_SUCCESS);
    }

    #[task(binds = GPIOB, priority = 2, resources = [shared])]
    fn gpiob(c: gpiob::Context) {
        // the higher priority task does *not* need a critical section
        *c.resources.shared += 1;

        hprintln!("D - shared = {}", *c.resources.shared).unwrap();
    }

    #[task(binds = GPIOC, priority = 3)]
    fn gpioc(_: gpioc::Context) {
        hprintln!("C").unwrap();
    }
};

This is unfortunate, as

  1. Introducing another task in the application may break code. gpiob would need to be changed API if allowing gpioc access to shared.
  2. If we want gpioa to pass the resource access to another (external) function (like a HAL), we would need to wrap shared by Exclusive (which gives a lock API to inner).
  3. Context cannot have 'static lifetime if allowed to have &mut T in resources. This will prevent from captures in static stored generators (which requires 'static lifetime). Generators can be useful as is (initial experiments confirm the possibility), and are used under the hood for async/await, so if we want to keep this door open, we will eventually run into this problem.

Proposal.

Offer only a single API (always resource proxy) for all shared resources.
Pros:

  • Symmetrical API with no risk of breakage.
  • Mitigates the need for Exclusive.
  • Potentially reducing the complexity of codegen internally in RTFM. (Right now there a bunch of edge cases in the codegen).

Cons:

  • More verbose user code, the price we pay for ensuring composition property. With the new multi-lock Mutex (korken), we can reduce rightward drift.

Performance:

  • Locks will be optimized out by the compiler in case your task has exclusive access.

[resource] vs [mut resource] (or [&resource] vs [resource])
In a related RFC, Jorge proposed to distinguish between mutable and immutable access. With the above proposal, this will amount in a slight syntax extension. (Following Rust I would think that [mut resource] would be appropriate, but the &syntax would be backwards compatible, so I don't have a strong opinion).

Pros:

  • Improved schedulability (less blocking from lower priority tasks)
  • Improved performance (unnecessary locks are optimized out at compile time)

Cons:

  • None.

Under the "Goodby Exclusive" proposal, the optimization can be done fully transparent to the API. lock will lend out &T/&mut T, for immutable/mutable access respectively and the compiler will block misuse.

I might have missed something, looking forward to input.
Best regards
Per

@MabezDev
Copy link

MabezDev commented Jan 8, 2020

The non symetrical API also introduces issues when using the resource proxy concrete types because the higher prio task doesn't get a resource proxy, instead it gets the resource directly. Example:

#[rtfm::app(device = lm3s6965)]
const APP: () = {
    struct Resources {
        // A resource
        #[init(0)]
        shared: u32,
    }

    #[task(resources = [shared])
    fn normal_prio(c: normal_prio::Context) {
        takes_proxy(c.resources.shared); // works
    }

    #[task(resources = [shared], priority = 5)
    fn high_prio(c: high_prio::Context) {
        // fails to compile, expected type `resources::shared` got `&mut u32`
        takes_proxy(c.resources.shared); 
    }

};

fn takes_proxy(shared: resources::shared) {
    shared.lock(|_s| {});
}

@perlindgren
Copy link
Contributor Author

Close as solved.

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

2 participants