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

[Alexandre Courbot]

On Tue Oct 28, 2025 at 7:55 PM JST, Alice Ryhl wrote:
> 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.
>
> The existing constructor could be renamed to with_capacity() to match
> constructors for other similar types, but it is removed as there is
> currently no user for it.
>
> Acked-by: Yury Norov (NVIDIA) <yury.norov@xxxxxxxxx>
> Reviewed-by: Burak Emir <bqe@xxxxxxxxxx>
> Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx>
> ---
>  rust/kernel/id_pool.rs | 46 ++++++++++++++++++----------------------------
>  1 file changed, 18 insertions(+), 28 deletions(-)
>
> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
> index a41a3404213ca92d53b14c80101afff6ac8c416e..d53628a357ed84a6e00ef9dfd03a75e85a87532c 100644
> --- a/rust/kernel/id_pool.rs
> +++ b/rust/kernel/id_pool.rs
> @@ -28,19 +28,21 @@
>  /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
>  /// use kernel::id_pool::IdPool;
>  ///
> -/// let mut pool = IdPool::new(64, GFP_KERNEL)?;
> -/// for i in 0..64 {
> +/// let mut pool = IdPool::new();
> +/// let cap = pool.capacity();
> +///
> +/// for i in 0..cap {
>  ///     assert_eq!(i, pool.acquire_next_id(i).ok_or(ENOSPC)?);
>  /// }
>  ///
> -/// pool.release_id(23);
> -/// assert_eq!(23, pool.acquire_next_id(0).ok_or(ENOSPC)?);
> +/// pool.release_id(5);
> +/// assert_eq!(5, pool.acquire_next_id(0).ok_or(ENOSPC)?);
>  ///
>  /// assert_eq!(None, pool.acquire_next_id(0));  // time to realloc.
>  /// let resizer = pool.grow_request().ok_or(ENOSPC)?.realloc(GFP_KERNEL)?;
>  /// pool.grow(resizer);
>  ///
> -/// assert_eq!(pool.acquire_next_id(0), Some(64));
> +/// assert_eq!(pool.acquire_next_id(0), Some(cap));
>  /// # Ok::<(), Error>(())
>  /// ```
>  ///
> @@ -96,16 +98,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

A word about the capacity of this pool would be helpful here IMHO -
without anything one could assume it is 0, which is not the case.

>      #[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_inline(),
> +        }
>      }
>  
>      /// Returns how many IDs this pool can currently have.
> @@ -119,20 +116,6 @@ pub fn capacity(&self) -> usize {
>      /// The capacity of an [`IdPool`] cannot be shrunk below [`BITS_PER_LONG`].
>      ///
>      /// [`BITS_PER_LONG`]: srctree/include/asm-generic/bitsperlong.h
> -    ///
> -    /// # Examples
> -    ///
> -    /// ```
> -    /// use kernel::alloc::{AllocError, flags::GFP_KERNEL};
> -    /// use kernel::id_pool::{ReallocRequest, IdPool};
> -    ///
> -    /// let mut pool = IdPool::new(1024, GFP_KERNEL)?;
> -    /// let alloc_request = pool.shrink_request().ok_or(AllocError)?;
> -    /// let resizer = alloc_request.realloc(GFP_KERNEL)?;
> -    /// pool.shrink(resizer);
> -    /// assert_eq!(pool.capacity(), kernel::bindings::BITS_PER_LONG as usize);
> -    /// # Ok::<(), AllocError>(())

Is this test deleted because the old constructor has been removed? If so
I'd say that justifies keeping it around, having tests is valuable in
itself and the example is quite helpful to understand how the
not-so-obvious resizing process works.