From: Asahi Lina <[email protected]> This allows Page references to be returned as borrowed references, without necessarily owning the struct page.
Remove `BorrowedPage` and update users to use `Owned<Page>`. Signed-off-by: Asahi Lina <[email protected]> [ Andreas: Fix formatting and add a safety comment, update users. ] Signed-off-by: Andreas Hindborg <[email protected]> Reviewed-by: Gary Guo <[email protected]> --- 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>, + page: Option<Owned<Page>>, 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<Page>) { // 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<Page> { + unsafe fn take_page(me: *mut PageInfo) -> Option<Owned<Page>> { // 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<u8>) -> page::BorrowedPage<'a> { + pub unsafe fn to_page<'a>(ptr: NonNull<u8>) -> &'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<page::BorrowedPage<'a>>, + _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<Self::Item> { 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<usize> { 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<MaybeUninit<[u8; PAGE_SIZE]>>) -> 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<Page>, 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<bindings::page>) -> 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<Item = BorrowedPage<'a>> + type Iter<'a>: Iterator<Item = &'a Page> 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<Page>`]; 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<bindings::page>, + page: Opaque<bindings::page>, } // 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<Self, AllocError> { + pub fn alloc_page(flags: Flags) -> Result<Owned<Self>, 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<Page>` 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<Self>) { + // 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
