Skip to content

Add SelfContainedChannel#2757

Merged
sporksmith merged 8 commits intoshadow:mainfrom
sporksmith:vasi-sync
Feb 28, 2023
Merged

Add SelfContainedChannel#2757
sporksmith merged 8 commits intoshadow:mainfrom
sporksmith:vasi-sync

Conversation

@sporksmith
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith commented Feb 21, 2023

We need a vasi (VirtualAddressSpaceIndependent) wrapper or reimplementation of IPCData (essentially a pair of fixed-size channels) for communication between shadow and the shim before migrating ManagedThread to Rust.

This PR:

  • moves SelfContainedMutex into a new crate, where we can put IPCData and any other vasi sync primitives we end up needing to implement.
  • Extracts some reusable loom abstractions from SelfContainedMutex
  • Adds SelfContainedChannel and corresponding loom-compatible tests. It should be simple to replace IPCData with a pair of these.

@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable labels Feb 21, 2023
@sporksmith sporksmith force-pushed the vasi-sync branch 6 times, most recently from 7b8ff79 to a08e75b Compare February 22, 2023 21:41
@sporksmith sporksmith changed the title Move SelfContainedMutex into a new crate vasi-sync Add SelfContainedChannel Feb 22, 2023
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still want to look at this a little more, but figured I might as well leave the comments I have so far.

@stevenengler stevenengler self-requested a review February 24, 2023 22:16
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@sporksmith
Copy link
Copy Markdown
Contributor Author

Thanks for the careful review!

@sporksmith sporksmith enabled auto-merge February 28, 2023 17:55
We're going to need to port additional synchronization primitives
suitable for use in shared memory to Rust. I think conceptually it makes
sense to put these in a crate together separate from the shared memory
allocator, which isn't as closely related.
We'll want to reuse these for additional modules in this crate.
This is a VirtualAddressSpaceIndependent implementation of a channel.
The intent is to use this to replace `BinarySpinningSem` and `IPCData`
to communicate with managed threads from ManagedThread in Shadow.
The previous implementation had potential undefined behavior due to
optimistically updating state without checking the previous state.

We do away with the `fetch_add` and `fetch_sub` tricks here (with sanity
checks after the fact), and instead use e.g. `compare_exchange`.
This results in additional atomic operations in the "happy" path, but is
easier to understand, especially in failure cases.
This is more general and more closely aligned with `std::mpsc::channel`
terminology.
@sporksmith sporksmith merged commit ab6f727 into shadow:main Feb 28, 2023
@sporksmith sporksmith deleted the vasi-sync branch February 28, 2023 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Build Build/install tools and dependencies Component: Libraries Support functions like LD_PRELOAD and logging Component: Main Composing the core Shadow executable

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants