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

Adds Shared<T> a wrapper for sharing data between handlers #15

Closed

Conversation

HarkonenBade
Copy link

@HarkonenBade HarkonenBade commented Jan 10, 2019

This adds Shared<T> which is a helper for sharing values between the main thread and interrupt handlers. It is essentially a wrapper around Mutex<RefCell<Option<T>>> that adds some helper methods to make the standard ways of executing this common pattern less painful.

e.g. in one of the examples from the stm32f0xx-hal it makes the following simplifications:

Declaration

static SHARED: Mutex<RefCell<Option<Shared>>> = Mutex::new(RefCell::new(None));

to

static SHARED: Shared<MyShared> = Shared::new();

Loading

*SHARED.borrow(cs).borrow_mut() = Some(Shared { adc, tx });

to

SHARED.put(cs, MyShared { adc, tx });

Referencing

if let Some(ref mut shared) = SHARED.borrow(cs).borrow_mut().deref_mut() {

to

if let Some(mut shared) = SHARED.get_mut(cs) {
    let shared = shared.deref_mut();
    /* interrupt handler */

There are still some deficiencies that I would like to solve at a future date, but I think it is suitable to be put into use as it is. I'd rather that get and get_mut could return Option<&T> and Option<&mut T> values respectively, but I don't think that can be done without replacing the use of RefCell internally. All attempts I've made have run afoul of lifetimes not living long enough to sustain the references passed externally.

@therealprof
Copy link
Contributor

I absolutely love it. Something I put on the wishlist in rust-embedded/wg#256 (comment)

src/lib.rs Outdated Show resolved Hide resolved
@japaric
Copy link
Member

japaric commented Jan 14, 2019

I consider multiple instances of Arc<Mutex<RefCell<T>>> (the std cousin of bare_metal::Mutex<RefCell<T>>) a warning sign that tells you to pause and think twice about how you are structuring your program (*) so I'd rather not add a wrapper that hides this warning sign.

(*) e.g. std::thread + Arc<Mutex<RefCell<T>>> (AKA unbounded parallelism) vs tokio tasks + futures channels

the standard ways of executing this common pattern less painful.

This sentence worries me. The problem of sharing, and moving, data between main and interrupt handlers already has a zero cost solution in the form of controlled scoping (e.g. RTFM). There are other solutions as well like dynamic interrupt handlers (see embedded-rs and this gist), that although not zero cost at least are non-panicking. Yet people seem to be reaching out for the worst possible solution (static + Mutex AKA unrestricted / global scoping). Perhaps we should consider an API like this to cortex-m-rt?

// syntax is a straw man
#[cortex_m_rt::app]
const APP: () = {
    // uninitalized static
    static mut MOVE: SYST = ();
    // NOTE: no RefCell
    static mut SHARED: u64 = 0;

    #[init]
    fn init() { // interrupts disabled
        // ..

        // initialize static at runtime
        MOVE = SYST;
    }

    #[idle(accesses = [SHARED])]
    fn idle() -> ! { // interrupts re-enabled
        // ..

        loop {
            let x = SHARED.lock(|shared: &mut u64| { // same as `interrupt::free`
                *shared -= 1;
                *shared
            });

            // .. do stuff with x ..
        }
    }

    #[exception(accesses = [MOVE, SHARED])]
    fn SysTick() {
        let syst: &mut SYST = MOVE;

        // ..

        // no critical section required
        let shared: &mut u64 = SHARED;
        *shared += 1;
    }
};

It kind of duplicates what RTFM offers (it doesn't handle priorities and it doesn't allow sharing between interrupts), but if it reduces the use of Mutex out there then I think the partial duplication is worthwhile.

I should add that I personally would like to eventually deprecate Mutex. It's Sync implementation is a lie (#12) and it's going to cause (composability) problems down the line with multicore programs.

I'd like to hear other people's opinions on this PR, Mutex's future and / or the suggested cortex-m-rt API. cc @rust-embedded/cortex-m (doesn't seem to work in this repo) cc @adamgreig @korken89 @thejpster @jamesmunns

@therealprof
Copy link
Contributor

Yet people seem to be reaching out for the worst possible solution (static + Mutex AKA unrestricted / global scoping).

Currently it is the only really known and hence standard way of doing this, apart from RTFM. A better standard way would be much appreciated; I don't think a whole lot of people know the required Rust-Fu to make it happen on their own.

#[cortex_m_rt::app]
const APP: () = {
...

The approach is interesting but I have a strong dislike for all those complicated custom notations which desugar into something non-trivial; magic like that breaks far too often in unexpected ways.

@HarkonenBade
Copy link
Author

HarkonenBade commented Jan 14, 2019

I kinda agree with @therealprof I'd prefer an API that used standard rust structure as much as possible and actively avoided macro driven custom syntax. Possibly something that was more focused towards interrupts as closures as given in the example you linked @japaric. As the major issue here, 'how to transfer data from main to an interrupt handler' is currently driven by the fact that they both exist as free functions floating at the top source level. So perhaps an API that enforces defining interrupt handlers within your init code as closures passed to a function would be useful. Also we would still potentially need something Mutex flavoured if you want to have shared mutable access, I recognise the deficiencies with the current implementation of Mutex is there anything better we could do?

@adamgreig
Copy link
Member

I'd love to see a better alternative than the Mutex<RefCell<Option<T>>> dance too, and would also like to see bare_metal::Mutex deprecated or changed.

But, I'm not a huge fan of putting basically RTFM-lite into cortex-m-rt, even as an optional API (how optional is it if you pretty much have to use it to safely share state between the main thread and interrupt handlers?). RTFM already exists to provide that type of solution, but I feel like cortex-m-rt should be more general-purpose. As with @therealprof I'd like to minimise custom notation where possible, too.

I think if we could make some sort of Shared<T> that had the right behaviour to somehow safely share between main and ISRs that could be a nice solution, but it would probably still have to involve static and I don't know exactly what it would look like inside. Perhaps it would be a #[shared] macro instead. Maybe using the separate #[init]/#[main] idea could help, since I think the only real need for the Option is to move the value in during initialisation. By the time I finished writing this paragraph I can't see how you'd string all this together without ending up with exactly what @japaric suggested, though.

@japaric could you elaborate on your objection to the Mutex<RefCell<Option<T>>> dance? Is it mainly around the runtime costs of the CS and option check/panic, or the semantics of shared globally scoped variables, or..?

@korken89
Copy link
Collaborator

I will chime in a bit on this, when it comes to Mutex it should be deprecated or renamed for init only purposes as most likely the implications of using this was not clear at the time of implementation and addition.
Something like this is OK for initialization, but after that it is a no-no.

For example if this is used in exception/interrupt handlers these systems' execution time analysis becomes tainted and systems such as RTFM should be used in its stead as there are no need for global locks when you have done the analysis.
The issue stems from that global locks (critical sections) degrade the global WCET of interrupts/exceptions rather than keeping the issue contained to lower priority (than the resource's priority) [@adamgreig this is the probably what @japaric is referring].

Plus, we can't really do better (from an execution time perspective) than the locks in RTFM so we should rather advocate for the use of that framework.

@therealprof
Copy link
Contributor

therealprof commented Jan 15, 2019

The issue stems from that global locks (critical sections) degrade the global WCET of interrupts/exceptions rather than keeping the issue contained to lower priority (than the resource's priority) [@adamgreig this is the probably what @japaric is referring].

If you're not using different priorities to preempt interrupt handling then the execution time of the handler is deterministic/can be reasoned about, conversely if you use different priorities and allow preemption your timing analysis will be off no matter what.

Plus, we can't really do better (from an execution time perspective) than the locks in RTFM so we should rather advocate for the use of that framework.

True, but then again most use cases don't actually need stiff hard realtime guarantees so why force the implementation complexity and quirks of RTFM on all users?

@HarkonenBade
Copy link
Author

One relevant bit is that in a notable portion of cases this isn't even about 'sharing' data between main and interrupt handlers, it is about being able to transfer ownership of data from main after initialisation into interrupt handler so that it can be used there.

Can we think of a way to achieve this that involves minimal intrusion into the users coding?

Also what are the active downsides to having runtime registration of interrupt handlers as closures? As to me that seems to solve this side of this issue neatly by allowing you to move values into the closure.

@therealprof
Copy link
Contributor

Also what are the active downsides to having runtime registration of interrupt handlers as closures? As to me that seems to solve this side of this issue neatly by allowing you to move values into the closure.

No can do, unless you want do veneers for every interrupt handler. The entry points of the interrupt handlers need to be known at link time since they have to be placed into the interrupt vector table. I think the only two options here would be veneers or some macro magic so extract the interrupt handler from the init/main function and place them outside as they're now.

@perlindgren
Copy link

perlindgren commented Jan 15, 2019

From my point of view, we should have APIs at all levels that are simple, clear, sound, and have predictable (if possible zero-cost) OH. (These requirements are not unique to embedded, but goes for Rust in general.)

RTFM aims at being just that, and with the move to 4.0 we are one step closer to standard Rust.

I would like to see some concrete examples of applications written in some existing or potential bare metal fashion that would better match the stated criteria. If we can find that, then we could put the focus on improving the usability (or other aspects) of RTFM. The goal is that RTFM should NEVER stand in your way, even for implementing the simplest type of application.

Best regards
Per Lindgren

@HarkonenBade
Copy link
Author

From my point of view, we should have APIs at all levels that are simple, clear, sound, and have a predictable (if possible zero-cost) OH. (These requirements are not unique to embedded, but goes for Rust in general.)

RTFM aims at being just that, and with the move to 4.0 we are one step closer to standard Rust.

I would like to see some concrete examples of applications written in some existing or potential bare metal fashion that would better match the stated criteria. If we can find that, then we could put the focus on improving the usability (or other aspects) of RTFM. The goal is that RTFM should NEVER stand in your way, even for implementing the simplest type of application.

Best regards
Per Lindgren

Is the assumption then that we would expect all embedded applications to be built using RTFM?

@perlindgren
Copy link

Well, our aim is to make RTFM the perfect fit for bare metal applications with static resource and task structure (that is we are NOT aiming at systems where you "load" applications or resources on the fly).

In general dynamic systems is out of reach for static analysis (required by RTFM). But we can think about extending RTFM to deal with systems having different operational modes (e.g., a low power mode, running only a subset of tasks), or other modes running another subsets of tasks (e.g., handling the system in a "limp back" state if some fault has been detected).

So in the future I foresee we can handle this limited type of dynamic behavior. RTFM already plays well with heapless (for working with statically allocated dynamic data structures, but that's another side of dynamic behavior).

/Per

@perlindgren
Copy link

With that said, RTFM does not cover/aim at dynamic applications. However, I do believe such an operating system (supporting dynamic loading) may well be built upon RTFM (but is not our primary goal). Many (or perhaps even most) embedded applications fall into the static task/resource category and hence suitable to SRP based scheduling. Given that the RTFM API does not stand in the way of (any) application development, it can serve as a common platform for application development.

Notice, PACs, and HAL-implementations etc. are RTFM agnostic, so there is no loss of generality implied.

So in effect, I believe it better spent effort to improve on RTFM than to come up with yet another framework for managing resources.
/Per

@therealprof
Copy link
Contributor

So in effect, I believe it better spent effort to improve on RTFM than to come up with yet another framework for managing resources.

No one is trying to write a framework for managing resources. This is all about providing safe low-level primitives to allow writing simple but safe applications. Think of the target community being entry to intermediate level programmers coming from (close to) bare-metal C environments or playgrounds like Arduino with little to no Rust experience.

RTFM is (probably) fantastic for mission critical hard real time industrial application but I find the custom DSL boilerplate unidiomatic and not very beginner and/or C-programmer friendly. It's also a huge jump from the content we have in the book to RTFM just for the sake of sharing peripherals with interrupt handlers.

@HarkonenBade
Copy link
Author

HarkonenBade commented Jan 15, 2019

Yeah, like my principle use case for this right now would be for small code examples in the stm32f0xx-hal, like https://github.com/stm32-rs/stm32f0xx-hal/blob/master/examples/flash_systick.rs

@jamesmunns
Copy link
Member

jamesmunns commented Jan 15, 2019

I've also found myself reaching for the unfortunate global pattern for items that are well described as "late resources" by RTFM.

I definitely think that RTFM has solved this problem well - I think the desire is to have a similar functionality to RTFM's resources (statically initialized, as well as early-runtime-initialized), without requiring use of RTFM. In the example above where I would like to share data between a driver instance and an interrupt provided by the driver, it would not make sense to use RTFM at all, as this is a HAL crate, rather than an application.

EDIT NOTE: For background, the example I linked above aims to provide "driver managed" async buffering and sending of data via a UART. To achieve this, the driver "takes over" the UART interrupt to service the DMA transactions chunk-at-a-time. The user/application code never "sees" the interrupt, however the memory space backing the queue is provided by the user, so that they can specify the size of the queue used by the driver. This means that the initialization of the driver must happen at runtime.

I'm aware that making a generic item (such as Shared<T> described above) is likely to be less optimal than RTFM, as it does not have the benefit of whole program analysis that RTFM has (by declaring structure using a DSL), however there is a real need for a "good enough" solution that fits more general use cases than RTFM provides at the moment.

@eddyp
Copy link

eddyp commented Jan 15, 2019

if you use different priorities and allow preemption your timing analysis will be off no matter what.

I am unsure if you meant that in this scenario you can't have RT guarantees, but if you do, I disagree. If you have multiple priorities and preemption you can have a guaranteed WCET since you can compute all the possible latencies/delays visible at your level based on the upper ones.

@korken89
Copy link
Collaborator

No one is trying to write a framework for managing resources. This is all about providing safe low-level primitives to allow writing simple but safe applications.

This is indeed fundamental, however teaching the mutex dance (or simplifying it, making it easier to use) is strengthening an anti-pattern and should be avoided.

RTFM is (probably) fantastic for mission critical hard real time industrial application
I find the custom DSL boilerplate unidiomatic and not very beginner and/or C-programmer friendly

This we can mitigate/solve with documentation and the RTFM book. It's important to see that this is not a trivial problem and that there are frameworks to handle it.
It feels like the resistance to it is rather a documentation issue and about hanging on to, arguably, dangerous patterns.


@jamesmunns While I agree that there is a real need for a "good enough" solutions, this is not it as it helps making an anti-pattern easier to use (and beginners will always continue what they use the first time) as it does not scale by hurting the entire program.

@korken89
Copy link
Collaborator

@eddyp @therealprof

if you use different priorities and allow preemption your timing analysis will be off no matter what.

eddyp is correct, there are no issues here. For example Klee symbolic execution can give the no. of clock cycles which then can be used for the analysis to give strong guarantees.

@therealprof
Copy link
Contributor

I am unsure if you meant that in this scenario you can't have RT guarantees, but if you do, I disagree. If you have multiple priorities and preemption you can have a guaranteed WCET since you can compute all the possible latencies/delays visible at your level based on the upper ones.

In theory. In practice it can be impractical or even impossible (e.g. if you're processing arbitrary lengths inputs or rely on external timing).

Also in many (even hard real-time) applications it is not necessary to guarantee execution WCET across all priority levels but only the highest. Not to mention there're other mechanisms to deal with hard requirements which do not involve the execution units of the MCU/CPU at all.

@eddyp
Copy link

eddyp commented Jan 15, 2019

I think the desire is to have a similar functionality to RTFM's resources (statically initialized, as well as early-runtime-initialized), without requiring use of RTFM

I agree. This is exactly what I have been wanting since I've been looking at rust because I want to write a RTOS that can be a drop in replacement for an older C implementation. Ditto for some drivers because the current C implementation has a standard interface.

@eddyp
Copy link

eddyp commented Jan 15, 2019

In theory. In practice...

I see what you mean, but we're not talking about a bad design which does O(n) processing in the interrupt handler.

Anyway, this is already off topic, so I'll stop here.

@jamesmunns
Copy link
Member

@korken89

While I agree that there is a real need for a "good enough" solutions, this is not it

Fair enough, I can definitely understand this not being the right solution, my comments were more broad about "this is a problem that needs to be solved", rather than stating "this is how we should solve this problem". If there is a better place for me to provide this feedback, I'm happy to move there :)

In general, I would also say many users of Rust at this time are also not considering WCET/RT deadlines at all, as this is generally only required by a select few industries. I would think that many users would prefer something that does work, and is teachable, even if sub-optimal, though I will admit this is just personal opinion.

That being said, deadlock is something that is likely to affect even non-RT users, so solutions we present should make that avoidable if possible - either by use of static analysis, or even just providing failable methods (try_lock() etc.). Though, I don't know what to do if the lock acquire fails in interrupt context, but this has been a problem that has traditionally addressed (or attempted to be addressed) in architecture, for better or worse.

@therealprof
Copy link
Contributor

This is indeed fundamental, however teaching the mutex dance (or simplifying it, making it easier to use) is strengthening an anti-pattern and should be avoided.

I don't see this as an anti-pattern at all. Raw static mut are the anti-pattern, the Mutex dance (NB: I love how people are using this phrase now) are the safest known alternative pattern to the anti-pattern.

It feels like the resistance to it is rather a documentation issue and about hanging on to, arguably, dangerous patterns.

For me it's a usability issue. We want to make it easy for beginners and converts to start coding for embedded systems in Rust and once they've understood the basic concept, we can introduce more advanced ones.

@korken89
Copy link
Collaborator

I think we should have a look at what properties are desired and then look into how to solve them for the simple case - attack this issue a bit more pragmatic.
Else we will just be throwing pro/cons properties of different approaches at each other forever :)

@HarkonenBade
Copy link
Author

HarkonenBade commented Jan 15, 2019

I think we should have a look at what properties are desired and then look into how to solve them for the simple case - attack this issue a bit more pragmatic.
Else we will just be throwing pro/cons properties of different approaches at each other forever :)

The properties required are a simple and minimal way to transfer ownership or share ownership of a fixed set of pieces of data between the main function and one or more interrupt handler functions. Particularly one that would be suitable for use inside a HAL.

As I find the note about encapsulating interrupt handlers within drivers/hal interfaces a notably compelling one. As this is a situation where you cannot take an approach that defines the whole program structure.

@adamgreig
Copy link
Member

The properties required are a simple and minimal way to transfer ownership or share ownership of a fixed set of pieces of data between the main function and one or more interrupt handler functions. Particularly one that would be suitable for use inside a HAL.

I think that's quite a big ask and we could probably get away with a lot less. For instance, many use cases are probably OK with only sharing between the main thread and a single interrupt handler -- and now you know the main thread cannot pre-empt that ISR, and can only be pre-empted by a single ISR, so you could imagine the ISR always getting direct and panic-free access, while the main thread only disables a single ISR to access the shared variable.

Another use is moving something from main to one ISR once ever and not sharing it thereafter, which again might be simpler to implement.

@eddyp
Copy link

eddyp commented Jan 15, 2019

I think we should have a look at what properties are desired and then look into how to solve them for the simple case - attack this issue a bit more pragmatic.

I agree. For instance, I see the current Mutex<> implementation analogous to an Rc<>. What we probably need is some conventions and some implementation based on how "wide" is the synchronization mechanism. For instance single core, cluster level in multi core symmetric systems, all cores in the same big.LITTLE configuration, all cores in the system (for hybrid platforms such as Freescale/NXP VF6xx which has an M4 and an A5 core) and so on.

It might be that focusing on the naming and conventions would allow us to come to the conclusion we need to define some traits and focus on HAL...

@jamesmunns
Copy link
Member

I've opened an issue on the /wg issue tracker, which might be better suited for discussing these design goals, outside of the scope of this particular PR.

@korken89
Copy link
Collaborator

Yeah, like my principle use case for this right now would be for small code examples in the stm32f0xx-hal, like https://github.com/stm32-rs/stm32f0xx-hal/blob/master/examples/flash_systick.rs

@HarkonenBade, just for reference, the RTFM version is like this so you can do side by side comparison:

#![no_main]
#![no_std]

extern crate panic_halt;

use cortex_m::peripheral::syst::SystClkSource;
use stm32f4::stm32f413::GPIOA;

use rtfm::app;

#[app(device = stm32f4::stm32f413)]
const APP: () = {
    // late resorce binding
    static mut GPIOA: GPIOA = ();

    // init runs in an interrupt free section
    #[init]
    fn init() {
        // configures the system timer to trigger a SysTick exception every second
        core.SYST.set_clock_source(SystClkSource::Core);
        core.SYST.set_reload(16_000_000); // period = 1s
        core.SYST.enable_counter();
        core.SYST.enable_interrupt();

        // power on GPIOA, RM0368 6.3.11
        device.RCC.ahb1enr.modify(|_, w| w.gpioaen().set_bit());
        // configure PA5 as output, RM0368 8.4.1
        device.GPIOA.moder.modify(|_, w| w.moder5().bits(1));

        // pass on late resources
        GPIOA = device.GPIOA;
    }

    #[exception (resources = [GPIOA])]
    fn SysTick() {
        static mut TOGGLE: bool = false;

        if *TOGGLE {
            resources.GPIOA.bsrr.write(|w| w.bs5().set_bit());
        } else {
            resources.GPIOA.bsrr.write(|w| w.br5().set_bit());
        }

        *TOGGLE = !*TOGGLE;
    }
};

@perlindgren
Copy link

So looking at the above example, I hope we can waive the conception of RTFM adding complexity, maybe especially for the beginner. No need to think about critical sections, things that may go wrong etc. So in effect it reduces complexity, while giving you all the goodies, deadlock freeness and soundness out the box. And it scales (adding new functionality and requirements, no need to redo the design, just add message passing when you need it...).

So maybe its just a matter of communication, documentation and good idiomatic examples.

I'm currently giving a course (starting next week) where LTU students will design their own ES, choosing components, designing PCBs, and program their applications. These students are expected 0 prior Rust experience, so they are the ultimate beginners.

We ran the course in this format last year (for the first time). All the student groups were able to get mandatory functionality, USART and ITM communication, PWM of some GPIO (e.g. led) , and ADC of some analogue signal, (e.g. battery voltage).

Additionally, each group of 4 students selected an application, we had robotic control of a ball-maze game (with led display), a GBA game slot/card emulator (with SD-Card FAT file system implemented in Rust from scratch by the students), a waker watch (using RTC as a programmable wakeup source), a moonshine heater (with LOTS of power electronics), CAN bus based zip game, etc. 7 groups in total, with a lot of diversity.

And guess what, students did this in 10 weeks half time studies:

  • schematics,
  • pcb design, yes they were sent off to an industrial grade prototype fab (Cogra),
  • soldered all components (mostly by hand, some in oven)
  • developing firmware in RTFM Rust
  • integration testing
  • demo

The teaching method was GitGuD, i.e., the students were working hard throughout the whole course challenging their potential. That works since its fun!!! Selecting a project of their own is highly motivating. Doing the impossible even more so ;) And they succeeded.

As teachers we were (almost always) available on telegram, answering all questions promptly, students should not get stuck for too long, but the answers were more of hints not solutions.

Regarding Rust, only a few lectures actually. But loads of assignments (10) taking them from the simplest bare metal "hello world" to a fully fledged serial terminal (with message parsing etc.). Yes, the assignments cover cortex-m, cortex-m-rt, embedded-hal, and cortex-m-rtfm.

I'm now in the process of updating the examples and assignments to RTFM 4. (Message passing will come handy:)

ST is sponsoring the course with Nucleo F401re/F411re devkits (each student get one). We also use the stm32cubemx tool to show the clock routing, for doing the PIN/IO assignments, etc. No automatic codegen as of now (but maybe someone out there want to give it a shot, configs are likely parseable xml so ...).

So in parallel with waiting for the PCBs to arrive back from fab, we took on Rust using the Nucleos, building experience and a code base that they could re-use in their own firmware.

In conclusion, neither Rust not RTFM was a show stopper to success (problems were more related inexperience of PCB design and datasheet reading/understanding). Comments regarding software development was mostly related to the lack of documentation (at that point).

We are looking forward to the upcoming installment, now with a more user friendly and capable RTFM4, better docs (the books) and all the great work you all put into making the embedded Rust such a pleasure to work with.
/Per

@HarkonenBade
Copy link
Author

@perlindgren How would you approach the use of contained interrupt handlers within driver/hal implementations as proposed above?

@jamesmunns
Copy link
Member

@perlindgren at least personally, I am a fan of RTFM, for the reasons you listed. I have used it in the past, and plan to continue recommending it to developers as well as customers of mine. In general, I'd like to make use of RTFM my default for all new embedded rust projects.

However the WG in general is supporting all developers of embedded rust, not just users of RTFM. At the moment, we have not made the use of RTFM a core/required part of the ecosystem, at least not as heavily as things such as svd2rust, the cortex-m crate, or cortex-m-rt are. This could change in the future, however I think it is likely that there will be developers (who think/feel/are justified in deciding) that RTFM may not be a good match, or are just not interested. I don't see this as a slight to RTFM.

I would still be interested in hearing your input regarding resource sharing within a driver that I outlined above.

@perlindgren
Copy link

@jamesmunns ,perhaps the driver impl should export a function (instead of specifying a handler) that you just call from your interrupt handler (defined in #[app]). If your driver should have a task local store, just declare in the interrupt handler/task, and pass it to the driver along with other resources the driver needs. In this way the resource sharing will be sound (no unsafe required). The only difference is that binding the driver to the interrupt vector is done in #[app], not hidden in the driver itself.

See the #[app] as being the switchboard (or interaction view) of your system. Functionality can be given in place or factored out. RTFM3, had the notion of paths to the actual implementation, you can still achieve the same thing with a trampoline, and for small enough examples you can write all your code in-place.

The main reason for the current choice in RTFM is convenience, we don't need to care about the concrete types of resources. Re-factoring in general is somewhat a bummer in Rust due to the choice of non inference in function signatures.

In the future, it is projected that proc macros (like #[app]) can pick up local (or even crate wide) attributes. That would allow drivers to bind resources, interrupts etc. If that is a good thing... well maybe it improves on convenience. However we loose the switchboard view. The main inconvenience is due to the lack of type inference, maybe that restriction can be lifted (which may be a good thing for code re-use in any case, but I don't know what kind of worm-hole that would open up... better keep tho lid on perhaps...)

Did that answer your question, or did I completely miss the point?
/Per

@jamesmunns
Copy link
Member

@perlindgren, I think that is an acceptable answer, though it makes the trade off of being less convenient in the non-RTFM case, to support the RTFM use case, and may cause unsound behavior if the inner interrupt function should only ever be called in a single interrupt context (not any valid pub context). We would have to make the method public to allow users to call it from RTFM context.

@perlindgren
Copy link

@jamesmunns , you mean unsound if you call into the driver code from different call sites (not just the intended interrupt handler)? Well, if the driver can be written in safe code (regarding shared resources), it is not unsound to call it from different call sites. It is the different call sites that are unsound (by grabbing the resources in an unsound manner), it is the price you pay by opting out a safety in the first place. At least it makes it clear that the call site has to take responsibility for the safety.

Rust memory model is about memory safety, if code bugs out due to some other reason (like unintended usage), its by definition not a soundness violation. We are intentionally using unsafe in a bit broader than just memory safety. E.g. its unsafe to write a register lacking an enum, just to indicate that the programmer has to take responsibility. With that said, I think +/- all HW access could be regarded unsafe in that sense.

Think of it, even in RTFM you can do stupid things in safe code, like changing the clock source to a non existing external xtal, .... bang..... at best you get a hard fault... In RTFM we can do only so much, - we offer soundness to the Rust memory model for concurrent resources. In a hal impl (that takes on setting up clocks and so forth) we can offer a bit more robustness to misuse.

Side note:
Extending the type system of Rust with effects might offer some means towards further robustness. There is some built in notions, like the ! (never type), with the implication that the function is ensured not to return. One could think of effects such pureness (side effect free), - we have that to some extent, but not entirely, since we can have interior mutability. For now we have to settle for the singleton approach, and consume (move/freeze) to restrict side effects. (See e.g., how the RCC freeze require FLASH as a parameter since we are affecting the FLASH on setting/freezing the clock.) Effect types could potentially capture such side effects in a more convenient matter... (I'm not a type systems expert though...)

/ Per

@perlindgren
Copy link

@jamesmunns , thinking of it. You may have a cfg feature for opting in the interrupt handler, that does the dirty work, calling into the device driver... Then it would work with or without RTFM.

@ALL We are eager to see other examples, where You feel that RTFM falls short, or stand in Your way implementing things.

/Per

1 similar comment
@perlindgren
Copy link

@jamesmunns , thinking of it. You may have a cfg feature for opting in the interrupt handler, that does the dirty work, calling into the device driver... Then it would work with or without RTFM.

@ALL We are eager to see other examples, where You feel that RTFM falls short, or stand in Your way implementing things.

/Per

@japaric
Copy link
Member

japaric commented Jan 22, 2019

@adamgreig

could you elaborate on your objection to the Mutex<RefCell<Option>> dance?

It's not zero cost: it has memory overhead, it can fail at runtime and always
prevents interrupts from starting.

In embedded, reliability and resource (e.g. RAM) usage are important metrics,
plus debugging is hard / time-costly so the less panics you can run into the
better. Mutex does bad on all those accounts. Hiding those flaws with newtype
makeup will only hurt Rust adoption, IMO -- it's not like people won't notice
them in the long run.

Also, one or two Mutex-es may cut it for a small / hobby project but Mutex is
not a foundation large / production applications can build upon -- we should not
encourage its use in applications of any size (people will try to carry over the
abstraction from their small tests to real applications).

the semantics of shared globally scoped variables

This is the root issue. RTFM v1 / v2 explored this space and the conclusion was
that there are no zero cost solutions if you want to use global static
variables; IOW, access control is required for zero cost moving and sharing
between different execution contexts. Plus, it's just plain hard to reason about
your code if anyone can modify the state of your interrupt handler, even if that
action is memory safe (i.e. accepted by the compiler).


@therealprof

I get that familiarity is important but Rust didn't set up to be just "C with
a package manager"; Rust is here to let you build safe, correct and fast
software. How does it do that? By rejecting bad patterns: pointer invalidation?
borrow checker error; out of bounds access? panics; unsynchronized access to
static mut? unsafe blocks.

I see global static variables as an anti-pattern in embedded applications. If
we can't reject this pattern at the language / compiler level then let's address
the issue in the core libraries of the ecosystem. We have better alternatives
today: procedural macros and dynamic handlers; let's encourage their use.

It's also a huge jump from the content we have in the book to RTFM

Then let's cover dynamic handlers first and then procedural macros. There are
ways to manage the amount of complexity we expose to readers.


@jamesmunns

I've also found myself reaching for the unfortunate global pattern for items
that are well described as "late resources" by RTFM.

Using unsafe static mut is playing with fire; it can easily lead to UB. From
a quick glance here's how to trigger UB with your API:

  • Enable UARTE0 interrupt
  • Pend UARTE0 interrupt
  • Have a higher priority interrupt preempt UARTE
  • Have the higher priority interrupt create UarteAsync and return to the
    UARTE0 handler
  • MAYBE_CONSUMER has changed out of blue from the point of view of UARTE0 thus
    UB.

Is this likely to occur in practice? Maybe not, but there may be more elaborated
ways to run into UB that could occur in practice -- in particular, creating
UarteAsync starting a write, freeing UarteAsync and then creating a new
UarteAsync with a different BBQueue seems like it could be problematic.

I would suggest moving DMA_STATUS into UARTE0_UART0 to make it safe and then
replacing static mut MAYBE_CONSUMER with something like spin::Once (yes, it
can deadlock but deadlocks are much easier to debug than UB).

If you want something zero cost then you need a "before, after" constraint, like
the #[init], #[idle] split in RTFM. If you have written this driver with a
"before, after" constraint in mind then the API would have been less ergonomic
but neither the library author or the application author would have need to use
unsafe static mut and it would have been easier to reason about the safety
of the API.


@HarkonenBade

Thanks for the PR, but I'm not going to accept it. Let's discuss better alternatives
in rust-embedded/wg#294

@japaric japaric closed this Jan 22, 2019
@therealprof
Copy link
Contributor

@japaric

It's not zero cost: it has memory overhead, it can fail at runtime and always
prevents interrupts from starting.

Huh? Nobody says it has to be zero cost. In fact it is lower cost than what other languages do which is a good thing. Zero cost would be nice but is not a must have if the vast majority of cases. Also if it fails it will fail deterministically which is a good thing!

I see global static variables as an anti-pattern in embedded applications.

That is something I agree with. The rest not so much.

If we can't reject this pattern at the language / compiler level then let's address
the issue in the core libraries of the ecosystem. We have better alternatives
today: procedural macros and dynamic handlers; let's encourage their use.

This is something I don't think I will ever be able to get behind. The use of macros in Rust is slowly turning into the exact same mess which they are in C/C++: Lots of opaque magic happening behind the facade trying to work around limitations/flaws of the programming language itself. Macros make it hard to reason about the implementation and cause random errors to appear out of nowhere, especially when trying to extend them.

Let's do less of those please and make sure that the language itself allows us do stuff.

(yes, it can deadlock but deadlocks are much easier to debug than UB).

But they're a lot harder to debug than a deterministic panic which is why I definitely prefer a panic over a deadlock any time of day.

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.

8 participants