All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yury Norov <ynorov@nvidia.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "Eliot Courtney" <ecourtney@nvidia.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Burak Emir" <bqe@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>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"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 3/4] rust: id_pool: add contiguous area allocation
Date: Tue, 7 Jul 2026 12:31:08 -0400	[thread overview]
Message-ID: <ak0pzDc69nAmnviD@yury> (raw)
In-Reply-To: <2026070745-unwired-maybe-769e@gregkh>

On Tue, Jul 07, 2026 at 04:13:30PM +0200, Greg KH wrote:
> On Tue, Jul 07, 2026 at 10:25:27PM +0900, Eliot Courtney wrote:
> > On Fri Jul 3, 2026 at 7:31 PM JST, Greg KH wrote:
> > > On Fri, Jul 03, 2026 at 07:16:06PM +0900, Eliot Courtney wrote:
> > >> Add support for contiguous area allocation. Add a new type,
> > >> `UnusedArea`, following the same pattern as `UnusedId`.
> > >> 
> > >> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> > >
> > > Why isn't the built-in idr library being used here instead of rolling
> > > your own data structure?
> > >
> > > thanks,
> > >
> > > greg k-h
> > 
> > For nova-core in this series, we need allocation of a contiguous
> > sequence of IDs with a specific length and sometimes a specific
> > alignment. IIUC, IDA/xarray do not support that (I checked
> > ida_alloc_range and it only allocates a single ID in a range, not a
> > contiguous sequence).
> > 
> > For IdPool before this series, I think it could have used IDA/xarray.
> > See [1] where Alice has posted some more context.
> > 
> > w.r.t. the structure choice, the IDs we need to allocate are channel
> > IDs, and the total range is limited to 2048 of them, so IMO bitmaps are
> > a better fit than e.g. maple tree.
> 
> But again, you are having to "roll your own" logic here, please reuse
> the data structures we already have in the kernel for this type of
> thing.  If a maple tree works, please use it.

I asked exactly the same question when Alice and Burak added wrappers
for bitmaps to implement their ID pool. This is the answer:

  An alternative route of vendoring an existing Rust bitmap package was
  considered but suboptimal overall. Reusing the C implementation is
  preferable for a basic data structure like bitmaps. It enables Rust
  code to be a lot more similar and predictable with respect to C code
  that uses the same data structures and enables the use of code that
  has been tried-and-tested in the kernel, with the same performance
  characteristics whenever possible.

And now it's in a commit message: 11eca92a2caeb

They measured the affect of their wrapper on performance, and it appears
to be ~5%. See lib/find_bit_benchmark_rust.rs.

I didn't see any side-to-side comparison between any native Rust API vs
imported C bitmaps. I'm sure, I asked for that, and I still believe
it's the important piece of data to avoid this back-and-forth type of
discussions. So, Alice, Burak or anybody...

> > > Why isn't the built-in idr library being used here instead of rolling
> > > your own data structure?

Now having more context, the ID pool's primary goal is to allocate
individual IDs, which naturally lays on find_bit() API in C. The
native Rust alternative is considered and found 'suboptimal overall'.

This series extends the existing data structure in Rust for Linux, not
adding a new one. So, the series itself looks justified to me because
it's a logical extension of the existing functionality.

Regarding C bindings vs native API... Yes we lack thorough performance
analysis. Do we have such analysis for every data structure in kernel?

No. So to me it's as simple as: they are developers, they know better.

Thanks,
Yury

  reply	other threads:[~2026-07-07 16:31 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-03 10:16 [PATCH 0/4] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-07-03 10:16 ` [PATCH 1/4] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-07-07 13:55   ` Alice Ryhl
2026-07-03 10:16 ` [PATCH 2/4] rust: bitmap: add contiguous area operations Eliot Courtney
2026-07-03 10:30   ` sashiko-bot
2026-07-06 16:29   ` Yury Norov
2026-07-06 17:21     ` Alice Ryhl
2026-07-06 18:22       ` Gary Guo
2026-07-03 10:16 ` [PATCH 3/4] rust: id_pool: add contiguous area allocation Eliot Courtney
2026-07-03 10:25   ` sashiko-bot
2026-07-03 10:31   ` Greg KH
2026-07-07 13:25     ` Eliot Courtney
2026-07-07 14:13       ` Greg KH
2026-07-07 16:31         ` Yury Norov [this message]
2026-07-03 10:16 ` [PATCH 4/4] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-07-06 11:48 ` [PATCH 0/4] rust: Add support for reserving of ranges of IDs Alice Ryhl
2026-07-06 11:53   ` Gary Guo
2026-07-06 12:46     ` Alice Ryhl
2026-07-07 13:26   ` Eliot Courtney
2026-07-07 13:32     ` Alice Ryhl

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=ak0pzDc69nAmnviD@yury \
    --to=ynorov@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=bqe@google.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.