Re: [PATCH v5 0/4] rust: adds Bitmap API, ID pool and bindings

[Burak Emir]

On Fri, Mar 21, 2025 at 3:31 PM Yury Norov <yury.norov@xxxxxxxxx> wrote:
>
> On Fri, Mar 21, 2025 at 11:15:28AM +0000, Burak Emir wrote:
> > This series adds a Rust bitmap API for porting the approach from
> > commit 15d9da3f818c ("binder: use bitmap for faster descriptor lookup")
> > to Rust. The functionality in dbitmap.h makes use of bitmap and bitops.
> >
> > The Rust bitmap API provides a safe abstraction to underlying bitmap
> > and bitops operations. For now, only includes method necessary for
> > dbitmap.h, more can be added later. We perform bounds checks for
> > hardening, violations are programmer errors that result in panics.
> >
> > This version includes an optimization to represent the bitmap inline,
> > as suggested by Yury.
>
> We have a tag for it:
>
> Suggested-by: Yury Norov <yury.norov@xxxxxxxxx>

Will add.

>
> > The Rust equivalent of dbitmap.h is included as id_pool.rs, which is
> > tightly coupled to the bitmap API. Includes an example of usage
> > that requires releasing a spinlock, as expected in Binder driver.
>
> I don't think it's worth to refer the existing dbitmap.h, because:
>
> 1. It's buggy;
> 2. You limit yourself with committing to provide an 'equivalent' API.
> 3. If you want to bring the existing dbitmaps.h, you'd just bring
>    bindings for them, not a re-implementation.
>
> Can you just say that you're adding dynamic bit arrays in rust?
>

True, "equivalent" is maybe too strong.

I considered "DybamicBitmap" but dbitmap is only providing a
dynamically sized bitmap under the hood (and with a specific API).
It does not expose `set_bit`, `clear_bit` methods as one would expect
from a Bitmap API.

That is why I found IdPool a better name. I am not attached to the
name, just dynamic bitmap seems misleading.
It is essentially a dynamically sized thing that happens to be
implemented efficiently using a bitmap.
I will leave it to Alice to explain why Binder needs it.


> > This is v5 of a patch introducing Rust bitmap API [v4]. Thanks
> > for all the helpful comments, this series has improved significantly
> > as a result of your work.
> >
> > Changes v4 --> v5: (suggested by Yury and Alice)
> > - rebased on next-20250318
> > - split MAINTAINERS changes
> > - no dependencies on [1] and [2] anymore - Viresh,
> >   please do add a separate section if you want to maintain cpumask.rs
> >   separately.
> > - imports atomic and non-atomic variants, introduces a naming convention
> >   set_bit and set_bit_atomic on the Rust side.
> > - changed naming and comments. Keeping `new`.
> > - change dynamic_id_pool to id_pool
> > - represent bitmap inline when possible
> > - add some more tests
> > - add bqe@xxxxxxxxxx as M: for the Rust abstractions
>
> Instead of 'bqe@xxxxxxxxxx' just say: myself.

Sure thing : )

Thanks,
Burak