Re: [PATCH v9 2/5] rust: add bindings for bitops.h

[Burak Emir]

On Tue, May 27, 2025 at 6:39 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>
> On Mon, May 26, 2025 at 03:01:31PM +0000, Burak Emir wrote:
> > --- /dev/null
> > +++ b/rust/helpers/bitops.c
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/bitops.h>
> > +
> > +void rust_helper___set_bit(unsigned int nr, unsigned long *addr)
>
> Why "unsigned int" instead of "unsigned long"? The C API uses
> "unsigned long" and in the Rust API, you uses "usize" for `nbits` and
> `index`s, therefore using "unsigned int" only introduces unnecessary "as
> u32" casting IMO, am I missing something here?

This is certainly a bit confusing to me.

As Jann points out here [1] the expand to asm macros arch___set_bit,
where `nr` is indeed unsigned long.

There is also the constraint that a bitmap's length must be shorter
than INT32_MAX bits, but that does not matter.

Being consistent with the C API, including parameter names, is what we
should do, so fixing this.
I found the underlying C API is just hard to find for these macros.
Thanks for catching.

cheers
- Burak

[1] https://lore.kernel.org/rust-for-linux/CAG48ez1NM7B8Vk7GOwhsitCipmfHi9eK6JNb3ve8aR4m8Cj0gA@xxxxxxxxxxxxxx/