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

On Mon, Aug 17, 2026 at 04:03:51PM +0900, Eliot Courtney wrote:
> On Fri Aug 14, 2026 at 11:53 PM JST, Yury Norov wrote:
> > On Fri, Aug 14, 2026 at 05:08:43AM -0400, Yury Norov wrote:
> >> On Fri, Aug 14, 2026 at 01:54:24PM +0900, Eliot Courtney wrote:
> >> > > 1. Provide a nz! macro to create constant non-zero things. Provide some
> >> > >    constants for common alignments
> >> > >
> >> > > Potentially, we could use build_assert! for cases it is provably
> >> > > non-zero but not a literal (although tbh not sure this is a good idea -
> >> > > at least not do this initially). 
> >> > >
> >> > > So either: nz!: const { NonZero::new(value).unwrap() }, or, nz!: some
> >> > > build assert gated construction of NonZero. A general nz! macro could
> >> > > also help ergonomics for other use cases too.
> >> > >
> >> > > That looks like this:
> >> > >
> >> > > pool.alloc_area(nz!(8), Alignment::AL_8)?;
> >> > 
> >> > Actually found that if we implement SizeConstants for Alignment then we
> >> > can get Alignment::SZ_4K etc almost for free.
> >> 
> >> Then maybe pass SZ_4K directly, and allow the alloc_area() to
> >> calculate the alignment:
> >> 
> >>         pool.alloc_area(nz!(8), SZ_4K)?;
> >
> > Even worse, you can advertise the function like: allocate a contiguous
> > set of channel IDs enough to represent XXX bytes of memory with YYY
> > alignment. And that way, you'll be able to do:
> >
> >           pool.alloc_area(SZ_8K, SZ_4K)?;
> >
> > If my understanding of the function is right, it would be a clear
> > improvement, because the user now doesn't think about ID channels pool
> > as a set of bits, but an abstraction over the memory access technique,
> > which it really is.
> 
> If we pass SZ_4K directly, we are back to using plain integers, which is
> not what we should be doing for rust interfaces according to Miguel. Let
> me send a new version of this with my Alignment constants + nz! macro
> idea, IMO it helps with the verbosity.

I meant that if you have 8-bit alignment and granularity, you can
create your own enum and pass it as parameter. But it's valid for
pre-blackwell channel allocations only, so not applicable in general.
 
> I don't quite understand your point about memory access.

Neither me. Scratch that. It was a hallucination, maybe somehow
related to the 1st day of flu. :)

> The purpose
> here is just to be an allocator of IDs, not memory - it's just that we
> have some contiguousness and alignment requirements of the IDs
> themselves.  The values we're passing in just represent numbers of
> channels and constraints on those. I think that callers will naturally
> want to allocate a particular number of channels, not a specific amount
> of memory that corresponds to some number of channels in some way (e.g.
> USERD?). On pre-Blackwell, OpenRM makes channel ID area reservations for
> vGPU power of two number of channels and and places them so the base
> channel ID is aligned to the number of channels. Sorry if I have
> misunderstood your point. Maybe we should rename `alloc_area` to
> `reserve_ids` or something like that if it would make it clearer?

Yes, reserve_ids() sounds better because the parent structure is named
ChannelIdPool.