Add a method to consume a `Box` and return a `NonNull`. This is a convenience wrapper around `Self::into_raw` for callers that need a `NonNull` pointer rather than a raw pointer. Signed-off-by: Andreas Hindborg Reviewed-by: Alice Ryhl Reviewed-by: Gary Guo --- rust/kernel/alloc/kbox.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index 35d1e015848dd..d534e8adcf7b3 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -211,6 +211,15 @@ pub fn leak<'a>(b: Self) -> &'a mut T { // which points to an initialized instance of `T`. unsafe { &mut *Box::into_raw(b) } } + + /// Consumes the `Box` and returns a `NonNull`. + /// + /// Like [`Self::into_raw`], but returns a `NonNull`. + #[inline] + pub fn into_non_null(b: Self) -> NonNull { + // SAFETY: `KBox::into_raw` returns a valid pointer. + unsafe { NonNull::new_unchecked(Self::into_raw(b)) } + } } impl Box, A> -- 2.51.2 From: Asahi Lina By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike `AlwaysRefCounted`, this mechanism expects the reference to be unique within Rust, and does not allow cloning. Conceptually, this is similar to a `KBox`, except that it delegates resource management to the `T` instead of using a generic allocator. [ om: - Split code into separate file and `pub use` it from types.rs. - Make from_raw() and into_raw() public. - Remove OwnableMut, and make DerefMut dependent on Unpin instead. - Usage example/doctest for Ownable/Owned. - Fixes to documentation and commit message. ] Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/ Signed-off-by: Asahi Lina Co-developed-by: Oliver Mangold Signed-off-by: Oliver Mangold Reviewed-by: Boqun Feng Reviewed-by: Daniel Almeida Reviewed-by: Gary Guo Reviewed-by: Alice Ryhl [ Andreas: Updated documentation, examples, and formatting. Change safety requirements, safety comments. ] Co-developed-by: Andreas Hindborg Signed-off-by: Andreas Hindborg --- rust/kernel/lib.rs | 1 + rust/kernel/owned.rs | 188 +++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/sync/aref.rs | 5 ++ rust/kernel/types.rs | 5 ++ 4 files changed, 199 insertions(+) diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 9512af7156df2..eb5256204a174 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -101,6 +101,7 @@ pub mod of; #[cfg(CONFIG_PM_OPP)] pub mod opp; +pub mod owned; pub mod page; #[cfg(CONFIG_PCI)] pub mod pci; diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs new file mode 100644 index 0000000000000..7fe9ec3e55126 --- /dev/null +++ b/rust/kernel/owned.rs @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Unique owned pointer types for objects with custom drop logic. +//! +//! These pointer types are useful for C-allocated objects which by API-contract +//! are owned by Rust, but need to be freed through the C API. + +use core::{ + mem::ManuallyDrop, + ops::{ + Deref, + DerefMut, // + }, + pin::Pin, + ptr::NonNull, // +}; + +/// Types that specify their own way of performing allocation and destruction. Typically, this trait +/// is implemented on types from the C side. +/// +/// Implementing this trait allows types to be referenced via the [`Owned`] pointer type. This +/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather +/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is +/// executed when the owned reference [`Owned`] pointing to the object is dropped. +/// +/// Note: The underlying object is not required to provide internal reference counting, because it +/// represents a unique, owned reference. If reference counting (on the Rust side) is required, +/// [`AlwaysRefCounted`](crate::sync::aref::AlwaysRefCounted) should be implemented. +/// +/// # Examples +/// +/// A minimal example implementation of [`Ownable`] and its usage with [`Owned`] looks like +/// this: +/// +/// ``` +/// # #![expect(clippy::disallowed_names)] +/// # use core::cell::Cell; +/// # use core::ptr::NonNull; +/// # use kernel::sync::global_lock; +/// # use kernel::alloc::{flags, kbox::KBox, AllocError}; +/// # use kernel::types::{Owned, Ownable}; +/// +/// // Let's count the allocations to see if freeing works. +/// kernel::sync::global_lock! { +/// // SAFETY: we call `init()` right below, before doing anything else. +/// unsafe(uninit) static FOO_ALLOC_COUNT: Mutex = 0; +/// } +/// // SAFETY: We call `init()` only once, here. +/// unsafe { FOO_ALLOC_COUNT.init() }; +/// +/// struct Foo; +/// +/// impl Foo { +/// fn new() -> Result> { +/// // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is +/// // not actually a C-allocated object. +/// let result = KBox::new( +/// Foo {}, +/// flags::GFP_KERNEL, +/// )?; +/// let result = KBox::into_non_null(result); +/// // Count new allocation +/// *FOO_ALLOC_COUNT.lock() += 1; +/// // SAFETY: +/// // - We just allocated the `Self`, thus it is valid and we own it. +/// // - We can transfer this ownership to the `from_raw` method. +/// Ok(unsafe { Owned::from_raw(result) }) +/// } +/// } +/// +/// impl Ownable for Foo { +/// unsafe fn release(this: NonNull) { +/// // SAFETY: The [`KBox`] is still alive. We can pass ownership to the [`KBox`], as +/// // by requirement on calling this function. +/// drop(unsafe { KBox::from_raw(this.as_ptr()) }); +/// // Count released allocation +/// *FOO_ALLOC_COUNT.lock() -= 1; +/// } +/// } +/// +/// { +/// let foo = Foo::new()?; +/// assert!(*FOO_ALLOC_COUNT.lock() == 1); +/// } +/// // `foo` is out of scope now, so we expect no live allocations. +/// assert!(*FOO_ALLOC_COUNT.lock() == 0); +/// # Ok::<(), Error>(()) +/// ``` +pub trait Ownable { + /// Tear down this `Ownable`. + /// + /// Implementers of `Ownable` can use this function to clean up the use of `Self`. This can + /// include freeing the underlying object. + /// + /// # Safety + /// + /// Callers must ensure that they have exclusive ownership of the `Self` pointed to by `this`, + /// and that this ownership is transferred to the `release` method. `this` must not be used + /// after calling this method, as the underlying object may have been freed. + unsafe fn release(this: NonNull); +} + +/// A mutable reference to an owned `T`. +/// +/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is +/// dropped. +/// +/// # Invariants +/// +/// - Until `T::release` is called, this `Owned` exclusively owns the underlying `T`. +/// - The `T` value is pinned. +pub struct Owned { + ptr: NonNull, +} + +impl Owned { + /// Creates a new instance of [`Owned`]. + /// + /// This function takes over ownership of the underlying object. + /// + /// # Safety + /// + /// Callers must ensure that: + /// - `ptr` points to a valid instance of `T`. + /// - Until `T::release` is called, the returned `Owned` exclusively owns the underlying `T`. + #[inline] + pub unsafe fn from_raw(ptr: NonNull) -> Self { + // INVARIANT: By function safety requirement we satisfy the first invariant of `Self`. + // We treat `T` as pinned from now on. + Self { ptr } + } + + /// Consumes the [`Owned`], returning a raw pointer. + /// + /// This function does not drop the underlying `T`. When this function returns, ownership of the + /// underlying `T` is with the caller. + #[inline] + pub fn into_raw(me: Self) -> NonNull { + ManuallyDrop::new(me).ptr + } + + /// Get a pinned mutable reference to the data owned by this `Owned`. + #[inline] + pub fn as_pin_mut(&mut self) -> Pin<&mut T> { + // SAFETY: The type invariants guarantee that the object is valid, and that we can safely + // return a mutable reference to it. + let unpinned = unsafe { self.ptr.as_mut() }; + + // SAFETY: By type invariant `T` is pinned. + unsafe { Pin::new_unchecked(unpinned) } + } +} + +// SAFETY: It is safe to send an [`Owned`] to another thread when the underlying `T` is [`Send`], +// because of the ownership invariant. Sending an [`Owned`] is equivalent to sending the `T`. +unsafe impl Send for Owned {} + +// SAFETY: It is safe to send [`&Owned`] to another thread when the underlying `T` is [`Sync`], +// because of the ownership invariant. Sending an [`&Owned`] is equivalent to sending the `&T`. +unsafe impl Sync for Owned {} + +impl Deref for Owned { + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + // SAFETY: The type invariants guarantee that the object is valid. + unsafe { self.ptr.as_ref() } + } +} + +impl DerefMut for Owned { + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + // SAFETY: The type invariants guarantee that the object is valid, and that we can safely + // return a mutable reference to it. + unsafe { self.ptr.as_mut() } + } +} + +impl Drop for Owned { + #[inline] + fn drop(&mut self) { + // SAFETY: By existence of `&mut self` we exclusively own `self` and the underlying `T`. As + // we are dropping `self`, we can transfer ownership of the `T` to the `release` method. + unsafe { T::release(self.ptr) }; + } +} diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs index b721b2e00b986..3bd5eb8a1a526 100644 --- a/rust/kernel/sync/aref.rs +++ b/rust/kernel/sync/aref.rs @@ -34,6 +34,11 @@ /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted /// instances of a type. /// +/// Note: Implementing this trait allows types to be wrapped in an [`ARef`]. It requires an +/// internal reference count and provides only shared references. If unique references are required +/// [`Ownable`](crate::types::Ownable) should be implemented which allows types to be wrapped in an +/// [`Owned`](crate::types::Owned). +/// /// # Safety /// /// Implementers must ensure that increments to the reference count keep the object alive in memory diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index ac316fd7b538f..c41eab0ec983c 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -15,6 +15,11 @@ pub mod for_lt; pub use for_lt::ForLt; +pub use crate::owned::{ + Ownable, + Owned, // +}; + /// Used to transfer ownership to and from foreign (non-Rust) languages. /// /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and -- 2.51.2 Implement `ForeignOwnable` for `Owned`. This allows use of `Owned` in places such as the `XArray`. Note that `T` does not need to implement `ForeignOwnable` for `Owned` to implement `ForeignOwnable`. Signed-off-by: Andreas Hindborg Reviewed-by: Gary Guo --- rust/kernel/owned.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs index 7fe9ec3e55126..93a5dfcc1e6f5 100644 --- a/rust/kernel/owned.rs +++ b/rust/kernel/owned.rs @@ -15,6 +15,8 @@ ptr::NonNull, // }; +use kernel::types::ForeignOwnable; + /// Types that specify their own way of performing allocation and destruction. Typically, this trait /// is implemented on types from the C side. /// @@ -186,3 +188,51 @@ fn drop(&mut self) { unsafe { T::release(self.ptr) }; } } + +// SAFETY: We derive the pointer to `T` from a valid `T`, so the returned +// pointer satisfy alignment requirements of `T`. +unsafe impl ForeignOwnable for Owned { + const FOREIGN_ALIGN: usize = core::mem::align_of::(); + + type Borrowed<'a> + = &'a T + where + Self: 'a; + type BorrowedMut<'a> + = Pin<&'a mut T> + where + Self: 'a; + + #[inline] + fn into_foreign(self) -> *mut kernel::ffi::c_void { + Owned::into_raw(self).as_ptr().cast() + } + + #[inline] + unsafe fn from_foreign(ptr: *mut kernel::ffi::c_void) -> Self { + // SAFETY: By function safety contract, `ptr` came from `into_foreign` and cannot be null. + let ptr = unsafe { NonNull::new_unchecked(ptr.cast()) }; + + // SAFETY: By the function safety contract, `ptr` was returned by `into_foreign`, which gave + // up exclusive ownership of a valid, pinned `T`; we retake that ownership here. + unsafe { Owned::from_raw(ptr) } + } + + #[inline] + unsafe fn borrow<'a>(ptr: *mut kernel::ffi::c_void) -> Self::Borrowed<'a> { + // SAFETY: By function safety requirements, `ptr` is valid for use as a + // reference for `'a`. + unsafe { &*ptr.cast() } + } + + #[inline] + unsafe fn borrow_mut<'a>(ptr: *mut kernel::ffi::c_void) -> Self::BorrowedMut<'a> { + // SAFETY: By function safety requirements, `ptr` is valid for use as a + // unique reference for `'a`. + let inner = unsafe { &mut *ptr.cast() }; + + // SAFETY: We never move out of inner, and we do not hand out mutable + // references when `T: !Unpin`. + unsafe { Pin::new_unchecked(inner) } + } +} -- 2.51.2 From: Asahi Lina This allows Page references to be returned as borrowed references, without necessarily owning the struct page. Remove `BorrowedPage` and update users to use `Owned`. Signed-off-by: Asahi Lina [ Andreas: Fix formatting and add a safety comment, update users. ] Signed-off-by: Andreas Hindborg Reviewed-by: Gary Guo --- drivers/android/binder/page_range.rs | 10 +-- rust/kernel/alloc/allocator.rs | 19 +++--- rust/kernel/alloc/allocator/iter.rs | 6 +- rust/kernel/page.rs | 122 +++++++++-------------------------- 4 files changed, 46 insertions(+), 111 deletions(-) diff --git a/drivers/android/binder/page_range.rs b/drivers/android/binder/page_range.rs index e54a90e62402a..7941eb85b4ef4 100644 --- a/drivers/android/binder/page_range.rs +++ b/drivers/android/binder/page_range.rs @@ -33,7 +33,7 @@ sync::{aref::ARef, Mutex, SpinLock}, task::Pid, transmute::FromBytes, - types::Opaque, + types::{Opaque, Owned}, uaccess::UserSliceReader, }; @@ -198,7 +198,7 @@ unsafe impl Send for Inner {} #[repr(C)] struct PageInfo { lru: bindings::list_head, - page: Option, + page: Option>, range: *const ShrinkablePageRange, } @@ -206,7 +206,7 @@ impl PageInfo { /// # Safety /// /// The caller ensures that writing to `me.page` is ok, and that the page is not currently set. - unsafe fn set_page(me: *mut PageInfo, page: Page) { + unsafe fn set_page(me: *mut PageInfo, page: Owned) { // SAFETY: This pointer offset is in bounds. let ptr = unsafe { &raw mut (*me).page }; @@ -229,13 +229,13 @@ unsafe fn get_page<'a>(me: *const PageInfo) -> Option<&'a Page> { let ptr = unsafe { &raw const (*me).page }; // SAFETY: The pointer is valid for reading. - unsafe { (*ptr).as_ref() } + unsafe { (*ptr).as_deref() } } /// # Safety /// /// The caller ensures that writing to `me.page` is ok for the duration of 'a. - unsafe fn take_page(me: *mut PageInfo) -> Option { + unsafe fn take_page(me: *mut PageInfo) -> Option> { // SAFETY: This pointer offset is in bounds. let ptr = unsafe { &raw mut (*me).page }; diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs index cd4203f27aed0..c7b9b069cf75d 100644 --- a/rust/kernel/alloc/allocator.rs +++ b/rust/kernel/alloc/allocator.rs @@ -169,7 +169,7 @@ unsafe fn realloc( } impl Vmalloc { - /// Convert a pointer to a [`Vmalloc`] allocation to a [`page::BorrowedPage`]. + /// Convert a pointer to a [`Vmalloc`] allocation to a [`Page`](page::Page) reference. /// /// # Examples /// @@ -202,20 +202,17 @@ impl Vmalloc { /// /// - `ptr` must be a valid pointer to a [`Vmalloc`] allocation. /// - `ptr` must remain valid for the entire duration of `'a`. - pub unsafe fn to_page<'a>(ptr: NonNull) -> page::BorrowedPage<'a> { + pub unsafe fn to_page<'a>(ptr: NonNull) -> &'a page::Page { // SAFETY: `ptr` is a valid pointer to `Vmalloc` memory. let page = unsafe { bindings::vmalloc_to_page(ptr.as_ptr().cast()) }; - // SAFETY: `vmalloc_to_page` returns a valid pointer to a `struct page` for a valid pointer - // to `Vmalloc` memory. - let page = unsafe { NonNull::new_unchecked(page) }; - // SAFETY: - // - `page` is a valid pointer to a `struct page`, given that by the safety requirements of - // this function `ptr` is a valid pointer to a `Vmalloc` allocation. - // - By the safety requirements of this function `ptr` is valid for the entire lifetime of - // `'a`. - unsafe { page::BorrowedPage::from_raw(page) } + // - `vmalloc_to_page` returns a valid, non-null pointer to a `struct page` for a valid + // pointer to `Vmalloc` memory, given that by the safety requirements of this function + // `ptr` is a valid pointer to a `Vmalloc` allocation. + // - By the safety requirements of this function `ptr`, and hence the `struct page`, is + // valid for the entire lifetime of `'a`. + unsafe { &*page.cast() } } } diff --git a/rust/kernel/alloc/allocator/iter.rs b/rust/kernel/alloc/allocator/iter.rs index 02fda3ea5cae6..8dcc16ed89893 100644 --- a/rust/kernel/alloc/allocator/iter.rs +++ b/rust/kernel/alloc/allocator/iter.rs @@ -9,7 +9,7 @@ ptr::NonNull, // }; -/// An [`Iterator`] of [`page::BorrowedPage`] items owned by a [`Vmalloc`] allocation. +/// An [`Iterator`] of [`Page`](page::Page) references owned by a [`Vmalloc`] allocation. /// /// # Guarantees /// @@ -28,11 +28,11 @@ pub struct VmallocPageIter<'a> { size: usize, /// The current page index of the [`Iterator`]. index: usize, - _p: PhantomData>, + _p: PhantomData<&'a page::Page>, } impl<'a> Iterator for VmallocPageIter<'a> { - type Item = page::BorrowedPage<'a>; + type Item = &'a page::Page; fn next(&mut self) -> Option { let offset = self.index.checked_mul(page::PAGE_SIZE)?; diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs index 8affd8262891b..6dc1c2395acaf 100644 --- a/rust/kernel/page.rs +++ b/rust/kernel/page.rs @@ -12,16 +12,16 @@ code::*, Result, // }, + types::{ + Opaque, + Ownable, + Owned, // + }, uaccess::UserSliceReader, // }; -use core::{ - marker::PhantomData, - mem::ManuallyDrop, - ops::Deref, - ptr::{ - self, - NonNull, // - }, // +use core::ptr::{ + self, + NonNull, // }; /// A bitwise shift for the page size. @@ -65,93 +65,29 @@ pub const fn page_align(addr: usize) -> Option { Some(sum & PAGE_MASK) } -/// Representation of a non-owning reference to a [`Page`]. -/// -/// This type provides a borrowed version of a [`Page`] that is owned by some other entity, e.g. a -/// [`Vmalloc`] allocation such as [`VBox`]. -/// -/// # Example -/// -/// ``` -/// # use kernel::{bindings, prelude::*}; -/// use kernel::page::{BorrowedPage, Page, PAGE_SIZE}; -/// # use core::{mem::MaybeUninit, ptr, ptr::NonNull }; -/// -/// fn borrow_page<'a>(vbox: &'a mut VBox>) -> BorrowedPage<'a> { -/// let ptr = ptr::from_ref(&**vbox); -/// -/// // SAFETY: `ptr` is a valid pointer to `Vmalloc` memory. -/// let page = unsafe { bindings::vmalloc_to_page(ptr.cast()) }; -/// -/// // SAFETY: `vmalloc_to_page` returns a valid pointer to a `struct page` for a valid -/// // pointer to `Vmalloc` memory. -/// let page = unsafe { NonNull::new_unchecked(page) }; -/// -/// // SAFETY: -/// // - `self.0` is a valid pointer to a `struct page`. -/// // - `self.0` is valid for the entire lifetime of `self`. -/// unsafe { BorrowedPage::from_raw(page) } -/// } -/// -/// let mut vbox = VBox::<[u8; PAGE_SIZE]>::new_uninit(GFP_KERNEL)?; -/// let page = borrow_page(&mut vbox); -/// -/// // SAFETY: There is no concurrent read or write to this page. -/// unsafe { page.fill_zero_raw(0, PAGE_SIZE)? }; -/// # Ok::<(), Error>(()) -/// ``` -/// -/// # Invariants -/// -/// The borrowed underlying pointer to a `struct page` is valid for the entire lifetime `'a`. -/// -/// [`VBox`]: kernel::alloc::VBox -/// [`Vmalloc`]: kernel::alloc::allocator::Vmalloc -pub struct BorrowedPage<'a>(ManuallyDrop, PhantomData<&'a Page>); - -impl<'a> BorrowedPage<'a> { - /// Constructs a [`BorrowedPage`] from a raw pointer to a `struct page`. - /// - /// # Safety - /// - /// - `ptr` must point to a valid `bindings::page`. - /// - `ptr` must remain valid for the entire lifetime `'a`. - pub unsafe fn from_raw(ptr: NonNull) -> Self { - let page = Page { page: ptr }; - - // INVARIANT: The safety requirements guarantee that `ptr` is valid for the entire lifetime - // `'a`. - Self(ManuallyDrop::new(page), PhantomData) - } -} - -impl<'a> Deref for BorrowedPage<'a> { - type Target = Page; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -/// Trait to be implemented by types which provide an [`Iterator`] implementation of -/// [`BorrowedPage`] items, such as [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter). +/// Trait to be implemented by types which provide an [`Iterator`] of [`Page`] references, such as +/// [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter). pub trait AsPageIter { /// The [`Iterator`] type, e.g. [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter). - type Iter<'a>: Iterator> + type Iter<'a>: Iterator where Self: 'a; - /// Returns an [`Iterator`] of [`BorrowedPage`] items over all pages owned by `self`. + /// Returns an [`Iterator`] of [`Page`] references over all pages owned by `self`. fn page_iter(&mut self) -> Self::Iter<'_>; } -/// A pointer to a page that owns the page allocation. +/// A `struct page`. +/// +/// A `Page` is accessed through a shared reference or through an owning [`Owned`]; the latter +/// frees the page allocation when it is dropped. /// /// # Invariants /// -/// The pointer is valid, and has ownership over the page. +/// The `Page` is backed by a valid `struct page`. +#[repr(transparent)] pub struct Page { - page: NonNull, + page: Opaque, } // SAFETY: Pages have no logic that relies on them staying on a given thread, so moving them across @@ -185,19 +121,20 @@ impl Page { /// # Ok::<(), kernel::alloc::AllocError>(()) /// ``` #[inline] - pub fn alloc_page(flags: Flags) -> Result { + pub fn alloc_page(flags: Flags) -> Result, AllocError> { // SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it // is always safe to call this method. let page = unsafe { bindings::alloc_pages(flags.as_raw(), 0) }; let page = NonNull::new(page).ok_or(AllocError)?; - // INVARIANT: We just successfully allocated a page, so we now have ownership of the newly - // allocated page. We transfer that ownership to the new `Page` object. - Ok(Self { page }) + // SAFETY: We just successfully allocated a page, so we now have ownership of the newly + // allocated page. We transfer that ownership to the new `Owned` object. + // Since `Page` is transparent, we can cast the pointer directly. + Ok(unsafe { Owned::from_raw(page.cast()) }) } /// Returns a raw pointer to the page. pub fn as_ptr(&self) -> *mut bindings::page { - self.page.as_ptr() + Opaque::cast_into(&self.page) } /// Get the node id containing this page. @@ -372,10 +309,11 @@ pub unsafe fn copy_from_user_slice_raw( } } -impl Drop for Page { +impl Ownable for Page { #[inline] - fn drop(&mut self) { - // SAFETY: By the type invariants, we have ownership of the page and can free it. - unsafe { bindings::__free_pages(self.page.as_ptr(), 0) }; + unsafe fn release(this: NonNull) { + // SAFETY: By the function safety requirements, we have ownership of the page and can free + // it. Since Page is transparent, we can cast the raw pointer directly. + unsafe { bindings::__free_pages(this.as_ptr().cast(), 0) }; } } -- 2.51.2 From: Oliver Mangold There are types where it may both be reference counted in some cases and owned in others. In such cases, obtaining `ARef` from `&T` would be unsound as it allows creation of `ARef` copy from `&Owned`. Therefore, we split `AlwaysRefCounted` into `RefCounted` (which `ARef` would require) and a marker trait to indicate that the type is always reference counted (and not `Ownable`) so the `&T` -> `ARef` conversion is possible. - Rename `AlwaysRefCounted` to `RefCounted`. - Add a new unsafe trait `AlwaysRefCounted`. - Implement the new trait `AlwaysRefCounted` for the newly renamed `RefCounted` implementations. This leaves functionality of existing implementers of `AlwaysRefCounted` intact. Suggested-by: Alice Ryhl Reviewed-by: Daniel Almeida Signed-off-by: Oliver Mangold [ Andreas: Updated commit message and rebase on rust-next (7.2) ] Acked-by: Igor Korotin Acked-by: Danilo Krummrich Acked-by: Viresh Kumar Reviewed-by: Gary Guo Co-developed-by: Andreas Hindborg Signed-off-by: Andreas Hindborg --- rust/kernel/auxiliary.rs | 10 ++++++- rust/kernel/block/mq/request.rs | 19 ++++++++----- rust/kernel/cred.rs | 16 +++++++++-- rust/kernel/device.rs | 12 +++++++-- rust/kernel/device/property.rs | 11 ++++++-- rust/kernel/drm/device.rs | 9 +++++-- rust/kernel/drm/gem/mod.rs | 16 ++++++++--- rust/kernel/fs/file.rs | 23 +++++++++++++--- rust/kernel/i2c.rs | 13 ++++++--- rust/kernel/mm.rs | 22 ++++++++++++--- rust/kernel/mm/mmput_async.rs | 12 +++++++-- rust/kernel/opp.rs | 16 ++++++++--- rust/kernel/owned.rs | 2 +- rust/kernel/pci.rs | 10 ++++++- rust/kernel/pid_namespace.rs | 15 +++++++++-- rust/kernel/platform.rs | 10 ++++++- rust/kernel/pwm.rs | 12 +++++++-- rust/kernel/sync/aref.rs | 59 +++++++++++++++++++++++++---------------- rust/kernel/task.rs | 13 +++++++-- rust/kernel/types.rs | 12 ++++++--- rust/kernel/usb.rs | 17 +++++++++--- rust/kernel/workqueue.rs | 8 +++--- 22 files changed, 260 insertions(+), 77 deletions(-) diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index c42928d5a2393..854525289c8b4 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -19,6 +19,10 @@ to_result, // }, prelude::*, + sync::aref::{ + AlwaysRefCounted, + RefCounted, // + }, types::{ ForLt, ForeignOwnable, @@ -344,7 +348,7 @@ unsafe impl device::AsBusDevice for Device kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. -unsafe impl crate::sync::aref::AlwaysRefCounted for Device { +unsafe impl RefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::get_device(self.as_ref().as_raw()) }; @@ -363,6 +367,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Device`. +unsafe impl AlwaysRefCounted for Device {} + impl AsRef> for Device { fn as_ref(&self) -> &device::Device { // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs index ce3e30c81cb5e..8dad15ae4cfb0 100644 --- a/rust/kernel/block/mq/request.rs +++ b/rust/kernel/block/mq/request.rs @@ -9,7 +9,11 @@ block::mq::Operations, error::Result, sync::{ - aref::{ARef, AlwaysRefCounted}, + aref::{ + ARef, + AlwaysRefCounted, + RefCounted, // + }, atomic::Relaxed, Refcount, }, @@ -229,11 +233,10 @@ unsafe impl Send for Request {} // mutate `self` are internally synchronized` unsafe impl Sync for Request {} -// SAFETY: All instances of `Request` are reference counted. This -// implementation of `AlwaysRefCounted` ensure that increments to the ref count -// keeps the object alive in memory at least until a matching reference count -// decrement is executed. -unsafe impl AlwaysRefCounted for Request { +// SAFETY: All instances of `Request` are reference counted. This implementation of `RefCounted` +// ensure that increments to the ref count keeps the object alive in memory at least until a +// matching reference count decrement is executed. +unsafe impl RefCounted for Request { fn inc_ref(&self) { self.wrapper_ref().refcount().inc(); } @@ -255,3 +258,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull) { } } } + +// SAFETY: We currently do not implement `Ownable`, thus it is okay to obtain an `ARef` +// from a `&Request` (but this will change in the future). +unsafe impl AlwaysRefCounted for Request {} diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs index ffa156b9df377..b17736a9adcd5 100644 --- a/rust/kernel/cred.rs +++ b/rust/kernel/cred.rs @@ -8,7 +8,15 @@ //! //! Reference: -use crate::{bindings, sync::aref::AlwaysRefCounted, task::Kuid, types::Opaque}; +use crate::{ + bindings, + sync::aref::RefCounted, + task::Kuid, + types::{ + AlwaysRefCounted, + Opaque, // + }, // +}; /// Wraps the kernel's `struct cred`. /// @@ -76,7 +84,7 @@ pub fn euid(&self) -> Kuid { } // SAFETY: The type invariants guarantee that `Credential` is always ref-counted. -unsafe impl AlwaysRefCounted for Credential { +unsafe impl RefCounted for Credential { #[inline] fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. @@ -90,3 +98,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull) { unsafe { bindings::put_cred(obj.cast().as_ptr()) }; } } + +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Credential`. +unsafe impl AlwaysRefCounted for Credential {} diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 645afc49a27d6..2e90f6a06fd05 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -8,8 +8,12 @@ bindings, fmt, prelude::*, - sync::aref::ARef, + sync::aref::{ + ARef, + RefCounted, // + }, types::{ + AlwaysRefCounted, ForeignOwnable, Opaque, // }, // @@ -448,7 +452,7 @@ pub fn name(&self) -> &CStr { kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. -unsafe impl crate::sync::aref::AlwaysRefCounted for Device { +unsafe impl RefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::get_device(self.as_raw()) }; @@ -460,6 +464,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Device`. +unsafe impl AlwaysRefCounted for Device {} + // SAFETY: As by the type invariant `Device` can be sent to any thread. unsafe impl Send for Device {} diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs index 5aead835fbbc0..cee7e25013689 100644 --- a/rust/kernel/device/property.rs +++ b/rust/kernel/device/property.rs @@ -14,7 +14,10 @@ fmt, prelude::*, str::{CStr, CString}, - sync::aref::ARef, + sync::aref::{ + ARef, + AlwaysRefCounted, // + }, types::Opaque, }; @@ -360,7 +363,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { } // SAFETY: Instances of `FwNode` are always reference-counted. -unsafe impl crate::sync::aref::AlwaysRefCounted for FwNode { +unsafe impl crate::sync::aref::RefCounted for FwNode { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the // refcount is non-zero. @@ -374,6 +377,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&FwNode`. +unsafe impl AlwaysRefCounted for FwNode {} + enum Node<'a> { Borrowed(&'a FwNode), Owned(ARef), diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs index 403fc35353c74..368742a258376 100644 --- a/rust/kernel/drm/device.rs +++ b/rust/kernel/drm/device.rs @@ -15,7 +15,8 @@ prelude::*, sync::aref::{ ARef, - AlwaysRefCounted, // + AlwaysRefCounted, + RefCounted, // }, types::Opaque, workqueue::{ @@ -227,7 +228,7 @@ fn deref(&self) -> &Self::Target { // SAFETY: DRM device objects are always reference counted and the get/put functions // satisfy the requirements. -unsafe impl AlwaysRefCounted for Device { +unsafe impl RefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::drm_dev_get(self.as_raw()) }; @@ -242,6 +243,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Device`. +unsafe impl AlwaysRefCounted for Device {} + impl AsRef for Device { fn as_ref(&self) -> &device::Device { // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid, diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs index 01b5bd47a3332..30d3718578fe8 100644 --- a/rust/kernel/drm/gem/mod.rs +++ b/rust/kernel/drm/gem/mod.rs @@ -17,7 +17,7 @@ prelude::*, sync::aref::{ ARef, - AlwaysRefCounted, // + RefCounted, // }, types::Opaque, }; @@ -29,7 +29,7 @@ #[cfg(CONFIG_RUST_DRM_GEM_SHMEM_HELPER)] pub mod shmem; -/// A macro for implementing [`AlwaysRefCounted`] for any GEM object type. +/// A macro for implementing [`RefCounted`] for any GEM object type. /// /// Since all GEM objects use the same refcounting scheme. #[macro_export] @@ -42,7 +42,7 @@ impl $( <$( $tparam_id:ident ),+> )? for $type:ty )? ) => { // SAFETY: All GEM objects are refcounted. - unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::AlwaysRefCounted for $type + unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::RefCounted for $type where Self: IntoGEMObject, $( $( $bind_param : $bind_trait ),+ )? @@ -61,6 +61,14 @@ unsafe fn dec_ref(obj: core::ptr::NonNull) { unsafe { bindings::drm_gem_object_put(obj) }; } } + + // SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<$type>` from a + // `&$type`. + unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::AlwaysRefCounted for $type + where + Self: IntoGEMObject, + $( $( $bind_param : $bind_trait ),+ )? + {} }; } #[cfg_attr(not(CONFIG_RUST_DRM_GEM_SHMEM_HELPER), allow(unused))] @@ -98,7 +106,7 @@ fn close(_obj: &::Object, _file: &DriverFile) } /// Trait that represents a GEM object subtype -pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted { +pub trait IntoGEMObject: Sized + super::private::Sealed + RefCounted { /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as /// this owning object is valid. fn as_raw(&self) -> *mut bindings::drm_gem_object; diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs index 23ee689bd2400..720e57418358d 100644 --- a/rust/kernel/fs/file.rs +++ b/rust/kernel/fs/file.rs @@ -12,8 +12,15 @@ cred::Credential, error::{code::*, to_result, Error, Result}, fmt, - sync::aref::{ARef, AlwaysRefCounted}, - types::{NotThreadSafe, Opaque}, + sync::aref::{ + ARef, + RefCounted, // + }, + types::{ + AlwaysRefCounted, + NotThreadSafe, + Opaque, // + }, // }; use core::ptr; @@ -197,7 +204,7 @@ unsafe impl Sync for File {} // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation // makes `ARef` own a normal refcount. -unsafe impl AlwaysRefCounted for File { +unsafe impl RefCounted for File { #[inline] fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. @@ -212,6 +219,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&File`. +unsafe impl AlwaysRefCounted for File {} + /// Wraps the kernel's `struct file`. Not thread safe. /// /// This type represents a file that is not known to be safe to transfer across thread boundaries. @@ -233,7 +244,7 @@ pub struct LocalFile { // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation // makes `ARef` own a normal refcount. -unsafe impl AlwaysRefCounted for LocalFile { +unsafe impl RefCounted for LocalFile { #[inline] fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. @@ -249,6 +260,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&LocalFile`. +unsafe impl AlwaysRefCounted for LocalFile {} + impl LocalFile { /// Constructs a new `struct file` wrapper from a file descriptor. /// diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs index 624b971ca8b0b..02b2c9220eb11 100644 --- a/rust/kernel/i2c.rs +++ b/rust/kernel/i2c.rs @@ -18,7 +18,8 @@ prelude::*, sync::aref::{ ARef, - AlwaysRefCounted, // + AlwaysRefCounted, + RefCounted, // }, types::Opaque, // }; @@ -424,7 +425,7 @@ pub fn get(index: i32) -> Result> { kernel::impl_device_context_into_aref!(I2cAdapter); // SAFETY: Instances of `I2cAdapter` are always reference-counted. -unsafe impl AlwaysRefCounted for I2cAdapter { +unsafe impl RefCounted for I2cAdapter { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::i2c_get_adapter(self.index()) }; @@ -435,6 +436,9 @@ unsafe fn dec_ref(obj: NonNull) { unsafe { bindings::i2c_put_adapter(obj.as_ref().as_raw()) } } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from an +// `&I2cAdapter`. +unsafe impl AlwaysRefCounted for I2cAdapter {} /// The i2c board info representation /// @@ -500,7 +504,7 @@ unsafe impl device::AsBusDevice for I2cClient) { unsafe { bindings::put_device(&raw mut (*obj.as_ref().as_raw()).dev) } } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from an +// `&I2cClient`. +unsafe impl AlwaysRefCounted for I2cClient {} impl AsRef> for I2cClient { fn as_ref(&self) -> &device::Device { diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs index 4764d7b68f2a7..83ed94fca14ca 100644 --- a/rust/kernel/mm.rs +++ b/rust/kernel/mm.rs @@ -13,8 +13,15 @@ use crate::{ bindings, - sync::aref::{ARef, AlwaysRefCounted}, - types::{NotThreadSafe, Opaque}, + sync::aref::{ + ARef, + RefCounted, // + }, + types::{ + AlwaysRefCounted, + NotThreadSafe, + Opaque, // + }, // }; use core::{ops::Deref, ptr::NonNull}; @@ -55,7 +62,7 @@ unsafe impl Send for Mm {} unsafe impl Sync for Mm {} // SAFETY: By the type invariants, this type is always refcounted. -unsafe impl AlwaysRefCounted for Mm { +unsafe impl RefCounted for Mm { #[inline] fn inc_ref(&self) { // SAFETY: The pointer is valid since self is a reference. @@ -69,6 +76,9 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a `&Mm`. +unsafe impl AlwaysRefCounted for Mm {} + /// A wrapper for the kernel's `struct mm_struct`. /// /// This type is like [`Mm`], but with non-zero `mm_users`. It can only be used when `mm_users` can @@ -91,7 +101,7 @@ unsafe impl Send for MmWithUser {} unsafe impl Sync for MmWithUser {} // SAFETY: By the type invariants, this type is always refcounted. -unsafe impl AlwaysRefCounted for MmWithUser { +unsafe impl RefCounted for MmWithUser { #[inline] fn inc_ref(&self) { // SAFETY: The pointer is valid since self is a reference. @@ -105,6 +115,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&MmWithUser`. +unsafe impl AlwaysRefCounted for MmWithUser {} + // Make all `Mm` methods available on `MmWithUser`. impl Deref for MmWithUser { type Target = Mm; diff --git a/rust/kernel/mm/mmput_async.rs b/rust/kernel/mm/mmput_async.rs index b8d2f051225c7..8fbc396e46028 100644 --- a/rust/kernel/mm/mmput_async.rs +++ b/rust/kernel/mm/mmput_async.rs @@ -10,7 +10,11 @@ use crate::{ bindings, mm::MmWithUser, - sync::aref::{ARef, AlwaysRefCounted}, + sync::aref::{ + ARef, + RefCounted, // + }, + types::AlwaysRefCounted, }; use core::{ops::Deref, ptr::NonNull}; @@ -34,7 +38,7 @@ unsafe impl Send for MmWithUserAsync {} unsafe impl Sync for MmWithUserAsync {} // SAFETY: By the type invariants, this type is always refcounted. -unsafe impl AlwaysRefCounted for MmWithUserAsync { +unsafe impl RefCounted for MmWithUserAsync { #[inline] fn inc_ref(&self) { // SAFETY: The pointer is valid since self is a reference. @@ -48,6 +52,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` +// from a `&MmWithUserAsync`. +unsafe impl AlwaysRefCounted for MmWithUserAsync {} + // Make all `MmWithUser` methods available on `MmWithUserAsync`. impl Deref for MmWithUserAsync { type Target = MmWithUser; diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs index 62e44676125d1..b8db6bdefd077 100644 --- a/rust/kernel/opp.rs +++ b/rust/kernel/opp.rs @@ -16,8 +16,14 @@ ffi::{c_char, c_ulong}, prelude::*, str::CString, - sync::aref::{ARef, AlwaysRefCounted}, - types::Opaque, + sync::aref::{ + ARef, + RefCounted, // + }, + types::{ + AlwaysRefCounted, + Opaque, // + }, // }; #[cfg(CONFIG_CPU_FREQ)] @@ -1041,7 +1047,7 @@ unsafe impl Send for OPP {} unsafe impl Sync for OPP {} /// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted. -unsafe impl AlwaysRefCounted for OPP { +unsafe impl RefCounted for OPP { #[inline] fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. @@ -1055,6 +1061,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from an +// `&OPP`. +unsafe impl AlwaysRefCounted for OPP {} + impl OPP { /// Creates an owned reference to a [`OPP`] from a valid pointer. /// diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs index 93a5dfcc1e6f5..a156267bf8bb1 100644 --- a/rust/kernel/owned.rs +++ b/rust/kernel/owned.rs @@ -27,7 +27,7 @@ /// /// Note: The underlying object is not required to provide internal reference counting, because it /// represents a unique, owned reference. If reference counting (on the Rust side) is required, -/// [`AlwaysRefCounted`](crate::sync::aref::AlwaysRefCounted) should be implemented. +/// [`RefCounted`](crate::types::RefCounted) should be implemented. /// /// # Examples /// diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 5071cae6543fd..ea9ef99cecb07 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -19,6 +19,10 @@ }, prelude::*, str::CStr, + sync::aref::{ + AlwaysRefCounted, + RefCounted, // + }, types::Opaque, ThisModule, // }; @@ -481,7 +485,7 @@ unsafe impl device::AsBusDevice for Device impl<'a> crate::dma::Device<'a> for Device> {} // SAFETY: Instances of `Device` are always reference-counted. -unsafe impl crate::sync::aref::AlwaysRefCounted for Device { +unsafe impl RefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::pci_dev_get(self.as_raw()) }; @@ -493,6 +497,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Device`. +unsafe impl AlwaysRefCounted for Device {} + impl AsRef> for Device { fn as_ref(&self) -> &device::Device { // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs index 979a9718f153d..067f68b99e8c5 100644 --- a/rust/kernel/pid_namespace.rs +++ b/rust/kernel/pid_namespace.rs @@ -7,7 +7,14 @@ //! C header: [`include/linux/pid_namespace.h`](srctree/include/linux/pid_namespace.h) and //! [`include/linux/pid.h`](srctree/include/linux/pid.h) -use crate::{bindings, sync::aref::AlwaysRefCounted, types::Opaque}; +use crate::{ + bindings, + sync::aref::RefCounted, + types::{ + AlwaysRefCounted, + Opaque, // + }, // +}; use core::ptr; /// Wraps the kernel's `struct pid_namespace`. Thread safe. @@ -41,7 +48,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self { } // SAFETY: Instances of `PidNamespace` are always reference-counted. -unsafe impl AlwaysRefCounted for PidNamespace { +unsafe impl RefCounted for PidNamespace { #[inline] fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. @@ -55,6 +62,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from +// a `&PidNamespace`. +unsafe impl AlwaysRefCounted for PidNamespace {} + // SAFETY: // - `PidNamespace::dec_ref` can be called from any thread. // - It is okay to send ownership of `PidNamespace` across thread boundaries. diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs index 9b362e0495d32..0ba676445b06d 100644 --- a/rust/kernel/platform.rs +++ b/rust/kernel/platform.rs @@ -27,6 +27,10 @@ }, of, prelude::*, + sync::aref::{ + AlwaysRefCounted, + RefCounted, // + }, types::Opaque, ThisModule, // }; @@ -518,7 +522,7 @@ pub fn optional_irq_by_name(&self, name: &CStr) -> Result> { impl<'a> crate::dma::Device<'a> for Device> {} // SAFETY: Instances of `Device` are always reference-counted. -unsafe impl crate::sync::aref::AlwaysRefCounted for Device { +unsafe impl RefCounted for Device { fn inc_ref(&self) { // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. unsafe { bindings::get_device(self.as_ref().as_raw()) }; @@ -530,6 +534,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Device`. +unsafe impl AlwaysRefCounted for Device {} + impl AsRef> for Device { fn as_ref(&self) -> &device::Device { // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid diff --git a/rust/kernel/pwm.rs b/rust/kernel/pwm.rs index 6c9d667009ef7..2d1cd74dd98e1 100644 --- a/rust/kernel/pwm.rs +++ b/rust/kernel/pwm.rs @@ -13,7 +13,11 @@ devres, error::{self, to_result}, prelude::*, - sync::aref::{ARef, AlwaysRefCounted}, + sync::aref::{ + ARef, + AlwaysRefCounted, + RefCounted, // + }, types::Opaque, // }; use core::{ @@ -629,7 +633,7 @@ pub fn new<'a>( } // SAFETY: Implements refcounting for `Chip` using the embedded `struct device`. -unsafe impl AlwaysRefCounted for Chip { +unsafe impl RefCounted for Chip { #[inline] fn inc_ref(&self) { // SAFETY: `self.0.get()` points to a valid `pwm_chip` because `self` exists. @@ -647,6 +651,10 @@ unsafe fn dec_ref(obj: NonNull>) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef>` from a +// `&Chip`. +unsafe impl AlwaysRefCounted for Chip {} + // SAFETY: `Chip` is a wrapper around `*mut bindings::pwm_chip`. The underlying C // structure's state is managed and synchronized by the kernel's device model // and PWM core locking mechanisms. Therefore, it is safe to move the `Chip` diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs index 3bd5eb8a1a526..ea5a16b8163a6 100644 --- a/rust/kernel/sync/aref.rs +++ b/rust/kernel/sync/aref.rs @@ -11,7 +11,7 @@ //! underlying object, but this refcount is internal to the object. It essentially is a Rust //! implementation of the `get_` and `put_` pattern used in C for reference counting. //! -//! To make use of [`ARef`], `MyType` needs to implement [`AlwaysRefCounted`]. It is a trait +//! To make use of [`ARef`], `MyType` needs to implement [`RefCounted`]. It is a trait //! for accessing the internal reference count of an object of the `MyType` type. //! //! [`Arc`]: crate::sync::Arc @@ -24,11 +24,9 @@ ptr::NonNull, // }; -/// Types that are _always_ reference counted. +/// Types that are internally reference counted. /// /// It allows such types to define their own custom ref increment and decrement functions. -/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference -/// [`ARef`]. /// /// This is usually implemented by wrappers to existing structures on the C side of the code. For /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted @@ -45,9 +43,8 @@ /// at least until matching decrements are performed. /// /// Implementers must also ensure that all instances are reference-counted. (Otherwise they -/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object -/// alive.) -pub unsafe trait AlwaysRefCounted { +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.) +pub unsafe trait RefCounted { /// Increments the reference count on the object. fn inc_ref(&self); @@ -60,11 +57,27 @@ pub unsafe trait AlwaysRefCounted { /// Callers must ensure that there was a previous matching increment to the reference count, /// and that the object is no longer used after its reference count is decremented (as it may /// result in the object being freed), unless the caller owns another increment on the refcount - /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls - /// [`AlwaysRefCounted::dec_ref`] once). + /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls [`RefCounted::dec_ref`] once). unsafe fn dec_ref(obj: NonNull); } +/// Always reference-counted type. +/// +/// It allows deriving a counted reference [`ARef`] from a `&T`. +/// +/// This provides some convenience, but it allows "escaping" borrow checks on `&T`. As it +/// complicates attempts to ensure that a reference to T is unique, it is optional to provide for +/// [`RefCounted`] types. See *Safety* below. +/// +/// # Safety +/// +/// Implementers must ensure that no safety invariants are violated by upgrading an `&T` to an +/// [`ARef`]. In particular that implies [`AlwaysRefCounted`] and [`crate::types::Ownable`] +/// cannot be implemented for the same type, as this would allow violating the uniqueness guarantee +/// of [`crate::types::Owned`] by dereferencing it into an `&T` and obtaining an [`ARef`] from +/// that. +pub unsafe trait AlwaysRefCounted: RefCounted {} + /// An owned reference to an always-reference-counted object. /// /// The object's reference count is automatically decremented when an instance of [`ARef`] is @@ -75,7 +88,7 @@ pub unsafe trait AlwaysRefCounted { /// /// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In /// particular, the [`ARef`] instance owns an increment on the underlying object's reference count. -pub struct ARef { +pub struct ARef { ptr: NonNull, _p: PhantomData, } @@ -84,19 +97,19 @@ pub struct ARef { // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs // `T` to be `Send` because any thread that has an `ARef` may ultimately access `T` using a // mutable reference, for example, when the reference count reaches zero and `T` is dropped. -unsafe impl Send for ARef {} +unsafe impl Send for ARef {} // SAFETY: It is safe to send `&ARef` to another thread when the underlying `T` is `Sync` // because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, // it needs `T` to be `Send` because any thread that has a `&ARef` may clone it and get an // `ARef` on that thread, so the thread may ultimately access `T` using a mutable reference, for // example, when the reference count reaches zero and `T` is dropped. -unsafe impl Sync for ARef {} +unsafe impl Sync for ARef {} // Even if `T` is pinned, pointers to `T` can still move. -impl Unpin for ARef {} +impl Unpin for ARef {} -impl ARef { +impl ARef { /// Creates a new instance of [`ARef`]. /// /// It takes over an increment of the reference count on the underlying object. @@ -125,12 +138,12 @@ pub unsafe fn from_raw(ptr: NonNull) -> Self { /// /// ``` /// use core::ptr::NonNull; - /// use kernel::sync::aref::{ARef, AlwaysRefCounted}; + /// use kernel::sync::aref::{ARef, RefCounted}; /// /// struct Empty {} /// /// # // SAFETY: TODO. - /// unsafe impl AlwaysRefCounted for Empty { + /// unsafe impl RefCounted for Empty { /// fn inc_ref(&self) {} /// unsafe fn dec_ref(_obj: NonNull) {} /// } @@ -148,7 +161,7 @@ pub fn into_raw(me: Self) -> NonNull { } } -impl Clone for ARef { +impl Clone for ARef { fn clone(&self) -> Self { self.inc_ref(); // SAFETY: We just incremented the refcount above. @@ -156,7 +169,7 @@ fn clone(&self) -> Self { } } -impl Deref for ARef { +impl Deref for ARef { type Target = T; fn deref(&self) -> &Self::Target { @@ -173,7 +186,7 @@ fn from(b: &T) -> Self { } } -impl Drop for ARef { +impl Drop for ARef { fn drop(&mut self) { // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to // decrement. @@ -183,19 +196,19 @@ fn drop(&mut self) { impl PartialEq> for ARef where - T: AlwaysRefCounted + PartialEq, - U: AlwaysRefCounted, + T: RefCounted + PartialEq, + U: RefCounted, { #[inline] fn eq(&self, other: &ARef) -> bool { T::eq(&**self, &**other) } } -impl Eq for ARef {} +impl Eq for ARef {} impl PartialEq<&'_ U> for ARef where - T: AlwaysRefCounted + PartialEq, + T: RefCounted + PartialEq, { #[inline] fn eq(&self, other: &&U) -> bool { diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs index 38273f4eedb51..6259430b0ca31 100644 --- a/rust/kernel/task.rs +++ b/rust/kernel/task.rs @@ -10,7 +10,12 @@ pid_namespace::PidNamespace, prelude::*, sync::aref::ARef, - types::{NotThreadSafe, Opaque}, + types::{ + AlwaysRefCounted, + NotThreadSafe, + Opaque, + RefCounted, // + }, }; use core::{ ops::Deref, @@ -347,7 +352,7 @@ pub fn group_leader(&self) -> &Task { } // SAFETY: The type invariants guarantee that `Task` is always refcounted. -unsafe impl crate::sync::aref::AlwaysRefCounted for Task { +unsafe impl RefCounted for Task { #[inline] fn inc_ref(&self) { // SAFETY: The existence of a shared reference means that the refcount is nonzero. @@ -361,6 +366,10 @@ unsafe fn dec_ref(obj: ptr::NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Task`. +unsafe impl AlwaysRefCounted for Task {} + impl PartialEq for Task { #[inline] fn eq(&self, other: &Self) -> bool { diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index c41eab0ec983c..5ef763717e59a 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -15,9 +15,15 @@ pub mod for_lt; pub use for_lt::ForLt; -pub use crate::owned::{ - Ownable, - Owned, // +pub use crate::{ + owned::{ + Ownable, + Owned, // + }, + sync::aref::{ + AlwaysRefCounted, + RefCounted, // + }, // }; /// Used to transfer ownership to and from foreign (non-Rust) languages. diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs index 7aff0c82d0afc..59350c6b0df2a 100644 --- a/rust/kernel/usb.rs +++ b/rust/kernel/usb.rs @@ -18,7 +18,10 @@ to_result, // }, prelude::*, - sync::aref::AlwaysRefCounted, + sync::aref::{ + AlwaysRefCounted, + RefCounted, // + }, types::Opaque, ThisModule, // }; @@ -392,7 +395,7 @@ fn as_ref(&self) -> &Device { } // SAFETY: Instances of `Interface` are always reference-counted. -unsafe impl AlwaysRefCounted for Interface { +unsafe impl RefCounted for Interface { fn inc_ref(&self) { // SAFETY: The invariants of `Interface` guarantee that `self.as_raw()` // returns a valid `struct usb_interface` pointer, for which we will @@ -406,6 +409,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Interface`. +unsafe impl AlwaysRefCounted for Interface {} + // SAFETY: A `Interface` is always reference-counted and can be released from any thread. unsafe impl Send for Interface {} @@ -443,7 +450,7 @@ fn as_raw(&self) -> *mut bindings::usb_device { kernel::impl_device_context_into_aref!(Device); // SAFETY: Instances of `Device` are always reference-counted. -unsafe impl AlwaysRefCounted for Device { +unsafe impl RefCounted for Device { fn inc_ref(&self) { // SAFETY: The invariants of `Device` guarantee that `self.as_raw()` // returns a valid `struct usb_device` pointer, for which we will @@ -457,6 +464,10 @@ unsafe fn dec_ref(obj: NonNull) { } } +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef` from a +// `&Device`. +unsafe impl AlwaysRefCounted for Device {} + impl AsRef> for Device { fn as_ref(&self) -> &device::Device { // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs index 7e253b6f299ce..77673b8ea45fb 100644 --- a/rust/kernel/workqueue.rs +++ b/rust/kernel/workqueue.rs @@ -192,7 +192,7 @@ sync::{ aref::{ ARef, - AlwaysRefCounted, // + RefCounted, // }, Arc, LockClassKey, // @@ -954,7 +954,7 @@ unsafe impl RawDelayedWorkItem for Pin> // implementation of `WorkItemPointer` for `ARef`. unsafe impl WorkItemPointer for ARef where - T: AlwaysRefCounted, + T: RefCounted, T: WorkItem, T: HasWork, { @@ -987,7 +987,7 @@ unsafe impl WorkItemPointer for ARef // requirements of `WorkItemPointer`. unsafe impl RawWorkItem for ARef where - T: AlwaysRefCounted, + T: RefCounted, T: WorkItem, T: HasWork, { @@ -1020,7 +1020,7 @@ unsafe impl RawDelayedWorkItem for ARef where T: WorkItem, T: HasDelayedWork, - T: AlwaysRefCounted, + T: RefCounted, { } -- 2.51.2 From: Oliver Mangold SAFETY comment in rustdoc example was just 'TODO'. Fixed. Signed-off-by: Oliver Mangold Reviewed-by: Daniel Almeida Co-developed-by: Andreas Hindborg Signed-off-by: Andreas Hindborg --- rust/kernel/sync/aref.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs index ea5a16b8163a6..f26ca39b84d0d 100644 --- a/rust/kernel/sync/aref.rs +++ b/rust/kernel/sync/aref.rs @@ -142,7 +142,9 @@ pub unsafe fn from_raw(ptr: NonNull) -> Self { /// /// struct Empty {} /// - /// # // SAFETY: TODO. + /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references and never + /// // frees the underlying object. Thus we can act as owning an increment on the refcount for + /// // the object that we pass to the newly created `ARef`. /// unsafe impl RefCounted for Empty { /// fn inc_ref(&self) {} /// unsafe fn dec_ref(_obj: NonNull) {} @@ -150,7 +152,7 @@ pub unsafe fn from_raw(ptr: NonNull) -> Self { /// /// let mut data = Empty {}; /// let ptr = NonNull::::new(&mut data).unwrap(); - /// # // SAFETY: TODO. + /// // SAFETY: We keep `data` around longer than the `ARef`. /// let data_ref: ARef = unsafe { ARef::from_raw(ptr) }; /// let raw_ptr: NonNull = ARef::into_raw(data_ref); /// -- 2.51.2 From: Oliver Mangold Types implementing one of these traits can safely convert between an `ARef` and an `Owned`. This is useful for types which generally are accessed through an `ARef` but have methods which can only safely be called when the reference is unique, like e.g. `block::mq::Request::end_ok()`. Signed-off-by: Oliver Mangold [ Andreas: Fix formatting, update documentation, fix error handling in examples. ] Co-developed-by: Andreas Hindborg Signed-off-by: Andreas Hindborg --- rust/kernel/owned.rs | 145 +++++++++++++++++++++++++++++++++++++++++++++-- rust/kernel/sync/aref.rs | 16 +++++- rust/kernel/types.rs | 1 + 3 files changed, 156 insertions(+), 6 deletions(-) diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs index a156267bf8bb1..acb611f084ff3 100644 --- a/rust/kernel/owned.rs +++ b/rust/kernel/owned.rs @@ -14,20 +14,26 @@ pin::Pin, ptr::NonNull, // }; +use kernel::{ + sync::aref::ARef, + types::RefCounted, // +}; use kernel::types::ForeignOwnable; /// Types that specify their own way of performing allocation and destruction. Typically, this trait /// is implemented on types from the C side. /// -/// Implementing this trait allows types to be referenced via the [`Owned`] pointer type. This -/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather -/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is -/// executed when the owned reference [`Owned`] pointing to the object is dropped. +/// Implementing this trait allows types to be referenced via the [`Owned`] pointer type. +/// - This is useful when it is desirable to tie the lifetime of an object reference to an owned +/// object, rather than pass around a bare reference. +/// - [`Ownable`] types can define custom drop logic that is executed when the owned reference +/// of type [`Owned<_>`] pointing to the object is dropped. /// /// Note: The underlying object is not required to provide internal reference counting, because it /// represents a unique, owned reference. If reference counting (on the Rust side) is required, -/// [`RefCounted`](crate::types::RefCounted) should be implemented. +/// [`RefCounted`] should be implemented. [`OwnableRefCounted`] should be implemented if conversion +/// between unique and shared (reference counted) ownership is needed. /// /// # Examples /// @@ -99,6 +105,8 @@ pub trait Ownable { /// Callers must ensure that they have exclusive ownership of the `Self` pointed to by `this`, /// and that this ownership is transferred to the `release` method. `this` must not be used /// after calling this method, as the underlying object may have been freed. + /// + /// `this` is pinned and implementers of this method must observe this constraint. unsafe fn release(this: NonNull); } @@ -136,6 +144,8 @@ pub unsafe fn from_raw(ptr: NonNull) -> Self { /// /// This function does not drop the underlying `T`. When this function returns, ownership of the /// underlying `T` is with the caller. + /// + /// Note that the returned pointer is pinned. #[inline] pub fn into_raw(me: Self) -> NonNull { ManuallyDrop::new(me).ptr @@ -236,3 +246,128 @@ unsafe fn borrow_mut<'a>(ptr: *mut kernel::ffi::c_void) -> Self::BorrowedMut<'a> unsafe { Pin::new_unchecked(inner) } } } + +/// A trait for objects that can be wrapped in either one of the reference types [`Owned`] and +/// [`ARef`]. +/// +/// # Examples +/// +/// A minimal example implementation of [`OwnableRefCounted`], [`Ownable`] and its usage with +/// [`ARef`] and [`Owned`] looks like this: +/// +/// ``` +/// # #![expect(clippy::disallowed_names)] +/// # use core::cell::Cell; +/// # use core::ptr::NonNull; +/// # use kernel::alloc::{flags, kbox::KBox, AllocError}; +/// # use kernel::sync::aref::{ARef, RefCounted}; +/// # use kernel::types::{Owned, Ownable, OwnableRefCounted}; +/// +/// // An internally refcounted struct for demonstration purposes. +/// // +/// // # Invariants +/// // +/// // - `refcount` is always non-zero for a valid object. +/// // - `refcount` is >1 if there is more than one Rust reference to it. +/// // +/// struct Foo { +/// refcount: Cell, +/// } +/// +/// impl Foo { +/// fn new() -> Result> { +/// // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is +/// // not actually a C-allocated object. +/// // INVARIANT: We initialize `refcount` to 1, satisfying the invariants. +/// let result = KBox::new( +/// Foo { +/// refcount: Cell::new(1), +/// }, +/// flags::GFP_KERNEL, +/// )?; +/// let result = KBox::into_non_null(result); +/// // SAFETY: +/// // - We just allocated the `Self`, thus it is valid and we own it. +/// // - We can transfer this ownership to the `from_raw` method. +/// Ok(unsafe { Owned::from_raw(result) }) +/// } +/// } +/// +/// // SAFETY: We increment and decrement each time the respective function is called and only free +/// // the `Foo` when the refcount reaches zero. +/// unsafe impl RefCounted for Foo { +/// fn inc_ref(&self) { +/// self.refcount.replace(self.refcount.get() + 1); +/// } +/// +/// unsafe fn dec_ref(this: NonNull) { +/// // SAFETY: By requirement on calling this function, the refcount is non-zero, +/// // implying the underlying object is valid. +/// let refcount = unsafe { &this.as_ref().refcount }; +/// let new_refcount = refcount.get() - 1; +/// if new_refcount == 0 { +/// // The `Foo` will be dropped when `KBox` goes out of scope. +/// // SAFETY: The [`KBox`] is still alive as the old refcount is 1. We can pass +/// // ownership to the [`KBox`] as by requirement on calling this function, +/// // the `Self` will no longer be used by the caller. +/// unsafe { KBox::from_raw(this.as_ptr()) }; +/// } else { +/// refcount.replace(new_refcount); +/// } +/// } +/// } +/// +/// impl OwnableRefCounted for Foo { +/// fn try_from_shared(this: ARef) -> Result, ARef> { +/// if this.refcount.get() == 1 { +/// // SAFETY: The `Foo` is still alive and has no other Rust references as the refcount +/// // is 1. +/// Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) }) +/// } else { +/// Err(this) +/// } +/// } +/// +/// fn into_shared(this: Owned) -> ARef { +/// // SAFETY: An `Owned` holds the unique reference (refcount 1), which we transfer to +/// // the new `ARef`. +/// unsafe { ARef::from_raw(Owned::into_raw(this)) } +/// } +/// } +/// +/// impl Ownable for Foo { +/// unsafe fn release(this: NonNull) { +/// // SAFETY: Using `dec_ref()` from [`RefCounted`] to release is okay, as the refcount is +/// // always 1 for an [`Owned`]. +/// unsafe { Foo::dec_ref(this) }; +/// } +/// } +/// +/// let foo = Foo::new()?; +/// let foo = ARef::from(foo); +/// { +/// let bar = foo.clone(); +/// assert!(Owned::try_from(bar).is_err()); +/// } +/// assert!(Owned::try_from(foo).is_ok()); +/// # Ok::<(), Error>(()) +/// ``` +pub trait OwnableRefCounted: RefCounted + Ownable + Sized { + /// Checks if the [`ARef`] is unique and converts it to an [`Owned`] if that is the case. + /// Otherwise it returns again an [`ARef`] to the same underlying object. + fn try_from_shared(this: ARef) -> Result, ARef>; + + /// Converts the [`Owned`] into an [`ARef`]. + fn into_shared(this: Owned) -> ARef; +} + +impl TryFrom> for Owned { + type Error = ARef; + /// Tries to convert the [`ARef`] to an [`Owned`] by calling + /// [`try_from_shared()`](OwnableRefCounted::try_from_shared). In case the [`ARef`] is not + /// unique, it returns again an [`ARef`] to the same underlying object. + #[inline] + fn try_from(b: ARef) -> Result, Self::Error> { + T::try_from_shared(b) + } +} diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs index f26ca39b84d0d..e6ffe7c650c39 100644 --- a/rust/kernel/sync/aref.rs +++ b/rust/kernel/sync/aref.rs @@ -23,6 +23,10 @@ ops::Deref, ptr::NonNull, // }; +use kernel::types::{ + OwnableRefCounted, + Owned, // +}; /// Types that are internally reference counted. /// @@ -35,7 +39,10 @@ /// Note: Implementing this trait allows types to be wrapped in an [`ARef`]. It requires an /// internal reference count and provides only shared references. If unique references are required /// [`Ownable`](crate::types::Ownable) should be implemented which allows types to be wrapped in an -/// [`Owned`](crate::types::Owned). +/// [`Owned`](crate::types::Owned). Implementing the trait +/// [`OwnableRefCounted`] allows to convert between unique and +/// shared references (i.e. [`Owned`](crate::types::Owned) and +/// [`ARef`]). /// /// # Safety /// @@ -188,6 +195,13 @@ fn from(b: &T) -> Self { } } +impl From> for ARef { + #[inline] + fn from(b: Owned) -> Self { + T::into_shared(b) + } +} + impl Drop for ARef { fn drop(&mut self) { // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 5ef763717e59a..6aa760952cb63 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -18,6 +18,7 @@ pub use crate::{ owned::{ Ownable, + OwnableRefCounted, Owned, // }, sync::aref::{ -- 2.51.2 From: Andreas Hindborg Add a method to `Page` that allows construction of an instance from `struct page` pointer. Signed-off-by: Andreas Hindborg Reviewed-by: Onur Özkan --- rust/kernel/page.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs index 6dc1c2395acaf..c88fda09ead5a 100644 --- a/rust/kernel/page.rs +++ b/rust/kernel/page.rs @@ -143,6 +143,20 @@ pub fn nid(&self) -> i32 { unsafe { bindings::page_to_nid(self.as_ptr()) } } + /// Create a `&Page` from a raw `struct page` pointer. + /// + /// # Safety + /// + /// `ptr` must be convertible to a shared reference with a lifetime of `'a`. + #[inline] + pub unsafe fn from_raw<'a>(ptr: *const bindings::page) -> &'a Self { + // INVARIANT: By the function safety requirements, `ptr` refers to a valid `struct page`, so + // the returned reference upholds the type invariant of `Page`. + // SAFETY: By function safety requirements, `ptr` is not null and is convertible to a shared + // reference. + unsafe { &*ptr.cast() } + } + /// Runs a piece of code with this page mapped to an address. /// /// The page is unmapped when this call returns. -- 2.51.2