Re: [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool

On Thu, Aug 13, 2026 at 10:48:50PM +0200, Danilo Krummrich wrote:
> On Thu Aug 13, 2026 at 8:32 PM CEST, Yury Norov wrote:
> >   pub(crate) fn alloc_area(
> >       &self,
> >       count: usize,
> >       align: usize,
> >   ) -> Result<ChannelIdArea<'_>> {
> >       let count = NonZero::new(count).ok_or(EINVAL)?;
> >       let align = Alignment::new_checked(align).ok_or(EINVAL)?;
> >
> >       let mut ids = self.inner.lock();
> >       let area = ids.find_unused_area(0, count, align).ok_or(ENOSPC)?;
> >
> >       // If the pool is small, the backing bitmap may be rounded up to a larger size.
> >       if area.range().end > self.num_chids {
> >           return Err(ENOSPC);
> >       }
> >
> >       Ok(ChannelIdArea {
> >           pool: self,
> >           range: area.acquire(),
> >       })
> >   }
> >   
> >   let area = pool.alloc_area(8, 4)?;
> >
> > See the difference? You still check the parameters, but don't make it
> > the part of interface.
> 
> Miguel already replied to this, so just briefly adding to this.
> 
> We usually want the arguments to already carry the invariants we require. If we
> make the arguments unconstrained, we may end up in situations where we already
> have types that provide certain guarantees about value constraints and yet we
> have to give up on them because the API takes unconstrained arguments and
> revalidates.
> 
> The code from Eliot does actually already takes advantage of this. In
> 
> 	pool.alloc_area(nz::<8>(), Alignment::new::<8>())?;
> 
> both arguments are already validated at compile time, whereas with unconstrained
> arguments we're left with a runtime check.
> 
> Yes, nz() does not actually validate it statically, but it easily could (and
> probably should).

Alright, I'm not against the static checks, and I don't insist on my
version. My complain is about readability and unnecessary complexity
for end user.

let's find a way to convert this beast:

 	pool.alloc_area(nz::<8>(), alignment::new::<8>())?;

to something more readable, ideally:

 	pool.alloc_area(8, 8)?;

We were able to do this for bitfields, and I don't think we should
give up here.