Skip to content

Commit

Permalink
Introduce Opaque type and use it in sync module.
Browse files Browse the repository at this point in the history
It has the advantage that the objects don't have to be zero-initialised
before calling their init functions (e.g., `mutex_init` for mutexes).
This makes the code performance closer to C.

This uses `UnsafeCell::raw_get`, which was stabilised in 1.56.0.

Signed-off-by: Wedson Almeida Filho <wedsonaf@google.com>
  • Loading branch information
wedsonaf committed Nov 5, 2021
1 parent da63645 commit 5755aed
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 31 deletions.
2 changes: 1 addition & 1 deletion rust/kernel/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub mod user_ptr;
pub use build_error::build_error;

pub use crate::error::{Error, Result};
pub use crate::types::{Mode, ScopeGuard};
pub use crate::types::{Mode, Opaque, ScopeGuard};

use core::marker::PhantomData;

Expand Down
9 changes: 4 additions & 5 deletions rust/kernel/sync/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
//!
//! [`Arc`]: https://doc.rust-lang.org/std/sync/struct.Arc.html

use crate::{bindings, Error, Result};
use crate::{bindings, Error, Opaque, Result};
use alloc::{
alloc::{alloc, dealloc},
vec::Vec,
};
use core::{
alloc::Layout,
cell::UnsafeCell,
convert::{AsRef, TryFrom},
marker::{PhantomData, Unsize},
mem::{ManuallyDrop, MaybeUninit},
Expand All @@ -47,7 +46,7 @@ pub struct Ref<T: ?Sized> {

#[repr(C)]
struct RefInner<T: ?Sized> {
refcount: UnsafeCell<bindings::refcount_t>,
refcount: Opaque<bindings::refcount_t>,
data: T,
}

Expand Down Expand Up @@ -85,7 +84,7 @@ impl<T> Ref<T> {
// INVARIANT: The refcount is initialised to a non-zero value.
let value = RefInner {
// SAFETY: Just an FFI call that returns a `refcount_t` initialised to 1.
refcount: UnsafeCell::new(unsafe { bindings::REFCOUNT_INIT(1) }),
refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }),
data: contents,
};
// SAFETY: `inner` is writable and properly aligned.
Expand Down Expand Up @@ -277,7 +276,7 @@ impl<T> TryFrom<Vec<T>> for Ref<[T]> {
core::ptr::slice_from_raw_parts_mut(ptr.as_ptr() as _, v.len()) as *mut RefInner<[T]>;

// SAFETY: Just an FFI call that returns a `refcount_t` initialised to 1.
let count = UnsafeCell::new(unsafe { bindings::REFCOUNT_INIT(1) });
let count = Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) });
// SAFETY: `inner.refcount` is writable and properly aligned.
unsafe { core::ptr::addr_of_mut!((*inner).refcount).write(count) };
// SAFETY: The contents of `v` as readable and properly aligned; `inner.data` is writable
Expand Down
20 changes: 10 additions & 10 deletions rust/kernel/sync/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
//! variable.

use super::{GuardMut, Lock, NeedsLockClass};
use crate::{bindings, str::CStr, task::Task};
use core::{cell::UnsafeCell, marker::PhantomPinned, mem::MaybeUninit, pin::Pin};
use crate::{bindings, str::CStr, task::Task, Opaque};
use core::{marker::PhantomPinned, pin::Pin};

/// Safely initialises a [`CondVar`] with the given name, generating a new lock class.
#[macro_export]
Expand All @@ -27,7 +27,7 @@ const POLLFREE: u32 = 0x4000;
///
/// [`struct wait_queue_head`]: ../../../include/linux/wait.h
pub struct CondVar {
pub(crate) wait_list: UnsafeCell<bindings::wait_queue_head>,
pub(crate) wait_list: Opaque<bindings::wait_queue_head>,

/// A condvar needs to be pinned because it contains a [`struct list_head`] that is
/// self-referential, so it cannot be safely moved once it is initialised.
Expand All @@ -49,7 +49,7 @@ impl CondVar {
/// The caller must call `CondVar::init` before using the conditional variable.
pub unsafe fn new() -> Self {
Self {
wait_list: UnsafeCell::new(bindings::wait_queue_head::default()),
wait_list: Opaque::uninit(),
_pin: PhantomPinned,
}
}
Expand All @@ -62,19 +62,19 @@ impl CondVar {
#[must_use = "wait returns if a signal is pending, so the caller must check the return value"]
pub fn wait<L: Lock>(&self, guard: &mut GuardMut<'_, L>) -> bool {
let lock = guard.guard.lock;
let mut wait = MaybeUninit::<bindings::wait_queue_entry>::uninit();
let wait = Opaque::<bindings::wait_queue_entry>::uninit();

// SAFETY: `wait` points to valid memory.
unsafe { bindings::init_wait(wait.as_mut_ptr()) };
unsafe { bindings::init_wait(wait.get()) };

// SAFETY: Both `wait` and `wait_list` point to valid memory.
unsafe {
bindings::prepare_to_wait_exclusive(
self.wait_list.get(),
wait.as_mut_ptr(),
wait.get(),
bindings::TASK_INTERRUPTIBLE as _,
);
}
)
};

// SAFETY: The guard is evidence that the caller owns the lock.
unsafe { lock.unlock(&mut guard.guard.context) };
Expand All @@ -85,7 +85,7 @@ impl CondVar {
guard.guard.context = lock.lock_noguard();

// SAFETY: Both `wait` and `wait_list` point to valid memory.
unsafe { bindings::finish_wait(self.wait_list.get(), wait.as_mut_ptr()) };
unsafe { bindings::finish_wait(self.wait_list.get(), wait.get()) };

Task::current().signal_pending()
}
Expand Down
11 changes: 4 additions & 7 deletions rust/kernel/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
//! This module allows Rust code to use the kernel's [`struct mutex`].

use super::{CreatableLock, GuardMut, Lock};
use crate::bindings;
use crate::str::CStr;
use crate::{bindings, str::CStr, Opaque};
use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};

/// Safely initialises a [`Mutex`] with the given name, generating a new lock class.
Expand All @@ -30,7 +29,7 @@ macro_rules! mutex_init {
/// [`struct mutex`]: ../../../include/linux/mutex.h
pub struct Mutex<T: ?Sized> {
/// The kernel `struct mutex` object.
mutex: UnsafeCell<bindings::mutex>,
mutex: Opaque<bindings::mutex>,

/// A mutex needs to be pinned because it contains a [`struct list_head`] that is
/// self-referential, so it cannot be safely moved once it is initialised.
Expand All @@ -55,7 +54,7 @@ impl<T> Mutex<T> {
/// The caller must call [`Mutex::init_lock`] before using the mutex.
pub unsafe fn new(t: T) -> Self {
Self {
mutex: UnsafeCell::new(bindings::mutex::default()),
mutex: Opaque::uninit(),
data: UnsafeCell::new(t),
_pin: PhantomPinned,
}
Expand Down Expand Up @@ -94,9 +93,7 @@ unsafe impl<T: ?Sized> Lock for Mutex<T> {

fn lock_noguard(&self) {
// SAFETY: `mutex` points to valid memory.
unsafe {
bindings::mutex_lock(self.mutex.get());
}
unsafe { bindings::mutex_lock(self.mutex.get()) };
}

unsafe fn unlock(&self, _: &mut ()) {
Expand Down
6 changes: 3 additions & 3 deletions rust/kernel/sync/seqlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! See <https://www.kernel.org/doc/Documentation/locking/seqlock.rst>.

use super::{CreatableLock, Guard, Lock, NeedsLockClass};
use crate::{bindings, str::CStr};
use crate::{bindings, str::CStr, Opaque};
use core::{cell::UnsafeCell, marker::PhantomPinned, ops::Deref, pin::Pin};

/// Exposes sequential locks backed by the kernel's `seqcount_t`.
Expand Down Expand Up @@ -54,7 +54,7 @@ use core::{cell::UnsafeCell, marker::PhantomPinned, ops::Deref, pin::Pin};
/// ```
pub struct SeqLock<L: CreatableLock + ?Sized> {
_p: PhantomPinned,
count: UnsafeCell<bindings::seqcount>,
count: Opaque<bindings::seqcount>,
write_lock: L,
}

Expand All @@ -78,7 +78,7 @@ impl<L: CreatableLock> SeqLock<L> {
{
Self {
_p: PhantomPinned,
count: UnsafeCell::new(bindings::seqcount::default()),
count: Opaque::uninit(),
// SAFETY: `L::init_lock` is called from `SeqLock::init`, which is required to be
// called by the function's safety requirements.
write_lock: unsafe { L::new_lock(data) },
Expand Down
7 changes: 3 additions & 4 deletions rust/kernel/sync/spinlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
//! See <https://www.kernel.org/doc/Documentation/locking/spinlocks.txt>.

use super::{CreatableLock, GuardMut, Lock};
use crate::bindings;
use crate::str::CStr;
use crate::{bindings, str::CStr, Opaque};
use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};

/// Safely initialises a [`SpinLock`] with the given name, generating a new lock class.
Expand All @@ -33,7 +32,7 @@ macro_rules! spinlock_init {
///
/// [`spinlock_t`]: ../../../include/linux/spinlock.h
pub struct SpinLock<T: ?Sized> {
spin_lock: UnsafeCell<bindings::spinlock>,
spin_lock: Opaque<bindings::spinlock>,

/// Spinlocks are architecture-defined. So we conservatively require them to be pinned in case
/// some architecture uses self-references now or in the future.
Expand All @@ -57,7 +56,7 @@ impl<T> SpinLock<T> {
/// The caller must call [`SpinLock::init_lock`] before using the spinlock.
pub unsafe fn new(t: T) -> Self {
Self {
spin_lock: UnsafeCell::new(bindings::spinlock::default()),
spin_lock: Opaque::uninit(),
data: UnsafeCell::new(t),
_pin: PhantomPinned,
}
Expand Down
24 changes: 23 additions & 1 deletion rust/kernel/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
sync::{Ref, RefBorrow},
};
use alloc::boxed::Box;
use core::{ops::Deref, pin::Pin, ptr::NonNull};
use core::{cell::UnsafeCell, mem::MaybeUninit, ops::Deref, pin::Pin, ptr::NonNull};

/// Permissions.
///
Expand Down Expand Up @@ -226,3 +226,25 @@ impl<T: FnOnce()> Drop for ScopeGuard<T> {
}
}
}

/// Stores an opaque value.
///
/// This is meant to be used with FFI objects that are never interpreted by Rust code.
pub struct Opaque<T>(MaybeUninit<UnsafeCell<T>>);

impl<T> Opaque<T> {
/// Creates a new opaque value.
pub fn new(value: T) -> Self {
Self(MaybeUninit::new(UnsafeCell::new(value)))
}

/// Creates an uninitialised value.
pub fn uninit() -> Self {
Self(MaybeUninit::uninit())
}

/// Returns a raw pointer to the opaque data.
pub fn get(&self) -> *mut T {
UnsafeCell::raw_get(self.0.as_ptr())
}
}

0 comments on commit 5755aed

Please sign in to comment.