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: Wed, 8 Jul 2026 16:37:38 -0400	[thread overview]
Message-ID: <ak61Embrj0qH1vas@yury> (raw)
In-Reply-To: <2026070839-quadrant-gentleman-2334@gregkh>

On Wed, Jul 08, 2026 at 07:33:16AM +0200, Greg KH wrote:

...

> > 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.
> 
> You are comparing the C vs. Rust data structures here, which is not what
> I am proposing.
> 
> Also, is this code being used on a "hot path" like the binder stuff is?
> 
> > 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...
> 
> Again, I'm not talking about Rust API vs. imported C bitmaps, I'm asking
> to use the C structures like maple-tree and idr instead of open-coding
> logic around the bitmap code.

I understand your point. I asked both questions: are they sure that bitmap
is the most optimal data structure for the ID pool, and if so, why not use
the built-in Rust bitmaps? The answer was: yes, it's the most optimal, and
using built-in bitmaps is suboptimal overall.

Burak, can you share more details please?

> > > > > 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'.
> 
> Allocating IDs is a probe() thing, which can be as slow as it wants,
> right?  Or is this some other hot-path where performance matters?  The
> patch was not very specific as to the tradeoffs needed.

I am a bitmaps maintainer, and I want them in Rust to perform equally well.
The ID pool case was just the first user. Even if bitmap performance is
not a critical path for ID pool, my role is to make sure that bitmap in
Rust is implemented well, including the performance part.

Said that, I recall that bitmaps performance was important for the ID
pool, and Alice and Burak spent some time optimizing the ID allocation
path. Particularly, added a logic to avoid allocation of small bitmaps.

Thanks,
Yury

  reply	other threads:[~2026-07-08 20:37 UTC|newest]

Thread overview: 27+ 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-09  9:01         ` Alice Ryhl
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
2026-07-08  5:33           ` Greg KH
2026-07-08 20:37             ` Yury Norov [this message]
2026-07-08 21:08               ` Alice Ryhl
2026-07-10 15:09                 ` Burak Emir
2026-07-10 14:21             ` Eliot Courtney
2026-07-10 14:23               ` Alice Ryhl
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=ak61Embrj0qH1vas@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.