lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aRIAp3LHs0NsEKvL@google.com>
Date: Mon, 10 Nov 2025 15:11:35 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Yury Norov <yury.norov@...il.com>
Cc: Tamir Duberstein <tamird@...il.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"Arve Hjønnevåg" <arve@...roid.com>, Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>, 
	Joel Fernandes <joelagnelf@...dia.com>, Christian Brauner <brauner@...nel.org>, 
	Carlos Llamas <cmllamas@...gle.com>, Suren Baghdasaryan <surenb@...gle.com>, Burak Emir <bqe@...gle.com>, 
	Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, 
	Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/6] rust: bitmap: add MAX_LEN and NO_ALLOC_MAX_LEN constants

On Mon, Nov 10, 2025 at 10:01:29AM -0500, Yury Norov wrote:
> On Mon, Nov 10, 2025 at 02:20:17PM +0000, Alice Ryhl wrote:
> > On Mon, Nov 10, 2025 at 08:59:36AM -0500, Tamir Duberstein wrote:
> > > On Mon, Nov 10, 2025 at 8:06 AM Alice Ryhl <aliceryhl@...gle.com> wrote:
> > > >
> > > > To avoid hard-coding these values in drivers, define constants for them
> > > > that drivers can reference.
> > > >
> > > > Acked-by: Danilo Krummrich <dakr@...nel.org>
> > > > Reviewed-by: Burak Emir <bqe@...gle.com>
> > > > Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> > > > ---
> > > >  rust/kernel/bitmap.rs | 16 +++++++++++-----
> > > >  1 file changed, 11 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/rust/kernel/bitmap.rs b/rust/kernel/bitmap.rs
> > > > index aa8fc7bf06fc99865ae755d8694e4bec3dc8e7f0..15fa23b45054b9272415fcc000e3e3b52c74d7c1 100644
> > > > --- a/rust/kernel/bitmap.rs
> > > > +++ b/rust/kernel/bitmap.rs
> > > > @@ -149,14 +149,14 @@ macro_rules! bitmap_assert_return {
> > > >  ///
> > > >  /// # Invariants
> > > >  ///
> > > > -/// * `nbits` is `<= i32::MAX` and never changes.
> > > > +/// * `nbits` is `<= MAX_LEN`.
> > > >  /// * if `nbits <= bindings::BITS_PER_LONG`, then `repr` is a `usize`.
> > > 
> > > Should this and other references to bindings::BITS_PER_LONG be
> > > `NO_ALLOC_MAX_LEN` instead?
> > 
> > Ah yeah it probably makes sense to update this in a bunch of places.
> 
> Yes, please. 
> 
> NO_ALLOC sounds a bit weird in exported API. Maybe NBITS_INPLACE
> or similar?

Ah, good point. We started using the "inplace" wording in other places,
so lets also do so here.

> Also, at this point we're really close to:
> 
>    pub const NBITS_INPLACE: usize = CONFIG_NBITS_INPLACE;
> 
>    union BitmapRepr {
>        bitmap: [usize, BITS_TO_LONGS(NBITS_INPLACE)]
>        ptr: NonNull<usize>,
>    }
> 
> That would be a very useful addition for some particular scenarios,
> I believe. Even if you don't want to make it configurable, let's
> keep this option in mind?

Actually, one option here is to define BitmapVec like this:

pub struct BitmapVec<const INPLACE_LEN: usize = 1> {
    repr: BitmapRepr<INPLACE_LEN>,
    nbits: usize,
}

union BitmapRepr<const INPLACE_LEN: usize> {
    bitmap: [usize; INPLACE_LEN],
    ptr: NonNull<usize>,
}

This way, the driver can specify this by saying: BitmapVec<4> for a
BitmapVec where the inline capacity is 4 longs.

And if Binder wanted to make that configurable, Binder could define a
constant based on a Binder specific CONFIG_* that controls what value
Binder passes.

Since I wrote `= 1` in the struct, you may also write BitmapVec without
specifying any number and get the default.

It may be possible to specify the number in bits rather than longs too,
but then we have to decide what to do if it's not divisible by
BITS_PER_LONG.

(But in the case of Rust Binder, the value we want is one long worth of
bits.)

Alice

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ