From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Burak Emir" <burak.emir@gmail.com>
Cc: "Yury Norov" <ynorov@nvidia.com>,
"Eliot Courtney" <ecourtney@nvidia.com>,
"Alice Ryhl" <aliceryhl@google.com>,
"Yury Norov" <yury.norov@gmail.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Trevor Gross" <tmgross@umich.edu>,
"Danilo Krummrich" <dakr@kernel.org>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Tamir Duberstein" <tamird@kernel.org>,
"Onur Özkan" <work@onurozkan.dev>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"John Hubbard" <jhubbard@nvidia.com>,
"Alistair Popple" <apopple@nvidia.com>,
"Timur Tabi" <ttabi@nvidia.com>, "Zhi Wang" <zhiw@nvidia.com>,
rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
nova-gpu@lists.linux.dev, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v9 8/9] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN
Date: Wed, 14 Oct 2026 11:04:46 +0900 [thread overview]
Message-ID: <DM46Y905AJVV.1K0F8AK1ZP4IO@nvidia.com> (raw)
In-Reply-To: <CABwQupMVzUOsqP_fTZQdwUU+xEO1k2Q4=RQQuq2qd5HCYY0ZUg@mail.gmail.com>
Hi Burak,
On Sat Oct 10, 2026 at 3:48 AM JST, Burak Emir wrote:
> On Fri, Oct 9, 2026 at 4:07 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> On Fri Oct 9, 2026 at 12:47 AM JST, Yury Norov wrote:
>
>> > If you still think that you need 0-length bitmaps in Rust, can you please
>> > give the clear and thorough explanation why rust needs those 0-length
>> > bitmaps. Are there any in-kernel examples? Any language concepts requiring
>> > it? If not, it's still a NAK.
>>
>> I don't know of an in-kernel example, but please look at the
>> `bitmap_vec_new` test which has been here since the API was initially
>> merged last year: the first thing it does is create a zero-sized bitmap.
>
> Well, yeah: I wanted the behavior that the implementation rounds up
> documented so it can be recognized as intentional.
> Later, the `new_inline` method was added, which is a clearer way to
> get the largest non-allocating BitmapVec.
>
> I think the problem in this discussion is that we want to treat
> capacity like length, which is maybe suggested by the BitmapVec name
> and its len method.
>
> With the minimum capacity behavior documented, it is as if we have a
> specification right now that no IdPool instances can ever have less
> capacity than BitmapVec::MAX_INLINE_LEN bits. This patch here proposes
> to change that.
>
> It is not clear though why client code would be happy about *less capacity*?
>
> In other words, why should the client of the IdPool have the right to
> control the *exact* initial capacity, even if it is below documented,
> free-of-charge minimum capacity?
The ID pool is used in Nova to track the number of hardware channels, of
which we have an exact, fixed number. For this use-case rounding up is
not "free-of-charge", it just allows to allocate IDs that are invalid as
there is no hardware resource to back them up.
>
> The IdPool, despite being backed by a BitmapVec, is not meant to be
> used like a vector: it does not grow by itself but client code calls
> grow and shrink explicitly, and client code has no control over how
> much that growing and shrinking happens.
>
> Like, you cannot grow to 65 and expect to get a capacity 65. So why
> insist on exact small sizes?
Actually with the current code you can call `IdPool::with_capacity(65)`
and you will get exactly 65 IDs. And if you call grow the new capacity
will be `65 * 2`.
The point being that in the current code the initial capacity is exact
for every value *except* those below `MAX_INLINE_LEN`, which doesn't
come across as consistent. All this patch does is remove that exception
for which I don't see a justification.
I think we are conflating capacity with length to some degree. Actually
the code does exactly that:
impl IdPool {
pub fn capacity(&self) -> usize {
self.map.len()
}
}
>
>> It is also easy to imagine a user starting with an empty pool and
>> growing it on-demand. Having the ability to create a zero-sized pool is
>> convenient to avoid special-casing user code, and in this case I'd say
>> expected, just like you can create a zero-sized vector. Again a size of
>> zero does not trigger anything that a larger capacity cannot trigger, so
>> I don't see a reason to forbid it.
>
> With an initial capacity of minimum MAX_INLINE_LEN capacity, it is not
> about forbidding, it is that the added capacity already comes for
> free.
> You could say "not enabling" instead of "forbidding", but pretending
> that the capacity is somehow not there does not seem to buy us
> anything.
The capacity is not necessarily there when allocating IDs for finite
resources. I suspect that's where the misunderstanding stems from, as
Nova is using the ID pool for a new use-case. But I think we can agree
that this use-case is not particularly specific and matches what one can
expect from an ID allocator, so it makes sense to support it, especially
as it doesn't affect other users (`with_capacity` has no caller in the
tree at the moment).
>
> Do the other patches in this series need this, or can we just drop this patch?
> If they do, maybe a demonstration of how significant the special-case
> handling is that can be avoided with the exact small capacity could be
> part of the commit message?
The next patch in this series needs this to implement `ChannelIdPool`.
The alternative would be to track the actual length of the pool and
return an error if `IdPool` returns an ID after that value, which is
what v5 [1] was doing (see `num_chids` in the v5 code).
That's the special-case handling that this patch is designed to avoid,
and I hope we can agree that it isn't a satisfying workaround,
especially when the backing `BitmapVec` *does* support exact sizes and
that artificial limitation is introduced by `IdPool` itself.
[1] https://lore.kernel.org/all/20260812-chid-v5-5-6c767770b3f4@nvidia.com/
next prev parent reply other threads:[~2026-10-14 2:04 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-30 2:42 [PATCH v9 0/9] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-09-30 2:42 ` [PATCH v9 1/9] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-09-30 2:42 ` [PATCH v9 2/9] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-09-30 2:42 ` [PATCH v9 3/9] rust: sizes: add sub-1K size constants Eliot Courtney
2026-09-30 9:12 ` Miguel Ojeda
2026-09-30 11:04 ` Alexandre Courbot
2026-09-30 11:22 ` Miguel Ojeda
2026-10-01 0:40 ` Gary Guo
2026-10-01 5:23 ` Eliot Courtney
2026-09-30 2:42 ` [PATCH v9 4/9] rust: sizes: implement SizeConstants for Alignment Eliot Courtney
2026-09-30 9:12 ` Miguel Ojeda
2026-09-30 2:42 ` [PATCH v9 5/9] rust: use Alignment size constants Eliot Courtney
2026-09-30 2:42 ` [PATCH v9 6/9] rust: bitmap: add contiguous area operations Eliot Courtney
2026-09-30 4:41 ` Yury Norov
2026-09-30 2:42 ` [PATCH v9 7/9] rust: id_pool: add contiguous ID reservation Eliot Courtney
2026-09-30 2:51 ` sashiko-bot
2026-09-30 4:50 ` Yury Norov
2026-09-30 2:42 ` [PATCH v9 8/9] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN Eliot Courtney
2026-09-30 2:52 ` sashiko-bot
2026-09-30 5:05 ` Yury Norov
2026-10-01 6:20 ` Alexandre Courbot
2026-10-08 15:47 ` Yury Norov
2026-10-08 16:04 ` Gary Guo
2026-10-09 14:07 ` Alexandre Courbot
2026-10-09 18:48 ` Burak Emir
2026-10-14 2:04 ` Alexandre Courbot [this message]
2026-10-14 7:20 ` Burak Emir
2026-09-30 2:42 ` [PATCH v9 9/9] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-10-08 15:04 ` [PATCH v9 0/9] rust: Add support for reserving of ranges of IDs Alexandre Courbot
2026-10-11 9:15 ` Miguel Ojeda
2026-10-13 13:26 ` Miguel Ojeda
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM46Y905AJVV.1K0F8AK1ZP4IO@nvidia.com \
--to=acourbot@nvidia.com \
--cc=a.hindborg@kernel.org \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=burak.emir@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=ecourtney@nvidia.com \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=nova-gpu@lists.linux.dev \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=tamird@kernel.org \
--cc=tmgross@umich.edu \
--cc=ttabi@nvidia.com \
--cc=work@onurozkan.dev \
--cc=ynorov@nvidia.com \
--cc=yury.norov@gmail.com \
--cc=zhiw@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.