Re: [PATCH v5 4/5] rust: id_pool: add contiguous area allocation

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

On Thu Aug 13, 2026 at 6:16 AM JST, Yury Norov wrote:
> On Wed, Aug 12, 2026 at 05:51:24PM +0900, Eliot Courtney wrote:
>> Add support for contiguous area allocation. Add a new type,
>> `UnusedArea`, following the same pattern as `UnusedId`.
>> 
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  rust/kernel/id_pool.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>> 
>> diff --git a/rust/kernel/id_pool.rs b/rust/kernel/id_pool.rs
>> index 384753fe0e44..eb911a0e3217 100644
>> --- a/rust/kernel/id_pool.rs
>> +++ b/rust/kernel/id_pool.rs
>> @@ -4,8 +4,14 @@
>>  
>>  //! Rust API for an ID pool backed by a [`BitmapVec`].
>>  
>> +use core::{
>> +    num::NonZero,
>> +    ops::Range, //
>> +};
>> +
>>  use crate::alloc::{AllocError, Flags};
>>  use crate::bitmap::BitmapVec;
>> +use crate::ptr::Alignment;
>>  
>>  /// Represents a dynamic ID pool backed by a [`BitmapVec`].
>>  ///
>> @@ -240,6 +246,33 @@ pub fn find_unused_id(&mut self, offset: usize) -> Option<UnusedId<'_>> {
>>      pub fn release_id(&mut self, id: usize) {
>>          self.map.clear_bit(id);
>>      }
>> +
>> +    /// Finds a contiguous area of `count` unused IDs at or after `offset`.
>> +    ///
>> +    /// The start of the returned area is a multiple of `align`.
>> +    ///
>> +    /// Returns an [`UnusedArea`] upon success, or [`None`] if no such area could be found.
>> +    #[inline]
>> +    #[must_use]
>> +    pub fn find_unused_area(
>> +        &mut self,
>> +        offset: usize,
>> +        count: NonZero<usize>,
>> +        align: Alignment,
>> +    ) -> Option<UnusedArea<'_>> {
>> +        let start = self.map.next_zero_area(offset, count.get(), align)?;
>> +        // INVARIANT: `next_zero_area()` returns None or a start with `start + count <= map.len()`.
>> +        Some(UnusedArea {
>> +            range: start..start + count.get(),
>> +            pool: self,
>> +        })
>> +    }
>> +
>> +    /// Releases a contiguous area of IDs.
>> +    #[inline]
>> +    pub fn release_area(&mut self, range: &Range<usize>) {
>> +        self.map.clear(range.start, range.len());
>> +    }
>>  }
>>  
>>  /// Represents an unused id in an [`IdPool`].
>> @@ -287,6 +320,42 @@ pub fn acquire(self) -> usize {
>>      }
>>  }
>>  
>> +/// Represents an unused, contiguous area of IDs in an [`IdPool`].
>> +///
>> +/// # Invariants
>> +///
>> +/// `range.start <= range.end <= pool.map.len()`.
>> +#[must_use = "the ID range is not reserved unless acquired"]
>> +pub struct UnusedArea<'pool> {
>> +    range: Range<usize>,
>> +    pool: &'pool mut IdPool,
>> +}
>
> So, the compilation message refers the "ID range", not the UnusedArea.
> To me, this 'unused' language is confusing. What should I do with the
> area that I just allocated? Drop the 'unused' one and create the 'used'?
>
> Can you rename it to id_range please? Then the API would look more
> consistent, at least to me.

tl;dr: I will remove `UnusedArea` according to your suggestion

`UnusedArea` here mirrors the design of `UnusedId`, which first finds
the unused ID then lets you actually set it whenever you want (first
introduced in f523d110a63b ("rust: id_pool: do not immediately acquire
new ids")). According to f523d110a63b, the reason for this design is to
allow some fallible operations once you know the ID you are getting
before committing to actually allocating it. So `UnusedArea` just
follows the existing design for this API. But, we don't need this
intermediate fallible operation behaviour right now, so I think it's ok
to get rid of it.

>
>> +
>> +impl<'pool> UnusedArea<'pool> {
>> +    /// Returns the unused ID range.
>> +    ///
>> +    /// Be aware that the area has not yet been acquired in the pool. The
>> +    /// [`acquire`] method must be called to prevent others from taking it.
>> +    ///
>> +    /// [`acquire`]: UnusedArea::acquire()
>
> So maybe implement the find_acquire() method? In the caller you
> serialize it with:
>
>         let mut ids = self.inner.lock();
>
> Is it possible to pass this down to the suggested find_acquire()? In
> my experience, having non-atomic sequence of find + acquire that
> requires the external locking is the recipe for troubles.

In this case, since we stash a mutable reference to `IdPool` in
`UnusedArea` (just like `UnusedId`), it's not possible to hit any
non-atomic find/acquire issues. The compiler will statically prevent you
from being able to allocate anything else in the `IdPool` as long as the
`UnusedArea` is alive. That is, it's always valid to acquire an
`UnusedArea` (or `UnusedId`).  In this case IMO it is more flexible to
let the caller decide how it will do the locking (and that's the
existing design for allocating single IDs here), since it can e.g.
control the granularity. Or maybe it really exists in a single threaded
context and doesn't need locking whatsoever (i.e. can get a mutable ref
to IdPool without locking).

>
>> +    #[inline]
>> +    #[must_use]
>> +    pub fn range(&self) -> Range<usize> {
>> +        self.range.clone()
>> +    }
>> +
>> +    /// Acquires the area.
>> +    ///
>> +    /// Returns the now-reserved ID range.
>> +    #[inline]
>> +    pub fn acquire(self) -> Range<usize> {
>> +        let Self { range, pool } = self;
>> +        // By the type invariants, the range is within bounds.
>> +        pool.map.set(range.start, range.end - range.start);
>> +        range
>
> From hierarchy perspective, the UnusedArea wraps the Range, and
> passing the Range to the higher layer breaks the hierarchy. If you
> follow my suggestion, the hierarchy will be enforced stricter:
>
>         ChannelIdRange -> IdRange-> Range
>
> instead of  
>
>         ChannelIdArea -> UnusedArea-> Range
>                       |
>                       -> Range
>
> Or I misunderstand the concept of the UnusedArea?
>
>> +    }
>> +}
>> +
>>  impl Default for IdPool {
>>      #[inline]
>>      fn default() -> Self {
>> 
>> -- 
>> 2.55.0