[PATCH v2 3/5] rust: id_pool: do not supply starting capacity

Alice Ryhl posted 5 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 3/5] rust: id_pool: do not supply starting capacity
Posted by Alice Ryhl 3 months, 2 weeks ago
When creating the initial IdPool, Rust Binder simply wants the largest
value that does not allocate. Having to handle allocating error failures
that cannot happen is inconvenient, so make the constructor infallible
by removing the size argument.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/id_pool.rs | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
index a41a3404213ca92d53b14c80101afff6ac8c416e..126e57f34c3407cb1dab3169417f01917e172dee 100644
--- a/rust/kernel/id_pool.rs
+++ b/rust/kernel/id_pool.rs
@@ -96,16 +96,11 @@ pub fn realloc(&self, flags: Flags) -> Result<PoolResizer, AllocError> {
 
 impl IdPool {
     /// Constructs a new [`IdPool`].
-    ///
-    /// A capacity below [`BITS_PER_LONG`] is adjusted to
-    /// [`BITS_PER_LONG`].
-    ///
-    /// [`BITS_PER_LONG`]: srctree/include/asm-generic/bitsperlong.h
     #[inline]
-    pub fn new(num_ids: usize, flags: Flags) -> Result<Self, AllocError> {
-        let num_ids = core::cmp::max(num_ids, BITS_PER_LONG);
-        let map = BitmapVec::new(num_ids, flags)?;
-        Ok(Self { map })
+    pub fn new() -> Self {
+        Self {
+            map: BitmapVec::new_small(),
+        }
     }
 
     /// Returns how many IDs this pool can currently have.
@@ -224,3 +219,10 @@ pub fn release_id(&mut self, id: usize) {
         self.map.clear_bit(id);
     }
 }
+
+impl Default for IdPool {
+    #[inline]
+    fn default() -> Self {
+        Self::new()
+    }
+}

-- 
2.51.0.869.ge66316f041-goog
Re: [PATCH v2 3/5] rust: id_pool: do not supply starting capacity
Posted by Yury Norov 3 months, 2 weeks ago
On Tue, Oct 21, 2025 at 01:32:45PM +0000, Alice Ryhl wrote:
> When creating the initial IdPool, Rust Binder simply wants the largest
> value that does not allocate. Having to handle allocating error failures

That "value that does not allocate" wording is pretty confusing.
Maybe:
        Rust binder is initially created with an arbitrary capacity
        such that the underlying bitmap is held inplace.

Regardless:

Acked-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>

> that cannot happen is inconvenient, so make the constructor infallible
> by removing the size argument.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/id_pool.rs | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
> index a41a3404213ca92d53b14c80101afff6ac8c416e..126e57f34c3407cb1dab3169417f01917e172dee 100644
> --- a/rust/kernel/id_pool.rs
> +++ b/rust/kernel/id_pool.rs
> @@ -96,16 +96,11 @@ pub fn realloc(&self, flags: Flags) -> Result<PoolResizer, AllocError> {
>  
>  impl IdPool {
>      /// Constructs a new [`IdPool`].
> -    ///
> -    /// A capacity below [`BITS_PER_LONG`] is adjusted to
> -    /// [`BITS_PER_LONG`].
> -    ///
> -    /// [`BITS_PER_LONG`]: srctree/include/asm-generic/bitsperlong.h
>      #[inline]
> -    pub fn new(num_ids: usize, flags: Flags) -> Result<Self, AllocError> {
> -        let num_ids = core::cmp::max(num_ids, BITS_PER_LONG);
> -        let map = BitmapVec::new(num_ids, flags)?;
> -        Ok(Self { map })
> +    pub fn new() -> Self {
> +        Self {
> +            map: BitmapVec::new_small(),
> +        }
>      }
>  
>      /// Returns how many IDs this pool can currently have.
> @@ -224,3 +219,10 @@ pub fn release_id(&mut self, id: usize) {
>          self.map.clear_bit(id);
>      }
>  }
> +
> +impl Default for IdPool {
> +    #[inline]
> +    fn default() -> Self {
> +        Self::new()
> +    }
> +}
> 
> -- 
> 2.51.0.869.ge66316f041-goog
Re: [PATCH v2 3/5] rust: id_pool: do not supply starting capacity
Posted by Alice Ryhl 3 months, 2 weeks ago
On Thu, Oct 23, 2025 at 01:37:05PM -0400, Yury Norov wrote:
> On Tue, Oct 21, 2025 at 01:32:45PM +0000, Alice Ryhl wrote:
> > When creating the initial IdPool, Rust Binder simply wants the largest
> > value that does not allocate. Having to handle allocating error failures
> 
> That "value that does not allocate" wording is pretty confusing.
> Maybe:
>         Rust binder is initially created with an arbitrary capacity
>         such that the underlying bitmap is held inplace.
> 
> Regardless:
> 
> Acked-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>

How about this?

Rust Binder wants to use inline bitmaps whenever possible to avoid
allocations, so introduce a constructor for an IdPool with arbitrary
capacity that stores the bitmap inline.

Alice
Re: [PATCH v2 3/5] rust: id_pool: do not supply starting capacity
Posted by Burak Emir 3 months, 2 weeks ago
On Tue, Oct 21, 2025 at 3:33 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> When creating the initial IdPool, Rust Binder simply wants the largest
> value that does not allocate. Having to handle allocating error failures
> that cannot happen is inconvenient, so make the constructor infallible
> by removing the size argument.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Burak Emir <bqe@google.com>

Since Binder is the one use case we made this abstraction is for, it
makes sense to me.

cheers,
Burak