Re: [PATCH 1/6] rust: alloc: add Vec::push_init
"Eliot Courtney" <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Mon Aug 17, 2026 at 11:02 PM JST, Gary Guo wrote: > On Mon Aug 17, 2026 at 1:56 PM BST, Eliot Courtney wrote: >> Add `Vec::push_init` which initializes a new element in place. We can't >> modify the existing `Vec::push` signature to take an `impl Init<T, E>` >> without changing its Error type. >> >> Signed-off-by: Eliot Courtney <[email protected]> >> --- >> rust/kernel/alloc/kvec.rs | 42 +++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs >> index c7546b9da4fa..9f6f25d7e218 100644 >> --- a/rust/kernel/alloc/kvec.rs >> +++ b/rust/kernel/alloc/kvec.rs >> @@ -52,7 +52,10 @@ >> }, // >> }; >> >> -use pin_init::Zeroable; >> +use pin_init::{ >> + Init, >> + Zeroable, // >> +}; >> >> mod errors; >> pub use self::errors::{InsertError, PushError, RemoveError}; >> @@ -359,6 +362,43 @@ pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { >> Ok(()) >> } >> >> + /// Appends an element to the back of the [`Vec`] instance by initializing it in place. >> + /// >> + /// # Examples >> + /// >> + /// ``` >> + /// struct Element { >> + /// buf: KVec<u8>, >> + /// } >> + /// >> + /// impl Element { >> + /// fn new() -> impl Init<Self, Error> { >> + /// try_init!(Element { >> + /// buf: KVec::with_capacity(16, GFP_KERNEL)?, >> + /// }? Error) >> + /// } >> + /// } >> + /// >> + /// let mut v: KVec<Element> = KVec::new(); >> + /// v.push_init(Element::new(), GFP_KERNEL)?; >> + /// assert!(v[0].buf.is_empty()); >> + /// # Ok::<(), Error>(()) >> + /// ``` >> + pub fn push_init<E>(&mut self, init: impl Init<T, E>, flags: Flags) -> Result<(), E> >> + where >> + E: From<AllocError>, >> + { >> + self.reserve(1, flags)?; >> + // SAFETY: The call to `reserve` was successful, so there is at least one spare slot; the >> + // pointer therefore refers to allocated, aligned memory valid for a write of one `T`. >> + unsafe { init.__init(self.spare_capacity_mut().as_mut_ptr().cast::<T>())? }; >> + // SAFETY: The call to `__init` returned `Ok`, so the first spare slot now holds an >> + // initialized `T`. The new length does not exceed the capacity because `reserve` ensured >> + // the capacity is greater than the length by at least one. >> + unsafe { self.inc_len(1) }; >> + Ok(()) >> + } > > Thinking about this from a fresh design perspective, I wonder if we can create > something more composable by splitting the allocation and insertion, like entry > APIs do. > > So > > impl<T, A: Allocator> Vec<T, A> { > pub fn reserve(&mut self, additional: usize, flags: Flags) -> Result<Reservation<'_, T>, AllocError> { > ... > } > } > > /// Type indicating vector with reserved capacity. > pub struct<'a> Reservation<'a, T> { > } > > impl<'a, T> Reservation<'a, T> { > pub fn init(&mut self, i: impl Init<T, E>) -> Result<(), E> { > ... > } > } > > You can imagine even pushing this further, e.g. have a type indicating just a > single reserved slot. Or perhaps have a type that is `Vec` but with fixed > capacity and cannot reallocate (something like `ArrayVec`) that the reserve > method will return. > > Best, > Gary Yeah that's an interesting idea. FWIW, I think the *_init idea has precedent already (Box::new, Box::init, etc.). I tried implementing something like this idea though, below. Since we don't have const generic exprs I used Peano-arithmetic like types. But if we only care about empty vs at least one empty capacity, we could use a boolean. The idea is to track the minimum extra capacity and provide a generalised set of vector operations that would work regardless of the underlying storage or allocator (well basically what you said I hope). Since it knows how much guaranteed spare capacity it has we can do a bunch of stuff infallibly and track the guaranteed spare capacity. If there's no guaranteed spare capacity then it will be fallible (but non-allocating). KVec then becomes a wrapper over these vector ops that just ensures it has enough guaranteed spare capacity (possibly allocating) before forwarding. Allocation/storage related ops remain on KVec. Then an ArrayVec and KVec can share most vector operations on a VecView. There's also some locations in other code that could use a VecView directly instead of taking a &mut Vec etc. Having a Vec-like thing that's guaranteed not to allocate also sounds potentially useful to me w.r.t. safety for contexts where you can't allocate/sleep. If you think this approach is ok I can send it as a separate series. Codegen appears fine practically speaking AFAICT. Using it looks kinda like: ``` let view = v.reserved::<Two>(GFP_KERNEL)?; let Ok(view) = view.push(1); view.push(Element::new())?; // only init can fail, push is guaranteed let mut view = v.view(); while view.push(0).is_ok() {} // can still do fallible stuff view.pop(); // ArrayVec shares vec-like ops. can add method forwarders if we want arrayvec.view().push(1)?; ``` WDYT? (subset of code demonstrating the idea follows): ``` mod sealed { pub trait Sealed {} impl Sealed for () {} impl<N: super::Count> Sealed for (N,) {} } /// Peano-like nested tuple type machinery. pub trait Count: sealed::Sealed { const COUNT: usize; } impl Count for () { const COUNT: usize = 0; } impl<N: Count> Count for (N,) { const COUNT: usize = 1 + N::COUNT; } pub type Zero = (); pub type Succ<N> = (N,); pub type One = Succ<Zero>; pub type Two = Succ<One>; /// A view of vector-like storage with `N` slots of guaranteed spare capacity. #[repr(C)] pub struct VecView<'a, T, N: Count = Zero> { buf: NonNull<T>, len: &'a mut usize, cap: usize, marker: PhantomData<(&'a mut [T], N)>, // Invariant to prevent stashing shorter refs etc. } impl<'a, T, N: Count> VecView<'a, T, N> { pub unsafe fn from_raw_parts(buf: NonNull<T>, len: &'a mut usize, cap: usize) -> Self { Self { buf, len, cap, marker: PhantomData, } } } // Infallible (except for Init) push. `Zero` spare VecView has the fallible version. impl<'a, T, N: Count> VecView<'a, T, Succ<N>> { pub fn push<E>(self, init: impl Init<T, E>) -> Result<VecView<'a, T, N>, E> { unsafe { init.__init(self.buf.as_ptr().add(*self.len))? }; *self.len += 1; Ok(VecView { buf: self.buf, len: self.len, cap: self.cap, marker: PhantomData, }) } } // Fallible but not allocating ops (can run out of space). impl<'a, T> VecView<'a, T> { pub fn push<I: Init<T, E>, E>(&mut self, init: I) -> Result<(), PushInitError<I, E>> { if *self.len == self.cap { return Err(PushInitError::Full(init)); } unsafe { init.__init(self.buf.as_ptr().add(*self.len)) }.map_err(PushInitError::Init)?; *self.len += 1; Ok(()) } // Bodies as in the current KVec implementations. pub fn pop(&mut self) -> Option<T> { ... } pub fn insert(&mut self, index: usize, element: T) -> Result<(), InsertError<T>> { ... } pub fn remove(&mut self, i: usize) -> Result<T, RemoveError> { ... } pub fn truncate(&mut self, len: usize) { ... } pub fn retain(&mut self, f: impl FnMut(&mut T) -> bool) { ... } pub fn drain_all(self) -> DrainAll<'a, T> { ... } pub fn len(&self) -> usize { ... } pub fn as_slice(&self) -> &[T] { ... } pub fn as_mut_slice(&mut self) -> &mut [T] { ... } pub fn spare_capacity(&self) -> usize { ... } pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] { ... } pub unsafe fn commit(&mut self, additional: usize) { ... } } impl<T: Clone> VecView<'_, T> { pub fn extend_with(&mut self, n: usize, value: T) -> Result<(), Error> { ... } pub fn extend_from_slice(&mut self, other: &[T]) -> Result<(), Error> { ... } } // Decay to read only ops. impl<'a, T, N: Count> Deref for VecView<'a, T, Succ<N>> { type Target = VecView<'a, T>; fn deref(&self) -> &Self::Target { unsafe { &*ptr::from_ref(self).cast() } } } pub enum PushInitError<I, E> { Full(I), Init(E), } impl<I, E: Into<Error>> From<PushInitError<I, E>> for Error { fn from(e: PushInitError<I, E>) -> Error { match e { PushInitError::Full(_) => EINVAL, PushInitError::Init(e) => e.into(), } } } // `reserved` gets you the guaranteed capacity VecView. impl<T, A: Allocator> Vec<T, A> { pub fn view(&mut self) -> VecView<'_, T> { let buf = self.ptr; let cap = self.capacity(); unsafe { VecView::from_raw_parts(buf, &mut self.len, cap) } } pub fn reserved<N: Count>(&mut self, flags: Flags) -> Result<VecView<'_, T, N>, AllocError> { self.reserve(N::COUNT, flags)?; let buf = self.ptr; let cap = self.capacity(); Ok(unsafe { VecView::from_raw_parts(buf, &mut self.len, cap) }) } pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> { let Ok(_) = self.reserved::<One>(flags)?.push(v); Ok(()) } } // Non allocating ArrayVec backing. pub struct ArrayVec<T, const N: usize> { buf: [MaybeUninit<T>; N], len: usize, } impl<T, const N: usize> ArrayVec<T, N> { pub fn view(&mut self) -> VecView<'_, T> { let buf = NonNull::from(&mut self.buf).cast::<T>(); unsafe { VecView::from_raw_parts(buf, &mut self.len, N) } } pub fn reserved<C: Count>(&mut self) -> Option<VecView<'_, T, C>> { const { assert!(C::COUNT <= N) } if C::COUNT > N - self.len { return None; } let buf = NonNull::from(&mut self.buf).cast::<T>(); Some(unsafe { VecView::from_raw_parts(buf, &mut self.len, N) }) } } ```