Skip to content

Commit

Permalink
Merge pull request #983 from cagatay-y/pinned-removal
Browse files Browse the repository at this point in the history 
Remove Pinned
  • Loading branch information
@mkroening
mkroening authored Feb 5, 2024
2 parents 5f94a89 + b3d0c1c commit 774c757
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 422 deletions.
1 change: 0 additions & 1 deletion src/drivers/net/virtio_net.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ impl RxQueues {
Ok(transfer)
} else {
warn!("Unfinished transfer in post processing. Returning buffer to queue. This will need explicit cleanup.");
transfer.close();
Err(VirtioNetError::ProcessOngoing)
}
}
Expand Down
237 changes: 22 additions & 215 deletions src/drivers/virtio/virtqueue/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ impl Virtq {
/// The `notif` parameter indicates if the driver wants to have a notification for this specific
/// transfer. This is only for performance optimization. As it is NOT ensured, that the device sees the
/// updated notification flags before finishing transfers!
fn dispatch(&self, tkn: TransferToken, notif: bool) -> Transfer {
fn dispatch(&self, tkn: TransferToken, notif: bool) {
match self {
Virtq::Packed(vq) => vq.dispatch(tkn, notif),
Virtq::Split(vq) => vq.dispatch(tkn, notif),
}
};
}
}

Expand Down Expand Up @@ -211,18 +211,6 @@ impl Virtq {
}
}

/// Does maintenance of the queue. This involces currently only, checking if early dropped transfers
/// have been finished and removes them and frees their ID's and memory areas.
///
/// This function is especially useful if ones memory pool is empty and one uses early drop of transfers
/// in order to fire-and-forget.
pub fn clean_up(&self) {
match self {
Virtq::Packed(vq) => vq.clean_up(),
Virtq::Split(vq) => vq.clean_up(),
}
}

/// Dispatches a batch of TransferTokens. The actual behaviour depends on the respective
/// virtqueue implementation. Please see the respective docs for details.
///
Expand All @@ -237,7 +225,7 @@ impl Virtq {
/// The `notif` parameter indicates if the driver wants to have a notification for this specific
/// transfer. This is only for performance optimization. As it is NOT ensured, that the device sees the
/// updated notification flags before finishing transfers!
pub fn dispatch_batch(tkns: Vec<TransferToken>, notif: bool) -> Vec<Transfer> {
pub fn dispatch_batch(tkns: Vec<TransferToken>, notif: bool) {
let mut used_vqs: Vec<(Rc<Virtq>, Vec<TransferToken>)> = Vec::new();

// Sort the TransferTokens depending in the queue their coming from.
Expand Down Expand Up @@ -267,17 +255,12 @@ impl Virtq {
}
}

let mut transfer_lst = Vec::new();
for (vq_ref, tkn_lst) in used_vqs {
match vq_ref.as_ref() {
Virtq::Packed(vq) => {
transfer_lst.append(vq.dispatch_batch(tkn_lst, notif).as_mut())
}
Virtq::Split(vq) => transfer_lst.append(vq.dispatch_batch(tkn_lst, notif).as_mut()),
Virtq::Packed(vq) => vq.dispatch_batch(tkn_lst, notif),
Virtq::Split(vq) => vq.dispatch_batch(tkn_lst, notif),
}
}

transfer_lst
}

/// Dispatches a batch of TransferTokens. The Transfers will be placed in to the `await_queue`
Expand Down Expand Up @@ -508,16 +491,6 @@ impl Virtq {
Virtq::Split(vq) => vq.prep_buffer(rc_self, send, recv),
}
}

/// Early drop provides a mechanism for the queue to detect, if an ongoing transfer or a transfer not yet polled by the driver
/// has been dropped. The queue implementation is responsible for taking care what should happen to the respective TransferToken
/// and BufferToken.
fn early_drop(&self, transfer_tk: Pinned<TransferToken>) {
match self {
Virtq::Packed(vq) => vq.early_drop(transfer_tk),
Virtq::Split(vq) => vq.early_drop(transfer_tk),
}
}
}

/// The trait needs to be implemented on structures which are to be used via the `prep_transfer()` function of virtqueues and for
Expand Down Expand Up @@ -562,45 +535,19 @@ pub trait AsSliceU8 {
}
}

/// The [Transfer] will be received when a [TransferToken] is dispatched via `TransferToken.dispatch()` or
/// via `TransferToken.dispatch_blocking()`.
/// The [Transfer] will be received when a [TransferToken] is dispatched via [TransferToken::dispatch_blocking] or through the await
/// queue given to [TransferToken::dispatch_await].
///
/// The struct represents an ongoing transfer or an active transfer. While this does NOT mean, that the transfer is at all times inside
/// actual virtqueue. The Transfers state can be polled via `Transfer.poll()`, which returns a bool if the transfer is finished.
///
/// **Finished Transfers:**
/// * Finished transfers are able to return their send and receive buffers. Either as a copy via `Transfer.ret_cpy()` or as the actual
/// buffers via `Transfer.ret()`.
/// * Finished transfers should be closed via `Transfer.close()` or via `Transfer.ret()`.
/// * Finished transfers can be reused via `Transfer.reuse()`.
/// * This closes the transfer
/// * And returns an normal BufferToken (One should be cautious with reusing transfers where buffers were created from raw pointers)
///
/// **Early dropped Transfers:**
///
/// If a transfer is dropped without being closed (independent of being finished or ongoing), the transfer will return the respective
/// `Pinned<TransferToken>` to the handling virtqueue, which will take of handling graceful shutdown. Which generally should take
/// care of waiting till the device handled the respective transfer and free the memory afterwards.
///
/// One could "abuse" this procedure in order to realize a "fire-and-forget" transfer.
/// A warning here: The respective queue implementation is taking care of handling this case and there are no guarantees that the queue
/// won't be unusable afterwards.
/// * This returns an normal BufferToken (One should be cautious with reusing transfers where buffers were created from raw pointers)
pub struct Transfer {
/// Needs to be `Option<Pinned<TransferToken>>` in order to prevent deallocation via None
// See custom drop function for clarity
transfer_tkn: Option<Pinned<TransferToken>>,
}

impl Drop for Transfer {
/// When an unclosed transfer is dropped. The [Pinned]<[TransferToken]> is returned to the respective
/// virtqueue, who is responsible of handling these situations.
fn drop(&mut self) {
if let Some(tkn) = self.transfer_tkn.take() {
// Unwrapping is okay here, as TransferToken MUST hold a BufferToken
let vq_ref = Rc::clone(&tkn.buff_tkn.as_ref().unwrap().vq);
vq_ref.early_drop(tkn)
}
}
transfer_tkn: Option<Box<TransferToken>>,
}

// Public Interface of Transfer
Expand Down Expand Up @@ -870,7 +817,7 @@ impl Transfer {
match state {
TransferState::Finished => {
// Desctructure Token
let mut transfer_tkn = self.transfer_tkn.take().unwrap().unpin();
let mut transfer_tkn = self.transfer_tkn.take().unwrap();

let mut buffer_tkn = transfer_tkn.buff_tkn.take().unwrap();

Expand Down Expand Up @@ -919,23 +866,6 @@ impl Transfer {
}
}

/// Closes an transfer. If the transfer was ongoing the respective transfer token will be returned to the virtqueue.
/// If it was finished the resources will be cleaned up.
pub fn close(mut self) {
match self.transfer_tkn.as_ref().unwrap().state {
TransferState::Processing => {
// Unwrapping is okay here, as TransferToken must hold a BufferToken
let vq = Rc::clone(&self.transfer_tkn.as_ref().unwrap().get_vq());
let transfer_tkn = self.transfer_tkn.take().unwrap();
vq.early_drop(transfer_tkn);
}
TransferState::Ready => {
unreachable!("Transfers MUST have tokens of states Processing or Finished.")
}
TransferState::Finished => (), // Do nothing and free everything.
}
}

/// If the transfer was finished returns the BufferToken inside the transfer else returns an error.
///
/// **WARN:**
Expand All @@ -955,14 +885,7 @@ impl Transfer {
.unwrap()
.reusable
{
let tkn = self
.transfer_tkn
.take()
.unwrap()
.unpin()
.buff_tkn
.take()
.unwrap();
let tkn = self.transfer_tkn.take().unwrap().buff_tkn.take().unwrap();

Ok(tkn.reset())
} else {
Expand Down Expand Up @@ -995,14 +918,7 @@ impl Transfer {
.unwrap()
.reusable
{
let tkn = self
.transfer_tkn
.take()
.unwrap()
.unpin()
.buff_tkn
.take()
.unwrap();
let tkn = self.transfer_tkn.take().unwrap().buff_tkn.take().unwrap();

Ok(tkn.reset_purge())
} else {
Expand Down Expand Up @@ -1058,24 +974,16 @@ impl TransferToken {
pub fn dispatch_await(mut self, await_queue: Rc<RefCell<VecDeque<Transfer>>>, notif: bool) {
self.await_queue = Some(Rc::clone(&await_queue));

// Prevent TransferToken from being dropped
// I.e. do NOT run the custom constructor which will
// deallocate memory.
self.get_vq()
.dispatch(self, notif)
.transfer_tkn
.take()
.unwrap()
.into_raw();
self.get_vq().dispatch(self, notif);
}

/// Dispatches the provided TransferToken to the respective queue and returns a transfer.
/// Dispatches the provided TransferToken to the respective queue.
///
/// The `notif` parameter indicates if the driver wants to have a notification for this specific
/// transfer. This is only for performance optimization. As it is NOT ensured, that the device sees the
/// updated notification flags before finishing transfers!
pub fn dispatch(self, notif: bool) -> Transfer {
self.get_vq().dispatch(self, notif)
pub fn dispatch(self, notif: bool) {
self.get_vq().dispatch(self, notif);
}

/// Dispatches the provided TransferToken to the respectuve queue and does
Expand All @@ -1090,18 +998,20 @@ impl TransferToken {
/// Upon finish notifications are enabled again.
pub fn dispatch_blocking(self) -> Result<Transfer, VirtqError> {
let vq = self.get_vq();
let transfer = self.get_vq().dispatch(self, false);
let rcv_queue = Rc::new(RefCell::new(VecDeque::with_capacity(1)));
self.dispatch_await(rcv_queue.clone(), false);

vq.disable_notifs();

while transfer.transfer_tkn.as_ref().unwrap().state != TransferState::Finished {
// Keep Spinning until the state changes to Finished
while rcv_queue.borrow().is_empty() {
// Keep Spinning until the receive queue is filled
vq.poll()
}

vq.enable_notifs();

Ok(transfer)
let result = Ok(rcv_queue.borrow_mut().pop_front().unwrap());
result
}
}

Expand Down Expand Up @@ -2373,109 +2283,6 @@ pub enum BuffSpec<'a> {
Indirect(&'a [Bytes]),
}

/// Ensures `T` is pinned at the same memory location.
/// This allows to refer to structures via raw pointers.
///
/// **WARN:**
///
/// Assuming a raw pointer `*mut T / *const T` is valid, is only safe as long as
/// the `Pinned<T>` does life!
///
/// **Properties:**
///
/// * `Pinned<T>` behaves like T and implements `Deref`.
/// * Drops `T` upon drop.
pub struct Pinned<T> {
raw_ptr: *mut T,
_drop_inner: bool,
}

impl<T: Sized> Pinned<T> {
/// Turns a `Pinned<T>` into a *mut T. Memory will remain valid.
fn into_raw(mut self) -> *mut T {
self._drop_inner = false;
self.raw_ptr
}

/// Creates a new pinned `T` by boxing and leaking it.
/// Be aware that this will result in a new heap allocation
/// for `T` to be boxed.
fn pin(val: T) -> Pinned<T> {
let boxed = Box::new(val);
Pinned {
raw_ptr: Box::into_raw(boxed),
_drop_inner: true,
}
}

/// Creates a new pinned `T` from a boxed `T`.
fn from_boxed(boxed: Box<T>) -> Pinned<T> {
Pinned {
raw_ptr: Box::into_raw(boxed),
_drop_inner: true,
}
}

/// Create a new pinned `T` from a `*mut T`
fn from_raw(raw_ptr: *mut T) -> Pinned<T> {
Pinned {
raw_ptr,
_drop_inner: true,
}
}

/// Unpins the pinned value and returns it. This is only
/// save as long as no one relies on the
/// memory location of `T`, as this location
/// will no longer be constant.
fn unpin(mut self) -> T {
self._drop_inner = false;

unsafe { *Box::from_raw(self.raw_ptr) }
}

/// Returns a pointer to `T`. The pointer
/// can be assumed to be constant over the lifetime of
/// `Pinned<T>`.
fn raw_addr(&self) -> *mut T {
self.raw_ptr
}
}

impl<T> Deref for Pinned<T> {
type Target = T;
fn deref(&self) -> &Self::Target {
unsafe { &*(self.raw_ptr) }
}
}

impl<T> DerefMut for Pinned<T> {
fn deref_mut(&mut self) -> &mut Self::Target {
unsafe { &mut *(self.raw_ptr) }
}
}

impl<T> Drop for Pinned<T> {
fn drop(&mut self) {
if self._drop_inner {
unsafe {
drop(Box::from_raw(self.raw_ptr));
}
}
}
}

//impl <K> Deref for Pinned<Vec<K>> {
// type Target = [K];
// fn deref(&self) -> &Self::Target {
// let vec = unsafe {
// & *(self.raw_ptr)
// };
//
// vec.as_slice()
// }
//}

/// Virtqueue descr flags as defined in the specification.
///
/// See Virtio specification v1.1. - 2.6.5
Expand Down
Loading

0 comments on commit 774c757

Please sign in to comment.