[PATCH v3 0/7] BorrowedPage, IntoPageIter and VmallocPageIter

[PATCH v3 0/7] BorrowedPage, IntoPageIter and VmallocPageIter

[Danilo Krummrich]

This patch series implements the BorrowedPage type, the IntoPageIter trait and
VmallocPageIter type.

IntoPageIter can be implemented by any type that owns pages to allow users to
borrow them through a generic interface.

For instance, this is useful to access and borrow the backing pages of
allocation primitives, such as Box and Vec, backing a scatterlist.

Hence, implement IntoPageIter for VBox and VVec.

Additionally, implement Vmalloc::to_page() and ArrayLayout::size(), which are
dependencies of the above.

Changes in v3:
  - Generalize the previous PageOwner impl of VBox and VVec in VmallocPageIter.
  - Correspondingly, replace PageOwner with IntoPageIter.

Changes in v2:
  - BorrowedPage
    - Add link to Ownable
    - Use borrow_page() in the example
  - Add PageOwner, Vmalloc::to_page(), ArrayLayout, VBox, VVec patches.

Danilo Krummrich (7):
  rust: page: implement BorrowedPage
  rust: alloc: vmalloc: implement Vmalloc::to_page()
  rust: alloc: implement VmallocPageIter
  rust: page: define trait IntoPageIter
  rust: alloc: kbox: implement IntoPageIter for VBox
  rust: alloc: layout: implement ArrayLayout::size()
  rust: alloc: kvec: implement IntoPageIter for VVec

 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/alloc/allocator.rs  | 141 ++++++++++++++++++++++++++++++++
 rust/kernel/alloc/kbox.rs       |  37 ++++++++-
 rust/kernel/alloc/kvec.rs       |  37 ++++++++-
 rust/kernel/alloc/layout.rs     |   5 ++
 rust/kernel/page.rs             |  85 ++++++++++++++++++-
 6 files changed, 303 insertions(+), 3 deletions(-)


base-commit: d2eedaa3909be9102d648a4a0a50ccf64f96c54f
-- 
2.50.1

[PATCH v3 1/7] rust: page: implement BorrowedPage

[Danilo Krummrich]

Currently, a Page always owns the underlying struct page.

However, sometimes a struct page may be owned by some other entity, e.g.
a vmalloc allocation.

Hence, introduce BorrowedPage to support such cases, until the Ownable
solution [1] lands.

This is required by the scatterlist abstractions.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/rust-for-linux/ZnCzLIly3DRK2eab@boqun-archlinux/ [1]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/page.rs             | 75 ++++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 84d60635e8a9..0e140e07758b 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -57,6 +57,7 @@
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
+#include <linux/mm.h>
 #include <linux/miscdevice.h>
 #include <linux/of_device.h>
 #include <linux/pci.h>
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 7c1b17246ed5..631718a6ad7d 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -9,7 +9,12 @@
     error::Result,
     uaccess::UserSliceReader,
 };
-use core::ptr::{self, NonNull};
+use core::{
+    marker::PhantomData,
+    mem::ManuallyDrop,
+    ops::Deref,
+    ptr::{self, NonNull},
+};
 
 /// A bitwise shift for the page size.
 pub const PAGE_SHIFT: usize = bindings::PAGE_SHIFT as usize;
@@ -30,6 +35,74 @@ pub const fn page_align(addr: usize) -> usize {
     (addr + (PAGE_SIZE - 1)) & 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
+    }
+}
+
 /// A pointer to a page that owns the page allocation.
 ///
 /// # Invariants
-- 
2.50.1

[PATCH v3 2/7] rust: alloc: vmalloc: implement Vmalloc::to_page()

[Danilo Krummrich]

Implement an abstraction of vmalloc_to_page() for subsequent use in the
PageOwner implementation of VBox and VVec.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/allocator.rs | 49 ++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index aa2dfa9dca4c..2315f5063011 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -15,6 +15,7 @@
 
 use crate::alloc::{AllocError, Allocator};
 use crate::bindings;
+use crate::page;
 use crate::pr_warn;
 
 /// The contiguous kernel allocator.
@@ -140,6 +141,54 @@ unsafe fn realloc(
     }
 }
 
+impl Vmalloc {
+    /// Convert a pointer to a [`Vmalloc`] allocation to a [`page::BorrowedPage`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use core::ptr::{NonNull, from_mut};
+    /// # use kernel::{page, prelude::*};
+    /// use kernel::alloc::allocator::Vmalloc;
+    ///
+    /// let mut vbox = VBox::<[u8; page::PAGE_SIZE]>::new_uninit(GFP_KERNEL)?;
+    ///
+    /// {
+    ///     // SAFETY: By the type invariant of `Box` the inner pointer of `vbox` is non-null.
+    ///     let ptr = unsafe { NonNull::new_unchecked(from_mut(&mut *vbox)) };
+    ///
+    ///     // SAFETY:
+    ///     // `ptr` is a valid pointer to a `Vmalloc` allocation.
+    ///     // `ptr` is valid for the entire lifetime of `page`.
+    ///     let page = unsafe { Vmalloc::to_page(ptr.cast()) };
+    ///
+    ///     // SAFETY: There is no concurrent read or write to the same page.
+    ///     unsafe { page.fill_zero_raw(0, page::PAGE_SIZE)? };
+    /// }
+    /// # Ok::<(), Error>(())
+    /// ```
+    ///
+    /// # Safety
+    ///
+    /// - `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> {
+        // 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) }
+    }
+}
+
 // SAFETY: `realloc` delegates to `ReallocFunc::call`, which guarantees that
 // - memory remains valid until it is explicitly freed,
 // - passing a pointer to a valid memory allocation is OK,
-- 
2.50.1

[PATCH v3 3/7] rust: alloc: implement VmallocPageIter

[Danilo Krummrich]

Introduce the VmallocPageIter type; an instance of VmallocPageIter may
be exposed by owners of vmalloc allocations to provide borrowed access
to the backing pages of the vmalloc allocation.

For instance, this is useful to access and borrow the backing pages of
allocation primitives, such as Box and Vec, backing a scatterlist.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/allocator.rs | 92 ++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index 2315f5063011..3ed2d5ff1e37 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -10,6 +10,7 @@
 
 use super::Flags;
 use core::alloc::Layout;
+use core::marker::PhantomData;
 use core::ptr;
 use core::ptr::NonNull;
 
@@ -236,3 +237,94 @@ unsafe fn realloc(
         unsafe { ReallocFunc::KVREALLOC.call(ptr, layout, old_layout, flags) }
     }
 }
+
+/// An [`Iterator`] of [`page::BorrowedPage`] items owned by a [`Vmalloc`] allocation.
+///
+/// # Guarantees
+///
+/// The pages iterated by the [`Iterator`] appear in the order as they are mapped in the CPU's
+/// virtual address space ascendingly.
+///
+/// # Invariants
+///
+/// - `buf` is a valid pointer to the beginning of a [`Vmalloc`] allocation.
+/// - `size` is the size of the [`Vmalloc`] allocation `buf` points to.
+pub struct VmallocPageIter<'a> {
+    /// The base address of the [`Vmalloc`] buffer.
+    buf: NonNull<u8>,
+    /// The size of the buffer pointed to by `buf` in bytes.
+    size: usize,
+    /// The current page index of the [`Iterator`].
+    index: usize,
+    _p: PhantomData<&'a u8>,
+}
+
+impl<'a> Iterator for VmallocPageIter<'a> {
+    type Item = page::BorrowedPage<'a>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let offset = self.index.checked_mul(page::PAGE_SIZE)?;
+
+        // Even though `self.size()` may be smaller than `Self::page_count() * page::PAGE_SIZE`, it
+        // is always a number between `(Self::page_count() - 1) * page::PAGE_SIZE` and
+        // `Self::page_count() * page::PAGE_SIZE`, hence the check below is sufficient.
+        if offset < self.size() {
+            self.index += 1;
+        } else {
+            return None;
+        }
+
+        // TODO: Use `NonNull::add()` instead, once the minimum supported compiler version is
+        // bumped to 1.80 or later.
+        //
+        // SAFETY: `offset` is in the interval `[0, (self.page_count() - 1) * page::PAGE_SIZE]`,
+        // hence the resulting pointer is guaranteed to be within the same allocation.
+        let ptr = unsafe { self.buf.as_ptr().add(offset) };
+
+        // SAFETY: `ptr` is guaranteed to be non-null given that it is derived from `self.buf`.
+        let ptr = unsafe { NonNull::new_unchecked(ptr) };
+
+        // SAFETY:
+        // - `ptr` is a valid pointer to a `Vmalloc` allocation.
+        // - `ptr` is valid for the duration of `'a`.
+        Some(unsafe { Vmalloc::to_page(ptr) })
+    }
+}
+
+impl<'a> VmallocPageIter<'a> {
+    /// Creates a new [`VmallocPageIter`] instance.
+    ///
+    /// # Safety
+    ///
+    /// - `buf` must be a pointer to the beginning of a [`Vmalloc`] allocation.
+    /// - `buf` points to must at least be valid for the lifetime of `'a`.
+    /// - `size` must be the size of the [`Vmalloc`] allocation `buf` points to.
+    pub unsafe fn new(buf: NonNull<u8>, size: usize) -> Self {
+        // INVARIANT: `buf` is a valid pointer to the beginning of a `Vmalloc` allocation by the
+        // safety requirements of this function.
+        Self {
+            buf,
+            size,
+            index: 0,
+            _p: PhantomData,
+        }
+    }
+
+    /// Returns the base address of the backing [`Vmalloc`] allocation.
+    pub fn base_address(&self) -> NonNull<u8> {
+        self.buf
+    }
+
+    /// Returns the size of the backing [`Vmalloc`] allocation in bytes.
+    ///
+    /// Note that this is the size the [`Vmalloc`] allocation has been allocated with. Hence, this
+    /// number may be smaller than `[`Self::page_count`] * [`page::PAGE_SIZE`]`.
+    pub fn size(&self) -> usize {
+        self.size
+    }
+
+    /// Returns the number of pages owned by the backing [`Vmalloc`] allocation.
+    pub fn page_count(&self) -> usize {
+        self.size().div_ceil(page::PAGE_SIZE)
+    }
+}
-- 
2.50.1

[PATCH v3 4/7] rust: page: define trait IntoPageIter

[Danilo Krummrich]

The IntoPageIter trait provides a common interface for types that
provide a page iterator, such as VmallocPageIter.

Subsequent patches will leverage this to let VBox and VVec provide a
VmallocPageIter though this trait.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/page.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 631718a6ad7d..bfb5898ab4b5 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -103,6 +103,16 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
+/// Trait to be implemented by types, which provide an [`Iterator`] implementation of
+/// [`BorrowedPage`] items, such as [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
+pub trait IntoPageIter<'a> {
+    /// The [`Iterator`] type, e.g. [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
+    type Iter: Iterator<Item = BorrowedPage<'a>>;
+
+    /// Returns an [`Iterator`] of [`BorrowedPage`] items over all pages owned by `self`.
+    fn page_iter(&'a mut self) -> Self::Iter;
+}
+
 /// A pointer to a page that owns the page allocation.
 ///
 /// # Invariants
-- 
2.50.1

[PATCH v3 5/7] rust: alloc: kbox: implement IntoPageIter for VBox

[Danilo Krummrich]

Implement IntoPageIter for VBox; this allows to iterate and borrow the
backing pages of a VBox. This, for instance, is useful in combination
with VBox backing a scatterlist.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/kbox.rs | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index 856d05aa60f1..8054bed0ab1c 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -3,7 +3,7 @@
 //! Implementation of [`Box`].
 
 #[allow(unused_imports)] // Used in doc comments.
-use super::allocator::{KVmalloc, Kmalloc, Vmalloc};
+use super::allocator::{KVmalloc, Kmalloc, Vmalloc, VmallocPageIter};
 use super::{AllocError, Allocator, Flags};
 use core::alloc::Layout;
 use core::borrow::{Borrow, BorrowMut};
@@ -18,6 +18,7 @@
 
 use crate::ffi::c_void;
 use crate::init::InPlaceInit;
+use crate::page::IntoPageIter;
 use crate::types::ForeignOwnable;
 use pin_init::{InPlaceWrite, Init, PinInit, ZeroableOption};
 
@@ -598,3 +599,37 @@ fn drop(&mut self) {
         unsafe { A::free(self.0.cast(), layout) };
     }
 }
+
+/// # Examples
+///
+/// ```
+/// # use kernel::prelude::*;
+/// use kernel::alloc::allocator::VmallocPageIter;
+/// use kernel::page::{IntoPageIter, PAGE_SIZE};
+///
+/// let mut vbox = VBox::new((), GFP_KERNEL)?;
+///
+/// assert!(vbox.page_iter().next().is_none());
+///
+/// let mut vbox = VBox::<[u8; PAGE_SIZE]>::new_uninit(GFP_KERNEL)?;
+///
+/// let page = vbox.page_iter().next().expect("At least one page should be available.\n");
+///
+/// // SAFETY: There is no concurrent read or write to the same page.
+/// unsafe { page.fill_zero_raw(0, PAGE_SIZE)? };
+/// # Ok::<(), Error>(())
+/// ```
+impl<'a, T> IntoPageIter<'a> for VBox<T> {
+    type Iter = VmallocPageIter<'a>;
+
+    fn page_iter(&'a mut self) -> Self::Iter {
+        let ptr = self.0.cast();
+        let size = size_of::<T>();
+
+        // SAFETY:
+        // - `ptr` is a valid pointer to the beginning of a `Vmalloc` allocation.
+        // - `ptr` is guaranteed to be valid for the lifetime of `'a`.
+        // - `size` is the size of the `Vmalloc` allocation `ptr` points to.
+        unsafe { VmallocPageIter::new(ptr, size) }
+    }
+}
-- 
2.50.1

[PATCH v3 6/7] rust: alloc: layout: implement ArrayLayout::size()

[Danilo Krummrich]

Provide a convenience method for ArrayLayout to calculate the size of
the ArrayLayout in bytes.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/layout.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/alloc/layout.rs b/rust/kernel/alloc/layout.rs
index 93ed514f7cc7..5c3db8e9f930 100644
--- a/rust/kernel/alloc/layout.rs
+++ b/rust/kernel/alloc/layout.rs
@@ -98,6 +98,11 @@ pub const fn len(&self) -> usize {
     pub const fn is_empty(&self) -> bool {
         self.len == 0
     }
+
+    /// Returns the size of the [`ArrayLayout`] in bytes.
+    pub const fn size(&self) -> usize {
+        self.len() * size_of::<T>()
+    }
 }
 
 impl<T> From<ArrayLayout<T>> for Layout {
-- 
2.50.1

[PATCH v3 7/7] rust: alloc: kvec: implement IntoPageIter for VVec

[Danilo Krummrich]

Implement IntoPageIter for VVec; this allows to iterate and borrow the
backing pages of a VVec. This, for instance, is useful in combination
with VVec backing a scatterlist.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/kvec.rs | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index 3c72e0bdddb8..d48db50bc2c4 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -3,10 +3,11 @@
 //! Implementation of [`Vec`].
 
 use super::{
-    allocator::{KVmalloc, Kmalloc, Vmalloc},
+    allocator::{KVmalloc, Kmalloc, Vmalloc, VmallocPageIter},
     layout::ArrayLayout,
     AllocError, Allocator, Box, Flags,
 };
+use crate::page::IntoPageIter;
 use core::{
     borrow::{Borrow, BorrowMut},
     fmt,
@@ -1017,6 +1018,40 @@ fn into_iter(self) -> Self::IntoIter {
     }
 }
 
+/// # Examples
+///
+/// ```
+/// # use kernel::prelude::*;
+/// use kernel::alloc::allocator::VmallocPageIter;
+/// use kernel::page::{IntoPageIter, PAGE_SIZE};
+///
+/// let mut vec = VVec::<u8>::new();
+///
+/// assert!(vec.page_iter().next().is_none());
+///
+/// vec.reserve(PAGE_SIZE, GFP_KERNEL)?;
+///
+/// let page = vec.page_iter().next().expect("At least one page should be available.\n");
+///
+/// // SAFETY: There is no concurrent read or write to the same page.
+/// unsafe { page.fill_zero_raw(0, PAGE_SIZE)? };
+/// # Ok::<(), Error>(())
+/// ```
+impl<'a, T> IntoPageIter<'a> for VVec<T> {
+    type Iter = VmallocPageIter<'a>;
+
+    fn page_iter(&'a mut self) -> Self::Iter {
+        let ptr = self.ptr.cast();
+        let size = self.layout.size();
+
+        // SAFETY:
+        // - `ptr` is a valid pointer to the beginning of a `Vmalloc` allocation.
+        // - `ptr` is guaranteed to be valid for the lifetime of `'a`.
+        // - `size` is the size of the `Vmalloc` allocation `ptr` points to.
+        unsafe { VmallocPageIter::new(ptr, size) }
+    }
+}
+
 /// An [`Iterator`] implementation for [`Vec`] that moves elements out of a vector.
 ///
 /// This structure is created by the [`Vec::into_iter`] method on [`Vec`] (provided by the
-- 
2.50.1

Re: [PATCH v3 4/7] rust: page: define trait IntoPageIter

[Alice Ryhl]

On Fri, Aug 08, 2025 at 08:10:18PM +0200, Danilo Krummrich wrote:
> The IntoPageIter trait provides a common interface for types that
> provide a page iterator, such as VmallocPageIter.
> 
> Subsequent patches will leverage this to let VBox and VVec provide a
> VmallocPageIter though this trait.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

If you call this trait Into*Iter, then it should take ownership of the
underlying object. Since you only borrow it, I would suggest calling
this AsPageIter instead (or go back to a name like PageOwner that
doesn't need to follow these naming conventions).

Alice

Re: [PATCH v3 4/7] rust: page: define trait IntoPageIter

[Danilo Krummrich]

On 8/11/25 10:57 AM, Alice Ryhl wrote:
> On Fri, Aug 08, 2025 at 08:10:18PM +0200, Danilo Krummrich wrote:
>> The IntoPageIter trait provides a common interface for types that
>> provide a page iterator, such as VmallocPageIter.
>>
>> Subsequent patches will leverage this to let VBox and VVec provide a
>> VmallocPageIter though this trait.
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> 
> If you call this trait Into*Iter, then it should take ownership of the
> underlying object. Since you only borrow it, I would suggest calling
> this AsPageIter instead (or go back to a name like PageOwner that
> doesn't need to follow these naming conventions).

Agreed, I think we should call it AsPageIter then.

Re: [PATCH v3 2/7] rust: alloc: vmalloc: implement Vmalloc::to_page()

[Alice Ryhl]

On Fri, Aug 08, 2025 at 08:10:16PM +0200, Danilo Krummrich wrote:
> Implement an abstraction of vmalloc_to_page() for subsequent use in the
> PageOwner implementation of VBox and VVec.
> 
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v3 3/7] rust: alloc: implement VmallocPageIter

[Alice Ryhl]

On Fri, Aug 08, 2025 at 08:10:17PM +0200, Danilo Krummrich wrote:
> Introduce the VmallocPageIter type; an instance of VmallocPageIter may
> be exposed by owners of vmalloc allocations to provide borrowed access
> to the backing pages of the vmalloc allocation.
> 
> For instance, this is useful to access and borrow the backing pages of
> allocation primitives, such as Box and Vec, backing a scatterlist.
> 
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/alloc/allocator.rs | 92 ++++++++++++++++++++++++++++++++++
>  1 file changed, 92 insertions(+)
> 
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index 2315f5063011..3ed2d5ff1e37 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -10,6 +10,7 @@
>  
>  use super::Flags;
>  use core::alloc::Layout;
> +use core::marker::PhantomData;
>  use core::ptr;
>  use core::ptr::NonNull;
>  
> @@ -236,3 +237,94 @@ unsafe fn realloc(
>          unsafe { ReallocFunc::KVREALLOC.call(ptr, layout, old_layout, flags) }
>      }
>  }
> +
> +/// An [`Iterator`] of [`page::BorrowedPage`] items owned by a [`Vmalloc`] allocation.
> +///
> +/// # Guarantees
> +///
> +/// The pages iterated by the [`Iterator`] appear in the order as they are mapped in the CPU's
> +/// virtual address space ascendingly.
> +///
> +/// # Invariants
> +///
> +/// - `buf` is a valid pointer to the beginning of a [`Vmalloc`] allocation.
> +/// - `size` is the size of the [`Vmalloc`] allocation `buf` points to.
> +pub struct VmallocPageIter<'a> {
> +    /// The base address of the [`Vmalloc`] buffer.
> +    buf: NonNull<u8>,
> +    /// The size of the buffer pointed to by `buf` in bytes.
> +    size: usize,
> +    /// The current page index of the [`Iterator`].
> +    index: usize,

My understanding is that an implementation using two pointers that
repeatedly increments the starting pointer until it reaches the end
pointer is more efficient.

Alice

> +    _p: PhantomData<&'a u8>,

Semantically, I think it would make sense to write

	PhantomData<page::BorrowedPage<'a>>

here.

> +}
> +
> +impl<'a> Iterator for VmallocPageIter<'a> {

We can implement size_hint() too.

> +    type Item = page::BorrowedPage<'a>;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        let offset = self.index.checked_mul(page::PAGE_SIZE)?;
> +
> +        // Even though `self.size()` may be smaller than `Self::page_count() * page::PAGE_SIZE`, it
> +        // is always a number between `(Self::page_count() - 1) * page::PAGE_SIZE` and
> +        // `Self::page_count() * page::PAGE_SIZE`, hence the check below is sufficient.
> +        if offset < self.size() {
> +            self.index += 1;
> +        } else {
> +            return None;
> +        }
> +
> +        // TODO: Use `NonNull::add()` instead, once the minimum supported compiler version is
> +        // bumped to 1.80 or later.
> +        //
> +        // SAFETY: `offset` is in the interval `[0, (self.page_count() - 1) * page::PAGE_SIZE]`,
> +        // hence the resulting pointer is guaranteed to be within the same allocation.
> +        let ptr = unsafe { self.buf.as_ptr().add(offset) };
> +
> +        // SAFETY: `ptr` is guaranteed to be non-null given that it is derived from `self.buf`.
> +        let ptr = unsafe { NonNull::new_unchecked(ptr) };
> +
> +        // SAFETY:
> +        // - `ptr` is a valid pointer to a `Vmalloc` allocation.
> +        // - `ptr` is valid for the duration of `'a`.
> +        Some(unsafe { Vmalloc::to_page(ptr) })
> +    }
> +}
> +
> +impl<'a> VmallocPageIter<'a> {
> +    /// Creates a new [`VmallocPageIter`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - `buf` must be a pointer to the beginning of a [`Vmalloc`] allocation.
> +    /// - `buf` points to must at least be valid for the lifetime of `'a`.
> +    /// - `size` must be the size of the [`Vmalloc`] allocation `buf` points to.
> +    pub unsafe fn new(buf: NonNull<u8>, size: usize) -> Self {
> +        // INVARIANT: `buf` is a valid pointer to the beginning of a `Vmalloc` allocation by the
> +        // safety requirements of this function.
> +        Self {
> +            buf,
> +            size,
> +            index: 0,
> +            _p: PhantomData,
> +        }
> +    }
> +
> +    /// Returns the base address of the backing [`Vmalloc`] allocation.
> +    pub fn base_address(&self) -> NonNull<u8> {
> +        self.buf
> +    }
> +
> +    /// Returns the size of the backing [`Vmalloc`] allocation in bytes.
> +    ///
> +    /// Note that this is the size the [`Vmalloc`] allocation has been allocated with. Hence, this
> +    /// number may be smaller than `[`Self::page_count`] * [`page::PAGE_SIZE`]`.
> +    pub fn size(&self) -> usize {
> +        self.size
> +    }
> +
> +    /// Returns the number of pages owned by the backing [`Vmalloc`] allocation.
> +    pub fn page_count(&self) -> usize {
> +        self.size().div_ceil(page::PAGE_SIZE)
> +    }
> +}
> -- 
> 2.50.1
> 

Re: [PATCH v3 4/7] rust: page: define trait IntoPageIter

[Alice Ryhl]

On Fri, Aug 08, 2025 at 08:10:18PM +0200, Danilo Krummrich wrote:
> The IntoPageIter trait provides a common interface for types that
> provide a page iterator, such as VmallocPageIter.
> 
> Subsequent patches will leverage this to let VBox and VVec provide a
> VmallocPageIter though this trait.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/page.rs | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index 631718a6ad7d..bfb5898ab4b5 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -103,6 +103,16 @@ fn deref(&self) -> &Self::Target {
>      }
>  }
>  
> +/// Trait to be implemented by types, which provide an [`Iterator`] implementation of
> +/// [`BorrowedPage`] items, such as [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
> +pub trait IntoPageIter<'a> {
> +    /// The [`Iterator`] type, e.g. [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
> +    type Iter: Iterator<Item = BorrowedPage<'a>>;

Oh, also, I would put the lifetime on Iter.

trait AsPageIter {
    type Iter<'a>: Iterator<Item = BorrowedPage<'a>>;
    fn page_iter(&mut self) -> Self::Iter<'_>;
}

Alice

Re: [PATCH v3 3/7] rust: alloc: implement VmallocPageIter

[Danilo Krummrich]

On Mon Aug 11, 2025 at 11:14 AM CEST, Alice Ryhl wrote:
> On Fri, Aug 08, 2025 at 08:10:17PM +0200, Danilo Krummrich wrote:
>> +/// An [`Iterator`] of [`page::BorrowedPage`] items owned by a [`Vmalloc`] allocation.
>> +///
>> +/// # Guarantees
>> +///
>> +/// The pages iterated by the [`Iterator`] appear in the order as they are mapped in the CPU's
>> +/// virtual address space ascendingly.
>> +///
>> +/// # Invariants
>> +///
>> +/// - `buf` is a valid pointer to the beginning of a [`Vmalloc`] allocation.
>> +/// - `size` is the size of the [`Vmalloc`] allocation `buf` points to.
>> +pub struct VmallocPageIter<'a> {
>> +    /// The base address of the [`Vmalloc`] buffer.
>> +    buf: NonNull<u8>,
>> +    /// The size of the buffer pointed to by `buf` in bytes.
>> +    size: usize,
>> +    /// The current page index of the [`Iterator`].
>> +    index: usize,
>
> My understanding is that an implementation using two pointers that
> repeatedly increments the starting pointer until it reaches the end
> pointer is more efficient.

If I directly advance the start pointer until hitting the end pointer, I need
three pointers, i.e. start, end and the advancing one, otherwise I can't
implement base_address() anymore, hence I went with an index instead.

Re: [PATCH v3 4/7] rust: page: define trait IntoPageIter

[Danilo Krummrich]

On 8/11/25 11:16 AM, Alice Ryhl wrote:
> On Fri, Aug 08, 2025 at 08:10:18PM +0200, Danilo Krummrich wrote:
>> The IntoPageIter trait provides a common interface for types that
>> provide a page iterator, such as VmallocPageIter.
>>
>> Subsequent patches will leverage this to let VBox and VVec provide a
>> VmallocPageIter though this trait.
>>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>>   rust/kernel/page.rs | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
>> index 631718a6ad7d..bfb5898ab4b5 100644
>> --- a/rust/kernel/page.rs
>> +++ b/rust/kernel/page.rs
>> @@ -103,6 +103,16 @@ fn deref(&self) -> &Self::Target {
>>       }
>>   }
>>   
>> +/// Trait to be implemented by types, which provide an [`Iterator`] implementation of
>> +/// [`BorrowedPage`] items, such as [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
>> +pub trait IntoPageIter<'a> {
>> +    /// The [`Iterator`] type, e.g. [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
>> +    type Iter: Iterator<Item = BorrowedPage<'a>>;
> 
> Oh, also, I would put the lifetime on Iter.
> 
> trait AsPageIter {
>      type Iter<'a>: Iterator<Item = BorrowedPage<'a>>;
>      fn page_iter(&mut self) -> Self::Iter<'_>;
> }
Makes sense, but I think it needs

+    type Iter<'a>: Iterator<Item = BorrowedPage<'a>>
+    where
+        Self: 'a;

then.

Re: [PATCH v3 4/7] rust: page: define trait IntoPageIter

[Alice Ryhl]

On Mon, Aug 11, 2025 at 12:38 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On 8/11/25 11:16 AM, Alice Ryhl wrote:
> > On Fri, Aug 08, 2025 at 08:10:18PM +0200, Danilo Krummrich wrote:
> >> The IntoPageIter trait provides a common interface for types that
> >> provide a page iterator, such as VmallocPageIter.
> >>
> >> Subsequent patches will leverage this to let VBox and VVec provide a
> >> VmallocPageIter though this trait.
> >>
> >> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> >> ---
> >>   rust/kernel/page.rs | 10 ++++++++++
> >>   1 file changed, 10 insertions(+)
> >>
> >> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> >> index 631718a6ad7d..bfb5898ab4b5 100644
> >> --- a/rust/kernel/page.rs
> >> +++ b/rust/kernel/page.rs
> >> @@ -103,6 +103,16 @@ fn deref(&self) -> &Self::Target {
> >>       }
> >>   }
> >>
> >> +/// Trait to be implemented by types, which provide an [`Iterator`] implementation of
> >> +/// [`BorrowedPage`] items, such as [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
> >> +pub trait IntoPageIter<'a> {
> >> +    /// The [`Iterator`] type, e.g. [`VmallocPageIter`](kernel::alloc::allocator::VmallocPageIter).
> >> +    type Iter: Iterator<Item = BorrowedPage<'a>>;
> >
> > Oh, also, I would put the lifetime on Iter.
> >
> > trait AsPageIter {
> >      type Iter<'a>: Iterator<Item = BorrowedPage<'a>>;
> >      fn page_iter(&mut self) -> Self::Iter<'_>;
> > }
> Makes sense, but I think it needs
>
> +    type Iter<'a>: Iterator<Item = BorrowedPage<'a>>
> +    where
> +        Self: 'a;

Yeah probably.

Alice

Re: [PATCH v3 6/7] rust: alloc: layout: implement ArrayLayout::size()

[Daniel Almeida]

> On 8 Aug 2025, at 15:10, Danilo Krummrich <dakr@kernel.org> wrote:
> 
> Provide a convenience method for ArrayLayout to calculate the size of
> the ArrayLayout in bytes.
> 
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> rust/kernel/alloc/layout.rs | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/rust/kernel/alloc/layout.rs b/rust/kernel/alloc/layout.rs
> index 93ed514f7cc7..5c3db8e9f930 100644
> --- a/rust/kernel/alloc/layout.rs
> +++ b/rust/kernel/alloc/layout.rs
> @@ -98,6 +98,11 @@ pub const fn len(&self) -> usize {
>     pub const fn is_empty(&self) -> bool {
>         self.len == 0
>     }
> +
> +    /// Returns the size of the [`ArrayLayout`] in bytes.
> +    pub const fn size(&self) -> usize {
> +        self.len() * size_of::<T>()
> +    }
> }
> 
> impl<T> From<ArrayLayout<T>> for Layout {
> -- 
> 2.50.1
> 
> 

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>