rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: Carlos Llamas <cmllamas@google.com>
Cc: "Boqun Feng" <boqun.feng@gmail.com>,
	"Jann Horn" <jannh@google.com>, "Burak Emir" <bqe@google.com>,
	"Kees Cook" <kees@kernel.org>,
	"Rasmus Villemoes" <linux@rasmusvillemoes.dk>,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Gustavo A . R . Silva" <gustavoars@kernel.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v8 5/5] rust: add dynamic ID pool abstraction for bitmap
Date: Wed, 21 May 2025 09:50:48 -0400	[thread overview]
Message-ID: <aC3aOHWEGzjjDNgb@yury> (raw)
In-Reply-To: <aC1PQ7tmcqMSmbHc@google.com>

On Wed, May 21, 2025 at 03:57:55AM +0000, Carlos Llamas wrote:
> On Mon, May 19, 2025 at 08:57:04PM -0400, Yury Norov wrote:
> > + Carlos Llamas

...

> > Carlos, can you please elaborate your motivation to switch to bitmaps?
> > Have you considered rb-trees with O(logn) lookup?
> 
> Yeah, we tried rb-trees. There was even a patch that implemented the
> augmented logic. See this:
> https://lore.kernel.org/all/20240917030203.286-1-ebpqwerty472123@gmail.com/
> IIRC, it just didn't make sense for our use case because of the extra
> memory bytes required for this solution. The performance ended up being
> the same (from my local testing).
> 
> I'm not certain of this but one potential factor is that the rb nodes
> are in-strucutre members allocated separately. This can lead to more
> cache misses when traversing them. I don't know how applicable this
> would be for the Rust implementation though. Take that with a grain of
> salt as I didn't actually look super close while running the tests.
> 
> I would also note, this whole logic wouldn't be required if userspace
> wasn't using these descriptor IDs as vector indexes. At some point this
> practice will be fixed and we can remove the "dbitmap" implementation.

Yeah, I expected to get this kind of feedback from real-world testing.
Your reply to the patch you mentioned above answers all my questions:

https://lore.kernel.org/all/ZwdWe_I2p3zD-v1O@google.com/

Let's stick to bitmaps unless someone shows clear benefit of using any
alternative approach, both in time and memory perspectives, supported
by testing.

Thanks,
Yury

  reply	other threads:[~2025-05-21 13:50 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-19 16:17 [PATCH v8 0/5] rust: adds Bitmap API, ID pool and bindings Burak Emir
2025-05-19 16:17 ` [PATCH v8 1/5] rust: add bindings for bitmap.h Burak Emir
2025-05-19 16:17 ` [PATCH v8 2/5] rust: add bindings for bitops.h Burak Emir
2025-05-19 16:17 ` [PATCH v8 3/5] rust: add bitmap API Burak Emir
2025-05-19 18:22   ` Yury Norov
2025-05-19 20:41     ` Burak Emir
2025-05-19 20:51       ` Jann Horn
2025-05-19 22:07       ` Yury Norov
2025-05-19 19:00   ` Jann Horn
2025-05-19 20:07     ` Burak Emir
2025-05-19 20:09       ` Burak Emir
2025-05-19 20:36       ` Jann Horn
2025-05-19 20:49         ` Boqun Feng
2025-05-19 21:42       ` Miguel Ojeda
2025-05-19 21:49         ` Burak Emir
2025-05-20  5:59   ` kernel test robot
2025-05-19 16:17 ` [PATCH v8 4/5] rust: add find_bit_benchmark_rust module Burak Emir
2025-05-19 17:39   ` Yury Norov
2025-05-19 18:11     ` Miguel Ojeda
2025-05-19 16:17 ` [PATCH v8 5/5] rust: add dynamic ID pool abstraction for bitmap Burak Emir
2025-05-19 19:46   ` Yury Norov
2025-05-19 22:51   ` Jann Horn
2025-05-19 23:12     ` Alice Ryhl
2025-05-19 23:43       ` Jann Horn
2025-05-19 23:56     ` Boqun Feng
2025-05-20  0:57       ` Yury Norov
2025-05-20  3:45         ` Alice Ryhl
2025-05-21  3:57         ` Carlos Llamas
2025-05-21 13:50           ` Yury Norov [this message]
2025-05-26 14:22             ` Burak Emir
2025-05-20  3:46       ` Alice Ryhl
2025-05-20  5:21         ` Boqun Feng
2025-05-20 12:42           ` Alice Ryhl
2025-05-20 12:56             ` Boqun Feng
2025-05-20 13:05               ` Alice Ryhl
2025-05-20 13:21                 ` Boqun Feng
2025-05-20 15:55                   ` Jann Horn
2025-05-20 16:54                     ` Boqun Feng
2025-05-20 19:43   ` Jann Horn
2025-05-19 18:34 ` [PATCH v8 0/5] rust: adds Bitmap API, ID pool and bindings Yury Norov

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=aC3aOHWEGzjjDNgb@yury \
    --to=yury.norov@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=bqe@google.com \
    --cc=cmllamas@google.com \
    --cc=gary@garyguo.net \
    --cc=gustavoars@kernel.org \
    --cc=jannh@google.com \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=viresh.kumar@linaro.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).