Messages in this thread | | | Date | Wed, 27 Mar 2024 17:09:47 +0000 | From | Benno Lossin <> | Subject | Re: [PATCH v2 06/10] rust: alloc: introduce the `BoxExt` trait |
| |
On 27.03.24 03:35, Wedson Almeida Filho wrote: > diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocatorrs > index 01ad139e19bc..fc0439455faa 100644 > --- a/rust/kernel/alloc/allocator.rs > +++ b/rust/kernel/alloc/allocator.rs > @@ -15,7 +15,11 @@ > /// > /// - `ptr` can be either null or a pointer which has been allocated by this allocator. > /// - `new_layout` must have a non-zero size. > -unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: bindings::gfp_t) -> *mut u8 { > +pub(crate) unsafe fn krealloc_aligned( > + ptr: *mut u8, > + new_layout: Layout, > + flags: bindings::gfp_t,
I think it is a good idea to use `Flags` here as well.
> +) -> *mut u8 { > // Customized layouts from `Layout::from_size_align()` can have size < align, so pad first. > let layout = new_layout.pad_to_align(); > > diff --git a/rust/kernel/alloc/box_ext.rs b/rust/kernel/alloc/box_ext.rs > new file mode 100644 > index 000000000000..aaa6a20d12b6 > --- /dev/null > +++ b/rust/kernel/alloc/box_ext.rs > @@ -0,0 +1,62 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Extensions to [`Box`] for fallible allocations. > + > +use super::Flags; > +use alloc::boxed::Box; > +use core::alloc::AllocError; > +use core::mem::MaybeUninit; > +use core::result::Result; > + > +/// Extensions to [`Box`]. > +pub trait BoxExt<T>: Sized { > + /// Allocates a new box. > + /// > + /// The allocation may fail, in which case an error is returned. > + fn new(x: T, flags: Flags) -> Result<Self, AllocError>; > + > + /// Allocates a new uninitialised box. > + /// > + /// The allocation may fail, in which case an error is returned. > + fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>>, AllocError>; > +} > + > +impl<T> BoxExt<T> for Box<T> { > + #[cfg(any(test, testlib))] > + fn new(x: T, _flags: Flags) -> Result<Self, AllocError> { > + Ok(Box::new(x)) > + } > + > + #[cfg(not(any(test, testlib)))] > + fn new(x: T, flags: Flags) -> Result<Self, AllocError> { > + let ptr = if core::mem::size_of::<T>() == 0 { > + core::ptr::NonNull::<T>::dangling().as_ptr() > + } else { > + let layout = core::alloc::Layout::new::<T>(); > + > + // SAFETY: Memory is being allocated (first arg is null). The only other source of > + // safety issues is sleeping on atomic context, which is addressed by klint. Lastly, > + // the type is not a SZT (checked above). > + let ptr = unsafe { > + super::allocator::krealloc_aligned(core::ptr::null_mut(), layout, flags.0) > + }; > + if ptr.is_null() { > + return Err(AllocError); > + } > + > + let ptr = ptr.cast::<T>(); > + > + // SAFETY: We just allocated the memory above, it is valid for write. > + unsafe { ptr.write(x) }; > + ptr > + }; > + > + // SAFETY: For non-zero-sized types, we allocate above using the global allocator. For > + // zero-sized types, we use `NonNull::dangling`. > + Ok(unsafe { Box::from_raw(ptr) }) > + } > + > + fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>>, AllocError> { > + <Box<_> as BoxExt<_>>::new(MaybeUninit::<T>::uninit(), flags)
Note that the expression `MaybeUninit::<T>::uninit()` introduces a temporary which will be allocated on the stack. If the type `T` is very big, then this function will cause immediate stack overflows.
I would recommend to implement `new` in terms of `new_uninit`. The `new_uninit` implementation can be almost the same as the current implementation of `new`, just remove the `ptr.write(x)` statement.
-- Cheers, Benno
> + } > +}
| |