Re: [PATCH v4 1/6] rust: bitmap: add MAX_LEN and NO_ALLOC_MAX_LEN constants

[Alice Ryhl <aliceryhl@google.com>]

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@google.com> wrote:
> > > >
> > > > To avoid hard-coding these values in drivers, define constants for them
> > > > that drivers can reference.
> > > >
> > > > Acked-by: Danilo Krummrich <dakr@kernel.org>
> > > > Reviewed-by: Burak Emir <bqe@google.com>
> > > > Signed-off-by: Alice Ryhl <aliceryhl@google.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