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

["Gary Guo" <gary@garyguo.net>]

On Mon Aug 17, 2026 at 1:16 PM BST, Danilo Krummrich wrote:
> On Mon Aug 17, 2026 at 1:49 PM CEST, Eliot Courtney wrote:
>> On Mon Aug 17, 2026 at 8:18 PM JST, Danilo Krummrich wrote:
>>> On Mon Aug 17, 2026 at 12:54 PM CEST, Gary Guo wrote:
>>>> IMO we can just support zero-sized allocation by always succeeding, similar to
>>>> how ZST allocation is handled in memory allocation.
>>>
>>> Probably, but in contrast to ZST allocations it wouldn't be very useful, so we'd
>>> silently succeed on something that never was a reasonable argument in the first
>>> palce.
>>
>> Yeah I agree that supporting a generalised zero sized alloc is in some
>> ways conceptually nicer, but I also can't see any good reason why you
>> would want to do it, it sounds like a bug. That's also why in the latest
>> version I made IdPool also not allow a zero capacity [1] (incidentally
>> required to not have to change the grow code to avoid trying to double a
>> zero capacity thing every time).
>
> (I assume this is more a reply to Gary?)
>
>> Tbh, I feel that NonZero (and Alignment) is hard to use currently
>> because of these ergonomics issues. But I reckon they're useful, so I
>> like the idea of trying to making them easier to use. Maybe you can make
>> an argument to not use NonZero here, but what about all the other times
>> we will want to use it?
>
> Personally, I don't see the argument for not using NonZero (or Alignment); the
> invariants required by the API are expressed very well by those types.
>
> I also don't see a reason to step back from using those types for this API. If
> we'd do so it would question all the other new types we have over primitives
> carrying certain invariants to a certain extend as well.

I think there is an distinction between semantic differences vs just range
restrictions. Consider the other cases where we use new types:

* flags: very natural as you don't even need to mention raw literals.

* units: for things that semantically carry a unit, writing things like
  `Delta::new_jiffies(...)` is quite natural, just like writing "2 seconds"

* addresses: things like dma addresses naturally fit in new types because you
  don't need to write literals for them.

Now, with `NonZero` or `Bounded`, we are doing none of that. The only thing here
is that there is a range restriction. Other than the value restriction
themselves, they carry no other semantic meanings. How you interpret these types
still fully depend on the API that accepts them. Therefore, it is very common
that you'd be using these with literals, and it becomes an ergnomic pain.

If Rust pattern types become available in the future and you can write

    fn alloc_area(&self, count: usize is 1..)

then I'd fully support adding that annotation to functions. Personally I value
ergnomics higher than possibility of misuse if latter can be easily mitigated
otherwise (in this case, by WARN_ON or just support zero-sized alloc).

Best,
Gary

>
> I understand the ergonomics concern, and I also see that for compile time
> evaluation turbofish syntax is not that popular, but this can be improved.
>
> The fundamental value remains that it nicely separates the invariants callers
> need to uphold about certain arguments from the API itself. It makes the code
> more maintainable (e.g. we don't need 10 different variants of a function
> depending on whether the value is a compile time, build time or run time value)
> and it makes the code more readable and robust (e.g. the semantic meaning of
> arguments is very obvious and confusing arguments becomes almost impossible).