Skip to content

Conversation

DanutAldea
Copy link
Contributor

This pull request adds the initial design idea for an async-compatible allow mechanism. This includes:

  • AllowRwBuffer structure to be used in asynchronous function
  • extension of the Syscalls trait to use the new allow interface
  • minimal unittesting to ensure correctness

@jrvanwhy
Copy link
Collaborator

In the Allow API, there are three things that can be tracked either at compile time (via const, generics, or separate types) or at runtime:

  1. The ID (driver number and buffer number)
  2. Whether this is a read-only allow or a read-write allow
  3. Whether the buffer is currently shared with the kernel

This PR's design makes the following choices for those options:

  1. The ID is const
  2. The allow type is tracked at compile time (because RW allow and RO allow are different structs)
  3. Whether the buffer is shared is tracked at runtime (the allowed field)

I was curious whether a different set of choices would be better, so I put together some benchmarks in the design-explorations repository: tock/design-explorations#5

Unfortunately, the results currently are giving us a painful tradeoff: the API is way better if we track whether the buffer is shared at runtime, but that generates 20%-80% more code for the allow calls.

There might be a way to produce a nice API where the mechanism that tracks whether the buffer is allowed (+ the allow type) is configurable by the user, but I might be out of time for this particular benchmarking/design project.

@jrvanwhy
Copy link
Collaborator

I had time to do some more benchmarking today, and got more promising results. I created a MUCH larger benchmark example (one that performs 100 read-only allow calls and 100 read-write allow calls with different IDs and lengths), as well as a "fully dynamic" API, which makes the following choices:

  1. The ID is fully dynamic (no const)
  2. The allow type is tracked at runtime
  3. Whether the buffer is currently shared is tracked at runtime.

and this API was competitive with the "don't track whether the buffer is shared" API. It's slightly more efficient on RISC-V, and somewhat less efficient or ARM, but it the differences are within the range of noise in my benchmarks. As a result, I think we can select based on the API we want.

I think the API we want looks like the API in allow_pin/src/full_dynamic.rs in tock/design-explorations#5. However, there is one area of expansion I need to consider: how do you perform Read-Only Allow on a large static buffer? One that is in .rodata that you don't want to copy into RAM. We probably need a handle type that can be created from either a &'static [u8] or from a Pin<&mut Buffer<...>> that APIs can use (and maybe this handle can be created from a share scope so the closure-based API is usable as well?).

@DanutAldea
Copy link
Contributor Author

Hey, this is a lot of good work. I tried to help by adding an initial design for such a handler in the full_dynamic module. It makes the same design choices:

  • fully dynamic IDs
  • runtime tracking of allow state

and tries to reuse some of the existing "fully dynamic" code to save some code size. The draft can be found here jrvanwhy/design-explorations#1.

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.

2 participants