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

Refactor the Task struct to reduce lock contention #433

Merged
merged 12 commits into from
Aug 18, 2021

Conversation

kevinaboos
Copy link
Member

@kevinaboos kevinaboos commented Aug 18, 2021

  • Move immutable states into Task and mutable states into TaskInner
  • Use properly-aligned and -sized atomic types such that native atomic instructions are actually used. For this, we switch from atomic::Atomic to crossbeam_utils::atomic::AtomicCell since it actually works. For a custom-target build like Theseus, the atomic crate does not get configured properly (due to autocfg does not work in the presence of -Z build-std cuviper/autocfg#34) and thus doesn't actually use native atomic instructions!
    • Use const_assert!() to ensure that all types wrapped in AtomicCell actually use native atomic instructions.
  • The runstate and running_on_cpu fields of Task are wrapped in AtomicCell, enabling them to be accessed in a lock-free manner.
    • This is beneficial for blocking/unblocking tasks in a lock-free context, e.g., within an interrupt handler.
  • Removes now-unnecessary unsafe statements from schedule()
  • Move ExitValue out of the RunState enum such that RunState can be placed within an atomic type.

as it's not actually atomic.
This is due to a build script configuration failure in `autocfg`, which
`atomic` relies on to determine whether a given type is actually atomic
on a given architecture at build time.

Thus, we need to use something else, perhaps crossbeam's AtomicCell?
…micCell` because it actually compiles down the native atomic instructions!

Still a WIP, things don't run properly.
@kevinaboos kevinaboos merged commit df721e3 into theseus-os:theseus_main Aug 18, 2021
@kevinaboos kevinaboos deleted the refactor_task branch August 18, 2021 20:54
github-actions bot pushed a commit that referenced this pull request Aug 18, 2021
* Move a Task's `ExitValue` out of its `RunState` so that `RunState` can be separately accessed atomically.
* Move immutable fields into `Task` and mutable fields into `TaskInner` (excluding atomic fields), which reduces lock contention. Most fields can now be accessed without acquiring locks.
* Use properly-aligned and -sized atomic types such that native atomic instructions are actually used. For this, we switch from `atomic::Atomic` to `crossbeam_utils::atomic::AtomicCell` since it actually compiles down to native atomics.
* Statically assert that all types wrapped in `AtomicCell` actually use native atomic instructions.
* The `runstate` and `running_on_cpu` fields of `Task` are now atomically accessible in a lock-free manner. This is beneficial for blocking/unblocking tasks in a lock-free context, e.g., within an interrupt handler.
* Remove now-unnecessary unsafe statements from `schedule()` df721e3
github-actions bot pushed a commit to jacob-earle/Theseus that referenced this pull request Aug 19, 2021
* Move a Task's `ExitValue` out of its `RunState` so that `RunState` can be separately accessed atomically.
* Move immutable fields into `Task` and mutable fields into `TaskInner` (excluding atomic fields), which reduces lock contention. Most fields can now be accessed without acquiring locks.
* Use properly-aligned and -sized atomic types such that native atomic instructions are actually used. For this, we switch from `atomic::Atomic` to `crossbeam_utils::atomic::AtomicCell` since it actually compiles down to native atomics.
* Statically assert that all types wrapped in `AtomicCell` actually use native atomic instructions.
* The `runstate` and `running_on_cpu` fields of `Task` are now atomically accessible in a lock-free manner. This is beneficial for blocking/unblocking tasks in a lock-free context, e.g., within an interrupt handler.
* Remove now-unnecessary unsafe statements from `schedule()` df721e3
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.

1 participant