Introduce Mutex<T: DeviceIoMut> adapter for DeviceIo #20
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jiangliu
requested review from
alexandruag,
andreeaflorescu,
liujing2 and
sameo
jiangliu
force-pushed
the
device_io_mut
branch
2 times, most recently
from
60d5697 to
31eba84
Compare
Many device backend drivers will mutate itself when handling IO requests. The DeviceIo trait assumes interior mutability, but it's a little complex to support interior mutability. So introduce the Mutex<T: DeviceIoMut> adapter to ease device backend driver implementations. And the Mutex<T: DeviceIoMut> adapter is an zero overhead abstraction without performance penalty. Liu Jiang <[email protected]>
Previously DeviceIo depends on Send, but doesn't depend on Sync, which fails to share Arc<IoManager> among vCPU threads. So make DeviceIo depend on Sync too. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
force-pushed
the
device_io_mut
branch
6 times, most recently
from
606840f to
794c377
Compare
alexandruag
reviewed
Feb 7, 2020
| /// | ||
| /// The Mutex<T: DeviceIoMut> adapter is an zero overhead abstraction without | ||
| /// performance penalty. | ||
| pub trait DeviceIoMut: Send { |
There was a problem hiding this comment.
I wonder whether adding the Send bound here is really necessary. What happens if we remove it?
There was a problem hiding this comment.
The propagation chain for Send/Sync is something like:
EpollHandler -> Vmm -> Device Manager -> IoManager -> DeviceIo
| /// focusing on high performance, they may use the Mutex<T: DeviceIoMut> | ||
| /// adapter to simplify implementation. | ||
| pub trait DeviceIo: Send { | ||
| pub trait DeviceIo: Send + Sync { |
There was a problem hiding this comment.
It makes sense for this to be Sync, but (just wondering, like the previous question) does anything depend on it being Send as well?
There was a problem hiding this comment.
The the DeviceIo needs to be Send + Sync for Arc to be Sync:)
src/resources.rs
Outdated
| } | ||
|
|
||
| impl Deref for DeviceResources { | ||
| type Target = Vec<Resource>; |
There was a problem hiding this comment.
Hmm, any reason to deref into a &Vec<Resource> and not a slice (&[Resource])?
There was a problem hiding this comment.
good suggestion, will change to return a slice.
Implement Clone for IoManager, which will be needed for RCU-style device hotplug. Liu Jiang <[email protected]>
Export IoSize and IoRange as pub, which may be reused. Liu Jiang <[email protected]>
Implement Deref for DeviceResources, so we could walk all resource entries in an Resources object. Signed-off-by: Liu Jiang <[email protected]>
Add #[derive(Debug)] for resource related data structs, so we could use assert!() and assert_eq!() etc in unit test cases. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
force-pushed
the
device_io_mut
branch
from
794c377 to
11f77fb
Compare
The design to multiplex IoAddress/IoSize for MMIO and PIO makes the device driver implmentation a little complex, so build dedicated data structs and interfaces to handle PIO requests. Also make PIO related code x86 specific. Signed-off-by: Liu Jiang <[email protected]>
jiangliu
force-pushed
the
device_io_mut
branch
8 times, most recently
from
5f0c72d to
2d9c7e6
Compare
This was referenced Jun 8, 2020
|
Can we close this one? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an following-up PR for #12 .
Many device backend drivers will mutate itself when handling IO
requests. The DeviceIo trait assumes interior mutability, but it's
a little complex to support interior mutability. So introduce the
Mutex<T: DeviceIoMut> adapter to ease device backend driver
implementations. And the Mutex<T: DeviceIoMut> adapter is an zero
overhead abstraction without performance penalty.
Liu Jiang [email protected]