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

[Yury Norov <yury.norov@gmail.com>]

On Thu, Mar 27, 2025 at 04:16:10PM +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.

So, I hung around and found the bit_set crate. Rust implements BitSet,
BitVec, and it seems it has functionality that you bind here.

I didn't find any discussions related to the bit_set in kernel context.
Is it possible to use it in kernel? If not, can you mention that in commit
message? If yes, I think you should consider to use internal language
tools.

> We include set_bit_atomic and clear_bit_atomic operations. One has
> to avoid races with non-atomic operations, which is ensure by the
> Rust type system: either callers have shared references &bitmap in
> which case the mutations are atomic operations. Or there is a
> exclusive reference &mut bitmap, in which case there is no concurrent
> access.
> 
> This version includes an optimization to represent the bitmap inline,
> as suggested by Yury.
> 
> We introduce a Rust API that would replace (dbitmap.h) in file id_pool.rs. 
> This data structure is tightly coupled to the bitmap API. Includes an example of usage
> that requires releasing a spinlock, as expected in Binder driver.
> 
> This is v6 of a patch introducing Rust bitmap API [v5]. Thanks
> for all the helpful comments, this series has improved significantly
> as a result of your work.
> 
> Not adding separate unit tests: the Rust unit test infrastructure
> is very new, and there does not seem to be benchmarking support
> for Rust tests yet.

I don't understand this.

Benchmarking is a very simple procedure - as simple as surrounding
blocks of tested code with ktime_get(). And I see that Alice even
implemented the Ktime class last year.

> Are the # Examples tests enough?
> Alternatively, can we add more test cases to those until 
> the unit test infrastructure is in place?

I encourage you to implement the tests as normal kernel tests - in
source files that may be enabled in config. I can't insist on that,
and will not block the series because of lack of benchmarks and
tests written in a traditional way.

But to me, scattered wrongly formatted commented-out in-place way of
writing tests is something fundamentally wrong. Not mentioning that
it bloats source files, making them harder to read.

Thanks,
Yury