Message ID | 20240911225449.152928-5-dakr@kernel.org (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Generic `Allocator` support for Rust | expand |
On 12.09.24 00:52, Danilo Krummrich wrote: > +/// # Invariants > +/// > +/// One of the following `krealloc`, `vrealloc`, `kvrealloc`. > +struct ReallocFunc( > + unsafe extern "C" fn(*const core::ffi::c_void, usize, u32) -> *mut core::ffi::c_void, > +); > + > +impl ReallocFunc { > + // INVARIANT: `krealloc` satisfies the type invariants. > + const KREALLOC: Self = Self(bindings::krealloc); > + > + /// # Safety > + /// > + /// This method has the same safety requirements as [`Allocator::realloc`]. > + /// > + /// # Guarantees > + /// > + /// This method has the same guarantees as `Allocator::realloc`. Additionally > + /// - it accepts any pointer to a valid memory allocation allocated by this function. > + /// - memory allocated by this function remains valid until it is passed to this function. > + unsafe fn call( > + &self, > + ptr: Option<NonNull<u8>>, > + layout: Layout, > + flags: Flags, > + ) -> Result<NonNull<[u8]>, AllocError> { > + let size = aligned_size(layout); > + let ptr = match ptr { > + Some(ptr) => ptr.as_ptr(), > + None => ptr::null(), > + }; > + > + // SAFETY: > + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc` and thus only requires that > + // `ptr` is NULL or valid. > + // - `ptr` is either NULL or valid by the safety requirements of this function. > + // > + // GUARANTEE: > + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc`. > + // - Those functions provide the guarantees of this function. > + let raw_ptr = unsafe { > + // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed. > + self.0(ptr.cast(), size, flags.0).cast() > + }; > + > + let ptr = if size == 0 { > + NonNull::dangling() > + } else { > + NonNull::new(raw_ptr).ok_or(AllocError)? > + }; > + > + Ok(NonNull::slice_from_raw_parts(ptr, size)) > + } > +} I remember asking you to split this into a different commit. I think you argued that it would be better to keep it in the same commit when bisecting. I don't think that applies in this case, are there any other disadvantages? --- Cheers, Benno > + > +// 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, > +// - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same. > +unsafe impl Allocator for Kmalloc { > + #[inline] > + unsafe fn realloc( > + ptr: Option<NonNull<u8>>, > + layout: Layout, > + flags: Flags, > + ) -> Result<NonNull<[u8]>, AllocError> { > + // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`. > + unsafe { ReallocFunc::KREALLOC.call(ptr, layout, flags) } > + } > +} > + > unsafe impl GlobalAlloc for Kmalloc { > unsafe fn alloc(&self, layout: Layout) -> *mut u8 { > // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero size by the function safety > -- > 2.46.0 >
On Thu, Sep 26, 2024 at 01:00:58PM +0000, Benno Lossin wrote: > On 12.09.24 00:52, Danilo Krummrich wrote: > > +/// # Invariants > > +/// > > +/// One of the following `krealloc`, `vrealloc`, `kvrealloc`. > > +struct ReallocFunc( > > + unsafe extern "C" fn(*const core::ffi::c_void, usize, u32) -> *mut core::ffi::c_void, > > +); > > + > > +impl ReallocFunc { > > + // INVARIANT: `krealloc` satisfies the type invariants. > > + const KREALLOC: Self = Self(bindings::krealloc); > > + > > + /// # Safety > > + /// > > + /// This method has the same safety requirements as [`Allocator::realloc`]. > > + /// > > + /// # Guarantees > > + /// > > + /// This method has the same guarantees as `Allocator::realloc`. Additionally > > + /// - it accepts any pointer to a valid memory allocation allocated by this function. > > + /// - memory allocated by this function remains valid until it is passed to this function. > > + unsafe fn call( > > + &self, > > + ptr: Option<NonNull<u8>>, > > + layout: Layout, > > + flags: Flags, > > + ) -> Result<NonNull<[u8]>, AllocError> { > > + let size = aligned_size(layout); > > + let ptr = match ptr { > > + Some(ptr) => ptr.as_ptr(), > > + None => ptr::null(), > > + }; > > + > > + // SAFETY: > > + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc` and thus only requires that > > + // `ptr` is NULL or valid. > > + // - `ptr` is either NULL or valid by the safety requirements of this function. > > + // > > + // GUARANTEE: > > + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc`. > > + // - Those functions provide the guarantees of this function. > > + let raw_ptr = unsafe { > > + // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed. > > + self.0(ptr.cast(), size, flags.0).cast() > > + }; > > + > > + let ptr = if size == 0 { > > + NonNull::dangling() > > + } else { > > + NonNull::new(raw_ptr).ok_or(AllocError)? > > + }; > > + > > + Ok(NonNull::slice_from_raw_parts(ptr, size)) > > + } > > +} > > I remember asking you to split this into a different commit. I think you > argued that it would be better to keep it in the same commit when > bisecting. I don't think that applies in this case, are there any other > disadvantages? I don't really like the intermediate `#[expect(dead_code)]`, plus it's additional work you didn't really give me a motivation for, i.e. you did not mention what would be the advantage. But sure, I will change it for the next version. > > --- > Cheers, > Benno > > > + > > +// 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, > > +// - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same. > > +unsafe impl Allocator for Kmalloc { > > + #[inline] > > + unsafe fn realloc( > > + ptr: Option<NonNull<u8>>, > > + layout: Layout, > > + flags: Flags, > > + ) -> Result<NonNull<[u8]>, AllocError> { > > + // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`. > > + unsafe { ReallocFunc::KREALLOC.call(ptr, layout, flags) } > > + } > > +} > > > > + > > unsafe impl GlobalAlloc for Kmalloc { > > unsafe fn alloc(&self, layout: Layout) -> *mut u8 { > > // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero size by the function safety > > -- > > 2.46.0 > > >
On 26.09.24 15:24, Danilo Krummrich wrote: > On Thu, Sep 26, 2024 at 01:00:58PM +0000, Benno Lossin wrote: >> On 12.09.24 00:52, Danilo Krummrich wrote: >>> +/// # Invariants >>> +/// >>> +/// One of the following `krealloc`, `vrealloc`, `kvrealloc`. >>> +struct ReallocFunc( >>> + unsafe extern "C" fn(*const core::ffi::c_void, usize, u32) -> *mut core::ffi::c_void, >>> +); >>> + >>> +impl ReallocFunc { >>> + // INVARIANT: `krealloc` satisfies the type invariants. >>> + const KREALLOC: Self = Self(bindings::krealloc); >>> + >>> + /// # Safety >>> + /// >>> + /// This method has the same safety requirements as [`Allocator::realloc`]. >>> + /// >>> + /// # Guarantees >>> + /// >>> + /// This method has the same guarantees as `Allocator::realloc`. Additionally >>> + /// - it accepts any pointer to a valid memory allocation allocated by this function. >>> + /// - memory allocated by this function remains valid until it is passed to this function. >>> + unsafe fn call( >>> + &self, >>> + ptr: Option<NonNull<u8>>, >>> + layout: Layout, >>> + flags: Flags, >>> + ) -> Result<NonNull<[u8]>, AllocError> { >>> + let size = aligned_size(layout); >>> + let ptr = match ptr { >>> + Some(ptr) => ptr.as_ptr(), >>> + None => ptr::null(), >>> + }; >>> + >>> + // SAFETY: >>> + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc` and thus only requires that >>> + // `ptr` is NULL or valid. >>> + // - `ptr` is either NULL or valid by the safety requirements of this function. >>> + // >>> + // GUARANTEE: >>> + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc`. >>> + // - Those functions provide the guarantees of this function. >>> + let raw_ptr = unsafe { >>> + // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed. >>> + self.0(ptr.cast(), size, flags.0).cast() >>> + }; >>> + >>> + let ptr = if size == 0 { >>> + NonNull::dangling() >>> + } else { >>> + NonNull::new(raw_ptr).ok_or(AllocError)? >>> + }; >>> + >>> + Ok(NonNull::slice_from_raw_parts(ptr, size)) >>> + } >>> +} >> >> I remember asking you to split this into a different commit. I think you >> argued that it would be better to keep it in the same commit when >> bisecting. I don't think that applies in this case, are there any other >> disadvantages? > > I don't really like the intermediate `#[expect(dead_code)]`, plus it's > additional work you didn't really give me a motivation for, i.e. you did not > mention what would be the advantage. The advantage would be that it's easier to review (granted it probably is a bit late for that). I got confused a couple of times (but that's probably on me). > But sure, I will change it for the next version. Thanks --- Cheers, Benno
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs index 6c21bd2edad9..535ed1527034 100644 --- a/rust/kernel/alloc.rs +++ b/rust/kernel/alloc.rs @@ -4,7 +4,7 @@ #[cfg(not(test))] #[cfg(not(testlib))] -mod allocator; +pub mod allocator; pub mod box_ext; pub mod vec_ext; diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs index e32182f91167..7a93442187a1 100644 --- a/rust/kernel/alloc/allocator.rs +++ b/rust/kernel/alloc/allocator.rs @@ -1,12 +1,28 @@ // SPDX-License-Identifier: GPL-2.0 //! Allocator support. +//! +//! Documentation for the kernel's memory allocators can found in the "Memory Allocation Guide" +//! linked below. For instance, this includes the concept of "get free page" (GFP) flags and the +//! typical application of the different kernel allocators. +//! +//! Reference: <https://docs.kernel.org/core-api/memory-allocation.html> use super::{flags::*, Flags}; use core::alloc::{GlobalAlloc, Layout}; use core::ptr; +use core::ptr::NonNull; -struct Kmalloc; +use crate::alloc::{AllocError, Allocator}; +use crate::bindings; + +/// The contiguous kernel allocator. +/// +/// `Kmalloc` is typically used for physically contiguous allocations up to page size, but also +/// supports larger allocations up to `bindings::KMALLOC_MAX_SIZE`, which is hardware specific. +/// +/// For more details see [self]. +pub struct Kmalloc; /// Returns a proper size to alloc a new object aligned to `new_layout`'s alignment. fn aligned_size(new_layout: Layout) -> usize { @@ -36,6 +52,77 @@ pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F unsafe { bindings::krealloc(ptr as *const core::ffi::c_void, size, flags.0) as *mut u8 } } +/// # Invariants +/// +/// One of the following `krealloc`, `vrealloc`, `kvrealloc`. +struct ReallocFunc( + unsafe extern "C" fn(*const core::ffi::c_void, usize, u32) -> *mut core::ffi::c_void, +); + +impl ReallocFunc { + // INVARIANT: `krealloc` satisfies the type invariants. + const KREALLOC: Self = Self(bindings::krealloc); + + /// # Safety + /// + /// This method has the same safety requirements as [`Allocator::realloc`]. + /// + /// # Guarantees + /// + /// This method has the same guarantees as `Allocator::realloc`. Additionally + /// - it accepts any pointer to a valid memory allocation allocated by this function. + /// - memory allocated by this function remains valid until it is passed to this function. + unsafe fn call( + &self, + ptr: Option<NonNull<u8>>, + layout: Layout, + flags: Flags, + ) -> Result<NonNull<[u8]>, AllocError> { + let size = aligned_size(layout); + let ptr = match ptr { + Some(ptr) => ptr.as_ptr(), + None => ptr::null(), + }; + + // SAFETY: + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc` and thus only requires that + // `ptr` is NULL or valid. + // - `ptr` is either NULL or valid by the safety requirements of this function. + // + // GUARANTEE: + // - `self.0` is one of `krealloc`, `vrealloc`, `kvrealloc`. + // - Those functions provide the guarantees of this function. + let raw_ptr = unsafe { + // If `size == 0` and `ptr != NULL` the memory behind the pointer is freed. + self.0(ptr.cast(), size, flags.0).cast() + }; + + let ptr = if size == 0 { + NonNull::dangling() + } else { + NonNull::new(raw_ptr).ok_or(AllocError)? + }; + + Ok(NonNull::slice_from_raw_parts(ptr, size)) + } +} + +// 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, +// - `realloc` satisfies the guarantees, since `ReallocFunc::call` has the same. +unsafe impl Allocator for Kmalloc { + #[inline] + unsafe fn realloc( + ptr: Option<NonNull<u8>>, + layout: Layout, + flags: Flags, + ) -> Result<NonNull<[u8]>, AllocError> { + // SAFETY: `ReallocFunc::call` has the same safety requirements as `Allocator::realloc`. + unsafe { ReallocFunc::KREALLOC.call(ptr, layout, flags) } + } +} + unsafe impl GlobalAlloc for Kmalloc { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { // SAFETY: `ptr::null_mut()` is null and `layout` has a non-zero size by the function safety
Implement `Allocator` for `Kmalloc`, the kernel's default allocator, typically used for objects smaller than page size. All memory allocations made with `Kmalloc` end up in `krealloc()`. It serves as allocator for the subsequently introduced types `KBox` and `KVec`. Signed-off-by: Danilo Krummrich <dakr@kernel.org> --- rust/kernel/alloc.rs | 2 +- rust/kernel/alloc/allocator.rs | 89 +++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 2 deletions(-)