This introduces a new method called `push_within_capacity` for appending
to a vector without attempting to allocate if the capacity is full. Rust
Binder will use this in various places to safely push to a vector while
holding a spinlock.

The implementation is moved to a push_within_capacity_unchecked method.
This is preferred over having push() call push_within_capacity()
followed by an unwrap_unchecked() for simpler unsafe.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/alloc/kvec.rs | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index ebca0cfd31c67f3ce13c4825d7039e34bb54f4d4..a005a295262cb1e8b7c118125ffa07ae252e257c 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -307,17 +307,52 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
     /// ```
     pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
         self.reserve(1, flags)?;
+        // SAFETY: The call to `reserve` was successful, so the capacity is at least one greater
+        // than the length.
+        unsafe { self.push_within_capacity_unchecked(v) };
+        Ok(())
+    }
+
+    /// Appends an element to the back of the [`Vec`] instance without reallocating.
+    ///
+    /// Fails if the vector does not have capacity for the new element.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let mut v = KVec::with_capacity(10, GFP_KERNEL);
+    /// for i in 0..10 {
+    ///     v.push_within_capacity(i).unwrap();
+    /// }
+    ///
+    /// assert!(v.push_within_capacity(10).is_err());
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn push_within_capacity(&mut self, v: T) -> Result<(), T> {
+        if self.len() < self.capacity() {
+            // SAFETY: The length is less than the capacity.
+            unsafe { self.push_within_capacity_unchecked(v) };
+            Ok(())
+        } else {
+            Err(v)
+        }
+    }
 
+    /// Appends an element to the back of the [`Vec`] instance without reallocating.
+    ///
+    /// # Safety
+    ///
+    /// The length must be less than the capacity.
+    pub unsafe fn push_within_capacity_unchecked(&mut self, v: T) {
         let spare = self.spare_capacity_mut();
 
         // SAFETY: The call to `reserve` was successful so the spare capacity is at least 1.
         unsafe { spare.get_unchecked_mut(0) }.write(v);
 
         // SAFETY: We just initialised the first spare entry, so it is safe to increase the length
-        // by 1. We also know that the new length is <= capacity because of the previous call to
-        // `reserve` above.
+        // by 1. We also know that the new length is <= capacity because the caller guarantees that
+        // the length is less than the capacity at the beginning of this function.
         unsafe { self.inc_len(1) };
-        Ok(())
     }
 
     /// Removes the last element from a vector and returns it, or `None` if it is empty.

-- 
2.49.0.805.g082f7c87e0-goog

On Tue, Apr 22, 2025 at 5:53 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> This introduces a new method called `push_within_capacity` for appending
> to a vector without attempting to allocate if the capacity is full. Rust
> Binder will use this in various places to safely push to a vector while
> holding a spinlock.
>
> The implementation is moved to a push_within_capacity_unchecked method.
> This is preferred over having push() call push_within_capacity()
> followed by an unwrap_unchecked() for simpler unsafe.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/alloc/kvec.rs | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index ebca0cfd31c67f3ce13c4825d7039e34bb54f4d4..a005a295262cb1e8b7c118125ffa07ae252e257c 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -307,17 +307,52 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
>      /// ```
>      pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
>          self.reserve(1, flags)?;
> +        // SAFETY: The call to `reserve` was successful, so the capacity is at least one greater
> +        // than the length.
> +        unsafe { self.push_within_capacity_unchecked(v) };
> +        Ok(())
> +    }
> +
> +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> +    ///
> +    /// Fails if the vector does not have capacity for the new element.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// let mut v = KVec::with_capacity(10, GFP_KERNEL);
> +    /// for i in 0..10 {
> +    ///     v.push_within_capacity(i).unwrap();
> +    /// }
> +    ///
> +    /// assert!(v.push_within_capacity(10).is_err());
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    pub fn push_within_capacity(&mut self, v: T) -> Result<(), T> {
> +        if self.len() < self.capacity() {
> +            // SAFETY: The length is less than the capacity.
> +            unsafe { self.push_within_capacity_unchecked(v) };
> +            Ok(())
> +        } else {
> +            Err(v)
> +        }
> +    }
>
> +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The length must be less than the capacity.
> +    pub unsafe fn push_within_capacity_unchecked(&mut self, v: T) {

Did you intend for this to be pub? The commit message doesn't
obviously indicate it.

>          let spare = self.spare_capacity_mut();
>
>          // SAFETY: The call to `reserve` was successful so the spare capacity is at least 1.

What call to reserve?

>          unsafe { spare.get_unchecked_mut(0) }.write(v);
>
>          // SAFETY: We just initialised the first spare entry, so it is safe to increase the length
> -        // by 1. We also know that the new length is <= capacity because of the previous call to
> -        // `reserve` above.
> +        // by 1. We also know that the new length is <= capacity because the caller guarantees that
> +        // the length is less than the capacity at the beginning of this function.
>          unsafe { self.inc_len(1) };
> -        Ok(())
>      }
>
>      /// Removes the last element from a vector and returns it, or `None` if it is empty.
>
> --
> 2.49.0.805.g082f7c87e0-goog
>
>

On Wed, Apr 23, 2025 at 11:38:28AM -0400, Tamir Duberstein wrote:
> On Tue, Apr 22, 2025 at 5:53 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > This introduces a new method called `push_within_capacity` for appending
> > to a vector without attempting to allocate if the capacity is full. Rust
> > Binder will use this in various places to safely push to a vector while
> > holding a spinlock.
> >
> > The implementation is moved to a push_within_capacity_unchecked method.
> > This is preferred over having push() call push_within_capacity()
> > followed by an unwrap_unchecked() for simpler unsafe.
> >
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// The length must be less than the capacity.
> > +    pub unsafe fn push_within_capacity_unchecked(&mut self, v: T) {
> 
> Did you intend for this to be pub? The commit message doesn't
> obviously indicate it.

Well, I don't think it hurts.

> >          let spare = self.spare_capacity_mut();
> >
> >          // SAFETY: The call to `reserve` was successful so the spare capacity is at least 1.
> 
> What call to reserve?

I have to update this comment, thanks.

> >          unsafe { spare.get_unchecked_mut(0) }.write(v);
> >
> >          // SAFETY: We just initialised the first spare entry, so it is safe to increase the length
> > -        // by 1. We also know that the new length is <= capacity because of the previous call to
> > -        // `reserve` above.
> > +        // by 1. We also know that the new length is <= capacity because the caller guarantees that
> > +        // the length is less than the capacity at the beginning of this function.
> >          unsafe { self.inc_len(1) };
> > -        Ok(())
> >      }

Alice

On Tue, Apr 22, 2025 at 09:52:18AM +0000, Alice Ryhl wrote:
> This introduces a new method called `push_within_capacity` for appending
> to a vector without attempting to allocate if the capacity is full. Rust
> Binder will use this in various places to safely push to a vector while
> holding a spinlock.
> 
> The implementation is moved to a push_within_capacity_unchecked method.
> This is preferred over having push() call push_within_capacity()
> followed by an unwrap_unchecked() for simpler unsafe.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/alloc/kvec.rs | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index ebca0cfd31c67f3ce13c4825d7039e34bb54f4d4..a005a295262cb1e8b7c118125ffa07ae252e257c 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -307,17 +307,52 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
>      /// ```
>      pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
>          self.reserve(1, flags)?;
> +        // SAFETY: The call to `reserve` was successful, so the capacity is at least one greater
> +        // than the length.
> +        unsafe { self.push_within_capacity_unchecked(v) };
> +        Ok(())
> +    }
> +
> +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> +    ///
> +    /// Fails if the vector does not have capacity for the new element.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// let mut v = KVec::with_capacity(10, GFP_KERNEL);

Should be:

    /// let mut v = KVec::with_capacity(10, GFP_KERNEL)?;

, right? I.e. a question mark is missing.

The rest looks good to me.

Regards,
Boqun

> +    /// for i in 0..10 {
> +    ///     v.push_within_capacity(i).unwrap();
> +    /// }
> +    ///
> +    /// assert!(v.push_within_capacity(10).is_err());
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    pub fn push_within_capacity(&mut self, v: T) -> Result<(), T> {
> +        if self.len() < self.capacity() {
> +            // SAFETY: The length is less than the capacity.
> +            unsafe { self.push_within_capacity_unchecked(v) };
> +            Ok(())
> +        } else {
> +            Err(v)
> +        }
> +    }
>  
> +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The length must be less than the capacity.
> +    pub unsafe fn push_within_capacity_unchecked(&mut self, v: T) {
>          let spare = self.spare_capacity_mut();
>  
>          // SAFETY: The call to `reserve` was successful so the spare capacity is at least 1.
>          unsafe { spare.get_unchecked_mut(0) }.write(v);
>  
>          // SAFETY: We just initialised the first spare entry, so it is safe to increase the length
> -        // by 1. We also know that the new length is <= capacity because of the previous call to
> -        // `reserve` above.
> +        // by 1. We also know that the new length is <= capacity because the caller guarantees that
> +        // the length is less than the capacity at the beginning of this function.
>          unsafe { self.inc_len(1) };
> -        Ok(())
>      }
>  
>      /// Removes the last element from a vector and returns it, or `None` if it is empty.
> 
> -- 
> 2.49.0.805.g082f7c87e0-goog
> 
>

On Tue, Apr 22, 2025 at 02:29:53PM -0700, Boqun Feng wrote:
> On Tue, Apr 22, 2025 at 09:52:18AM +0000, Alice Ryhl wrote:
> > This introduces a new method called `push_within_capacity` for appending
> > to a vector without attempting to allocate if the capacity is full. Rust
> > Binder will use this in various places to safely push to a vector while
> > holding a spinlock.
> > 
> > The implementation is moved to a push_within_capacity_unchecked method.
> > This is preferred over having push() call push_within_capacity()
> > followed by an unwrap_unchecked() for simpler unsafe.
> > 
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/kernel/alloc/kvec.rs | 41 ++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 38 insertions(+), 3 deletions(-)
> > 
> > diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> > index ebca0cfd31c67f3ce13c4825d7039e34bb54f4d4..a005a295262cb1e8b7c118125ffa07ae252e257c 100644
> > --- a/rust/kernel/alloc/kvec.rs
> > +++ b/rust/kernel/alloc/kvec.rs
> > @@ -307,17 +307,52 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
> >      /// ```
> >      pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
> >          self.reserve(1, flags)?;
> > +        // SAFETY: The call to `reserve` was successful, so the capacity is at least one greater
> > +        // than the length.
> > +        unsafe { self.push_within_capacity_unchecked(v) };
> > +        Ok(())
> > +    }
> > +
> > +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> > +    ///
> > +    /// Fails if the vector does not have capacity for the new element.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// let mut v = KVec::with_capacity(10, GFP_KERNEL);
> 
> Should be:
> 
>     /// let mut v = KVec::with_capacity(10, GFP_KERNEL)?;
> 
> , right? I.e. a question mark is missing.
> 
> The rest looks good to me.

Will be fixed in the next version. Let me know if you want me to add
your Reviewed-by tag with this fixed?

Alice

On Wed, Apr 23, 2025 at 08:55:34AM +0000, Alice Ryhl wrote:
> On Tue, Apr 22, 2025 at 02:29:53PM -0700, Boqun Feng wrote:
> > On Tue, Apr 22, 2025 at 09:52:18AM +0000, Alice Ryhl wrote:
> > > This introduces a new method called `push_within_capacity` for appending
> > > to a vector without attempting to allocate if the capacity is full. Rust
> > > Binder will use this in various places to safely push to a vector while
> > > holding a spinlock.
> > > 
> > > The implementation is moved to a push_within_capacity_unchecked method.
> > > This is preferred over having push() call push_within_capacity()
> > > followed by an unwrap_unchecked() for simpler unsafe.
> > > 
> > > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > > ---
> > >  rust/kernel/alloc/kvec.rs | 41 ++++++++++++++++++++++++++++++++++++++---
> > >  1 file changed, 38 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> > > index ebca0cfd31c67f3ce13c4825d7039e34bb54f4d4..a005a295262cb1e8b7c118125ffa07ae252e257c 100644
> > > --- a/rust/kernel/alloc/kvec.rs
> > > +++ b/rust/kernel/alloc/kvec.rs
> > > @@ -307,17 +307,52 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
> > >      /// ```
> > >      pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
> > >          self.reserve(1, flags)?;
> > > +        // SAFETY: The call to `reserve` was successful, so the capacity is at least one greater
> > > +        // than the length.
> > > +        unsafe { self.push_within_capacity_unchecked(v) };
> > > +        Ok(())
> > > +    }
> > > +
> > > +    /// Appends an element to the back of the [`Vec`] instance without reallocating.
> > > +    ///
> > > +    /// Fails if the vector does not have capacity for the new element.
> > > +    ///
> > > +    /// # Examples
> > > +    ///
> > > +    /// ```
> > > +    /// let mut v = KVec::with_capacity(10, GFP_KERNEL);
> > 
> > Should be:
> > 
> >     /// let mut v = KVec::with_capacity(10, GFP_KERNEL)?;
> > 
> > , right? I.e. a question mark is missing.
> > 
> > The rest looks good to me.
> 
> Will be fixed in the next version. Let me know if you want me to add
> your Reviewed-by tag with this fixed?
> 

Sure, feel free to add

Reviewed-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> Alice