All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alice Ryhl <aliceryhl@google.com>
To: Andreas Hindborg <a.hindborg@kernel.org>
Cc: "Yury Norov" <yury.norov@gmail.com>,
	"Burak Emir" <bqe@google.com>, "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rust: bitops: add helpers for _find_* functions
Date: Thu, 4 Dec 2025 09:04:34 +0000	[thread overview]
Message-ID: <aTFOouqQ3-PbN0Kk@google.com> (raw)
In-Reply-To: <87ikenw5v9.fsf@t14s.mail-host-address-is-not-set>

On Wed, Dec 03, 2025 at 11:35:54PM +0100, Andreas Hindborg wrote:
> Alice Ryhl <aliceryhl@google.com> writes:
> 
> > On 32-bit ARM, _find_next_bit does not exist because ARM provides that
> > function through a #define instead. This means that Rust, which calls
> > the underscored version, fails to find it. This triggers errors:
> >
> > 	ld.lld: error: undefined symbol: _find_next_bit
> > 	>>> referenced by bitmap.rs:459 (rust/kernel/bitmap.rs:459)
> > 	>>>               rust/kernel.o:(kernel::bitmap::tests::kunit_rust_wrapper_bitmap_borrow) in archive vmlinux.a
> > 	>>> referenced by bitmap.rs:459 (rust/kernel/bitmap.rs:459)
> > 	>>>               rust/kernel.o:(kernel::bitmap::tests::kunit_rust_wrapper_bitmap_set_clear_find) in archive vmlinux.a
> > 	>>> referenced by bitmap.rs:459 (rust/kernel/bitmap.rs:459)
> > 	>>>               rust/kernel.o:(kernel::bitmap::tests::kunit_rust_wrapper_bitmap_set_clear_find) in archive vmlinux.a
> > 	>>> referenced 10 more times
> >
> > 	ld.lld: error: undefined symbol: _find_next_zero_bit
> > 	>>> referenced by bitmap.rs:479 (rust/kernel/bitmap.rs:479)
> > 	>>>               rust/kernel.o:(kernel::bitmap::tests::kunit_rust_wrapper_bitmap_copy) in archive vmlinux.a
> > 	>>> referenced by bitmap.rs:479 (rust/kernel/bitmap.rs:479)
> > 	>>>               rust/kernel.o:(kernel::bitmap::tests::kunit_rust_wrapper_bitmap_set_clear_find) in archive vmlinux.a
> > 	>>> referenced by bitmap.rs:479 (rust/kernel/bitmap.rs:479)
> > 	>>>               rust/kernel.o:(kernel::bitmap::tests::kunit_rust_wrapper_owned_bitmap_out_of_bounds) in archive vmlinux.a
> > 	>>> referenced 6 more times
> >
> > To fix this, add Rust helpers in this particular case.
> >
> > Fixes: 6cf93a9ed39e ("rust: add bindings for bitops.h")
> > Reported-by: Andreas Hindborg <a.hindborg@kernel.org>
> > Closes: https://rust-for-linux.zulipchat.com/#narrow/channel/x/topic/x/near/561677301
> > Tested-by: Andreas Hindborg <a.hindborg@kernel.org>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/helpers/bitops.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/rust/helpers/bitops.c b/rust/helpers/bitops.c
> > index 5d0861d29d3f0d705a014ae4601685828405f33b..84061af591a261f7268ffe6535282bf3c7608e2d 100644
> > --- a/rust/helpers/bitops.c
> > +++ b/rust/helpers/bitops.c
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  
> >  #include <linux/bitops.h>
> > +#include <linux/find.h>
> >  
> >  void rust_helper___set_bit(unsigned long nr, unsigned long *addr)
> >  {
> > @@ -21,3 +22,34 @@ void rust_helper_clear_bit(unsigned long nr, volatile unsigned long *addr)
> >  {
> >  	clear_bit(nr, addr);
> >  }
> > +
> > +/*
> > + * Rust normally calls the single-underscore-prefixed version of these
> > + * functions, which are not inlined. However, on some platforms, they do not
> > + * exist. In those cases, provide a rust helper for the underscored version.
> > + */
> > +#ifdef find_next_zero_bit
> > +__rust_helper unsigned long
> > +rust_helper__find_next_zero_bit(const unsigned long *addr, unsigned long size,
> > +				unsigned long offset)
> > +{
> > +	return find_next_zero_bit(addr, size, offset);
> > +}
> > +#endif /* find_next_zero_bit */
> > +
> > +#ifdef find_next_bit
> > +__rust_helper unsigned long
> > +rust_helper__find_next_bit(const unsigned long *addr, unsigned long size,
> > +			   unsigned long offset)
> > +{
> > +	return find_next_bit(addr, size, offset);
> > +}
> > +#endif /* find_next_bit */
> > +
> > +#ifdef find_last_bit
> > +__rust_helper unsigned long
> > +rust_helper__find_last_bit(const unsigned long *addr, unsigned long size)
> > +{
> > +	return find_last_bit(addr, size);
> > +}
> > +#endif /* find_last_bit */
> >
> > ---
> > base-commit: 54e3eae855629702c566bd2e130d9f40e7f35bde
> > change-id: 20251203-bitops-find-helper-25ed1bbae700
> >
> > Best regards,
> > -- 
> > Alice Ryhl <aliceryhl@google.com>
> 
> I messed up when testing this patch. It actually does not solve the
> issue.
> 
> It appears that bindgen will emit an extern "C" declaration for the
> missing C functions, even though they are not in the symbol table. When
> we add the rust helpers, the declarations emitted based on the
> non-existent C functions will mask the rust helper functions.

Yeah, when Rust sees both a helper and real function, we configured it
to pick the real one.

> We can circumvent this by using another name for our helper. While it
> works, it is not fixing the root cause. I am not sure why bindgen emits
> these functions even though they are not in the symbol table at the end.

I think a much better workaround is to adjust the header file:

#ifndef find_next_zero_bit
unsigned long _find_next_zero_bit(const unsigned long *addr, unsigned long nbits,
					 unsigned long start);
#endif

Alice

      reply	other threads:[~2025-12-04  9:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-03 15:42 [PATCH] rust: bitops: add helpers for _find_* functions Alice Ryhl
2025-12-03 22:35 ` Andreas Hindborg
2025-12-04  9:04   ` Alice Ryhl [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aTFOouqQ3-PbN0Kk@google.com \
    --to=aliceryhl@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=bqe@google.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.