All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Eliot Courtney <ecourtney@nvidia.com>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
	"Burak Emir" <burak.emir@gmail.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,
	dri-devel <dri-devel-bounces@lists.freedesktop.org>
Subject: Re: [PATCH v7 06/10] rust: id_pool: take a NonZero capacity in with_capacity
Date: Tue, 25 Aug 2026 13:12:54 +0000	[thread overview]
Message-ID: <ao2U1jNaT7waibJW@google.com> (raw)
In-Reply-To: <DKXZ7VHUCLWL.C6IPHWPTEJW4@nvidia.com>

On Tue, Aug 25, 2026 at 08:09:13PM +0900, Eliot Courtney wrote:
> On Fri Aug 21, 2026 at 5:39 PM JST, Alexandre Courbot wrote:
> > On Mon Aug 17, 2026 at 4:04 PM JST, Eliot Courtney wrote:
> >> There is no good reason to allocate an IdPool with zero capacity.
> >> Reflect this in IdPool::with_capacity.
> >>
> >> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> >
> > I am not sure this one is justifiable; `KVec::with_capacity(0)` is
> > doable, so why not here? As long as it doesn't introduce soundness
> > issues I'd say this is the caller's business; a driver with a legitimate
> > empty IdPool use-case would now need to special-case it.
> >
> > Now we do have an actual soundness issue with zero-sized IdPools, which
> > is that `find_unused_id` would panic with `CONFIG_RUST_BITMAP_HARDENED`,
> > but as I said on patch 5 I don't think it should anyway. Another
> > potential issue is that `grow_request` would not grow anything; but that
> > should be fixed there by handling the `capacity == 0` case. Actually
> > that would give justification for empty IdPools to exist: just like a
> > vector can start empty and grow, so can an IdPool.
> 
> I don't have a very strong opinion here but I can't really think of a
> use case for a zero capacity IdPool. Unlike an empty vector, since
> IdPool doesn't automatically grow (there is a notion of a fixed ID
> space), the only thing you can do with a zero capacity IdPool is grow it
> to non-zero. All the other operations don't do anything useful.

It may not grow automatically, but that's only because Binder (which
will grow its IdPool) holds it in a spinlock and needs to use the
PoolResizer and so on to grow it without allocating under said spinlock.

> If such a use case exists, maybe it'd have to be something like you are
> using the capacity to identify your ID space size (and the ID space size
> is important otherwise you would just use IdPool::new() with the
> MAX_INLINE_LEN capacity) but then the only way you can grow it is via
> grow_request() which doesn't grow the ID space in caller controllable
> way.
> 
> Anyway, let me know if you feel strongly about this one. FWIW, previous
> to this patch series you couldn't construct a 0 capacity IdPool either.

I feel strongly.

Using NonZero to prevent passing zero is a very strong mitigation due to
its big ergonomic cost. There's nothing really wrong about a
zero-capacity IdPool, so let's not pay the ergonomics cost when we don't
need to.

Alice

  reply	other threads:[~2026-08-25 13:12 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  7:04 [PATCH v7 00/10] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 01/10] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-08-17 10:29   ` Gary Guo
2026-08-17 19:42     ` Burak Emir
2026-08-17  7:04 ` [PATCH v7 02/10] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-08-17 19:51   ` Burak Emir
2026-08-21  7:37   ` Alexandre Courbot
2026-08-24 12:04     ` Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 03/10] rust: num: add nz! macro for compile time NonZero values Eliot Courtney
2026-08-19 20:07   ` Gary Guo
2026-08-25 11:10     ` Eliot Courtney
2026-08-21  7:39   ` Alexandre Courbot
2026-08-17  7:04 ` [PATCH v7 04/10] rust: sizes: implement SizeConstants for Alignment Eliot Courtney
2026-08-17  7:12   ` sashiko-bot
2026-08-21  7:46   ` Alexandre Courbot
2026-08-25  6:59     ` Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 05/10] rust: bitmap: add contiguous area operations Eliot Courtney
2026-08-17 20:12   ` Burak Emir
2026-08-21  8:11   ` Alexandre Courbot
2026-08-21  8:31     ` Miguel Ojeda
2026-08-21 11:04       ` Alexandre Courbot
2026-08-21 19:35         ` Miguel Ojeda
2026-08-25  8:33     ` Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 06/10] rust: id_pool: take a NonZero capacity in with_capacity Eliot Courtney
2026-08-17  7:12   ` sashiko-bot
2026-08-17 20:13   ` Burak Emir
2026-08-21  8:39   ` Alexandre Courbot
2026-08-25 11:09     ` Eliot Courtney
2026-08-25 13:12       ` Alice Ryhl [this message]
2026-08-25 23:43         ` Eliot Courtney
2026-08-25 12:10   ` Alice Ryhl
2026-08-17  7:04 ` [PATCH v7 07/10] rust: id_pool: add contiguous ID reservation Eliot Courtney
2026-08-17  7:14   ` sashiko-bot
2026-08-17 20:21   ` Burak Emir
2026-08-21  8:30   ` Alexandre Courbot
2026-08-17  7:04 ` [PATCH v7 08/10] rust: id_pool: do not round capacity up to BitmapVec::MAX_INLINE_LEN Eliot Courtney
2026-08-17 20:39   ` Burak Emir
2026-08-17  7:04 ` [PATCH v7 09/10] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-08-17  7:04 ` [PATCH v7 10/10] rust: use Alignment size constants Eliot Courtney
2026-08-21 11:04   ` Alexandre Courbot

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=ao2U1jNaT7waibJW@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.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-bounces@lists.freedesktop.org \
    --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=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.