Re: [PATCH v11 4/5] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN

["Eliot Courtney" <ecourtney@nvidia.com>]

On Mon Nov 9, 2026 at 9:36 PM JST, Gary Guo wrote:
> On Mon Nov 9, 2026 at 8:32 AM GMT, Eliot Courtney wrote:
>> Current code in IdPool::with_capacity rounds the capacity up to
>> BitmapVec::MAX_INLINE_LEN, but BitmapVec::new works fine with values
>> smaller than this and still uses an inline representation. Remove this
>> behaviour.
>>
>> This allows specifying a real capacity of 0, which was not previously
>> possible. This breaks `grow_request` in this case, so change it to grow
>> to at least `BitmapVec::MAX_INLINE_LEN`, mirroring the capacity floor in
>> `shrink_request`.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Burak Emir <burak.emir@gmail.com>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
[...]
>>      /// Returns a [`ReallocRequest`] for growing this [`IdPool`], if possible.
>>      ///
>> +    /// Grows to at least [`MAX_INLINE_LEN`].
>>      /// The capacity of an [`IdPool`] cannot be grown above [`MAX_LEN`].
>>      ///
>> +    /// [`MAX_INLINE_LEN`]: BitmapVec::MAX_INLINE_LEN
>>      /// [`MAX_LEN`]: BitmapVec::MAX_LEN
>> +    ///
>> +    /// # Examples
>> +    ///
>> +    /// ```
>> +    /// use kernel::{
>> +    ///     alloc::AllocError,
>> +    ///     bitmap::BitmapVec,
>> +    ///     id_pool::IdPool, //
>> +    /// };
>> +    ///
>> +    /// // Grow goes to at least BitmapVec::MAX_INLINE_LEN.
>> +    /// let mut pool = IdPool::with_capacity(1, GFP_KERNEL)?;
>> +    /// let resizer = pool.grow_request().ok_or(AllocError)?.realloc(GFP_KERNEL)?;
>> +    /// pool.grow(resizer);
>> +    /// assert_eq!(pool.capacity(), BitmapVec::MAX_INLINE_LEN);
>> +    ///
>> +    /// // Grow doubles if at least BitmapVec::MAX_INLINE_LEN.
>> +    /// let resizer = pool.grow_request().ok_or(AllocError)?.realloc(GFP_KERNEL)?;
>> +    /// pool.grow(resizer);
>> +    /// assert_eq!(pool.capacity(), 2 * BitmapVec::MAX_INLINE_LEN);
>> +    /// # Ok::<(), AllocError>(())
>> +    /// ```
>>      #[inline]
>>      pub fn grow_request(&self) -> Option<ReallocRequest> {
>> -        let num_ids = self.capacity() * 2;
>> +        let num_ids = usize::max(BitmapVec::MAX_INLINE_LEN, self.capacity() * 2);
>
> It looks like your want the `IdPool` have a controlled capacity. However, as
> soon as `grow_request` is used, the capacity becomes uncontrolled.
>
> It feels that this actually means that there are two use cases here: unbounded
> IdPool (where capacity just means the _current_ capacity) and bounded IdPool
> (where capacity means the _max_ capacity).
>
> This is more similar to a difference between `len()` and `capacity()`. It feels
> to me that you're merrging `Vec<bool>` and `Box<[bool]>` into a single type
> where it shouldn't.

Yerp I agree there are two use cases here and this is adding the
"fixed+controlled capacity" use case. But e.g. for `Vec`, we also have
this "double use case" with `push_within_capacity`. So you can use `Vec`
in both use cases as well. 

The analogy would be:
Vec::len <=> # of actually allocated ids
Vec::capacity <=> # of available or allocated ids
Vec::push_within_capacity <=> IdPool::find_unused_id / reserve_ids
Vec::reserve <=> IdPool::grow / IdPool::grow_request 
Vec::shrink_to <=> IdPool::shrink / IdPool::shrink_request

Vec::push equivalent doesn't exist because neither binder's use case
(needs to separate allocation) nor nova's use case (uses fixed capacity)
requires a combined "reserve and push_within_capacity".

That said with this patch `IdPool::with_capacity` provides a guarantee
of the capacity which `Vec::with_capacity` doesn't (altho I think it
could for non-ZSTs). So I agree that there is a conceptual difference
here w.r.t. "controlled" (guaranteed) capacity. The reason I did it this
way is that I think that providing this extra guarantee is easy to do
(removes code) without unduly complicating IdPool, plus adding
to/allowing reuse of the existing common API functions.

We could consider in a follow-up to introduce a typestate, like Fixed vs
Growable (defaulting to Growable) and grow/shrink is only implemented
for `Growable`. But IMO it's overcomplicated to do this - if you don't
want to grow it, just don't grow it. It's currently very hard to
accidentally grow IdPool (compared to Vec where it would be far easier
to mess it up if you tried to control the capacity). Perhaps if later
many more things are added to IdPool then it'll then be worth to split
it.

WDYT?