Re: [PATCH v8 11/12] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sun Aug 30, 2026 at 2:37 AM JST, Yury Norov wrote:
> On Thu, Aug 27, 2026 at 04:28:39PM +0900, 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`.
>
> It wasn't possible previously for a reason: allocating 0-bit bitmap is
> something questionable.

We had this discussion on the previous revision [1] [2] and the
conclusion was that it should be supported. Let me elaborate a bit on
why.

We tend to use `NonZero`, `Bounded` and other limiting types in order to
guarantee that invalid values that would otherwise require runtime error
checking cannot be expressed. The main benefit being that it removes
error paths and simplifies the code by removing footguns.

Here disallowing 0-bit bitmaps doesn't give us that benefit, it just
adds some burden on the user in that they need to build a `NonZero`. But
a 0-bit bitmap is not intrinsically invalid - it's actually a reasonable
starting point for a user that eventually wants to grow it dynamically
now that `grow_request` can support it.

That being said, you are correct when you say below that
CONFIG_RUST_BITMAP_HARDENED will make any query panic, but the problem
is that the documentation of the panicking methods does not mention that
behavior (although [3] addresses that). It is also not specific to a
length of `0`: if the bitmap had a length of, say, `4` and the user
called `bitmap.next_zero_bit(4)`, then it will panic just the same.
There is nothing special about a size of `0` in that regard.

[1] https://lore.kernel.org/rust-for-linux/DKUHJF1YTTY6.1XXC1L2SYXEMF@nvidia.com/
[2] https://lore.kernel.org/rust-for-linux/ao2U1jNaT7waibJW@google.com/
[3] https://lore.kernel.org/all/20260828133543.2259029-1-georgeandrout13@gmail.com/

>  
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  rust/kernel/id_pool.rs | 38 ++++++++++++++++++++++++++++++++------
>>  1 file changed, 32 insertions(+), 6 deletions(-)
>> 
>> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
>> index 06a4c71c4c6c..4f329249df9d 100644
>> --- a/rust/kernel/id_pool.rs
>> +++ b/rust/kernel/id_pool.rs
>> @@ -112,13 +112,8 @@ pub fn new() -> Self {
>>      }
>>  
>>      /// Constructs a new [`IdPool`] with space for a specific number of bits.
>> -    ///
>> -    /// A capacity below [`MAX_INLINE_LEN`] is adjusted to [`MAX_INLINE_LEN`].
>> -    ///
>> -    /// [`MAX_INLINE_LEN`]: BitmapVec::MAX_INLINE_LEN
>>      #[inline]
>>      pub fn with_capacity(num_ids: usize, flags: Flags) -> Result<Self, AllocError> {
>> -        let num_ids = usize::max(num_ids, BitmapVec::MAX_INLINE_LEN);
>>          let map = BitmapVec::new(num_ids, flags)?;
>>          Ok(Self { map })
>>      }
>> @@ -152,6 +147,13 @@ pub fn capacity(&self) -> usize {
>>      /// let resizer = alloc_request.realloc(GFP_KERNEL)?;
>>      /// pool.shrink(resizer);
>>      /// assert_eq!(pool.capacity(), BitmapVec::MAX_INLINE_LEN);
>> +    ///
>> +    /// // A pool at the `MAX_INLINE_LEN` floor cannot shrink further.
>> +    /// assert!(pool.shrink_request().is_none());
>> +    ///
>> +    /// // Neither can a pool with a capacity below `MAX_INLINE_LEN`.
>> +    /// let small = IdPool::with_capacity(8, GFP_KERNEL)?;
>> +    /// assert!(small.shrink_request().is_none());
>>      /// # Ok::<(), AllocError>(())
>>      /// ```
>>      #[inline]
>> @@ -198,12 +200,36 @@ pub fn shrink(&mut self, mut resizer: PoolResizer) {
>>  
>>      /// 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(0, GFP_KERNEL)?;
>
> Please don't add explicit examples for creating ID pools with 0
> capacity. It's a factual error, and should not be explicitly expressed
> in documentation.

I tend to agree that starting with a small capacity > 0 is probably a
better illustration of how to use the API.

>
> Also, it looks like your 0-bit bitmamp would trigger bitmap assertion:
>
>  IdPool::find_unused_id(0)
>    -> Bitmap::next_zero_bit()
>      -> assert!(start < self.len())
>        -> assert!(0 < 0)
>          -> panic if CONFIG_RUST_BITMAP_HARDENED=y
>
> I like your version because it allows to create an arbitrary capacity
> for ID pool, i.e. 4 bits. Right now one can explicitly create ID pool
> for 4 IDs, and allocate up to MAX_INLINE_LEN from it.
>
> But 0-bit ID pools must be prohibited.

I really think [4] we should consider removing
CONFIG_RUST_BITMAP_HARDENED, make the conditions for panicking clear in
the methods documentation (set/clear bit do panic, other methods don't),
and add checked variants to allow users to write more idiomatic code.
Bitmaps are really just arrays of bits, we should make them work as
close as possible to regular Rust arrays.

[4] https://lore.kernel.org/rust-for-linux/DKUKM1RPFDA7.1MBU5EY40OJ4J@nvidia.com/