All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yury Norov <yury.norov@gmail.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: "Burak Emir" <bqe@google.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"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,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] rust: bitops: fix missing _find_* functions on 32-bit ARM
Date: Mon, 5 Jan 2026 12:03:27 -0500	[thread overview]
Message-ID: <aVvu3zF2rYKR3XC0@yury> (raw)
In-Reply-To: <20260105-bitops-find-helper-v2-1-ae70b4fc9ecc@google.com>

On Mon, Jan 05, 2026 at 10:44:06AM +0000, Alice Ryhl wrote:
> atus: O
> Content-Length: 4697
> Lines: 121
> 
> On 32-bit ARM, you may encounter linker errors such as this one:
> 
> 	ld.lld: error: undefined symbol: _find_next_zero_bit
> 	>>> referenced by rust_binder_main.43196037ba7bcee1-cgu.0
> 	>>>               drivers/android/binder/rust_binder_main.o:(<rust_binder_main::process::Process>::insert_or_update_handle) in archive vmlinux.a
> 	>>> referenced by rust_binder_main.43196037ba7bcee1-cgu.0
> 	>>>               drivers/android/binder/rust_binder_main.o:(<rust_binder_main::process::Process>::insert_or_update_handle) in archive vmlinux.a
> 
> This error occurs because even though the functions are declared by
> include/linux/find.h, the definition is #ifdef'd out on 32-bit ARM. This
> is because arch/arm/include/asm/bitops.h contains:
> 
> 	#define find_first_zero_bit(p,sz)	_find_first_zero_bit_le(p,sz)
> 	#define find_next_zero_bit(p,sz,off)	_find_next_zero_bit_le(p,sz,off)
> 	#define find_first_bit(p,sz)		_find_first_bit_le(p,sz)
> 	#define find_next_bit(p,sz,off)		_find_next_bit_le(p,sz,off)
> 
> And the underscore-prefixed function is conditional on #ifndef of the
> non-underscore-prefixed name, but the declaration in find.h is *not*
> conditional on that #ifndef.
> 
> To fix the linker error, we ensure that the symbols in question exist
> when compiling Rust code. We do this by definining them in rust/helpers/
> whenever the normal definition is #ifndef'd out.
> 
> Note that these helpers are somewhat unusual in that they do not have
> the rust_helper_ prefix that most helpers have. Adding the rust_helper_
> prefix does not compile, as 'bindings::_find_next_zero_bit()' will
> result in a call to a symbol called _find_next_zero_bit as defined by
> include/linux/find.h rather than a symbol with the rust_helper_ prefix.
> This is because when a symbol is present in both include/ and
> rust/helpers/, the one from include/ wins under the assumption that the
> current configuration is one where that helper is unnecessary. This
> heuristic fails for _find_next_zero_bit() because the header file always
> declares it even if the symbol does not exist.
> 
> The functions still use the __rust_helper annotation. This lets the
> wrapper function be inlined into Rust code even if full kernel LTO is
> not used once the patch series for that feature lands.
> 
> Cc: stable@vger.kernel.org
> 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
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Which means, you're running active testing, which in turn means that
Rust is in a good shape indeed. Thanks to you and Andreas for the work.

Before I merge it, can you also test m68k build? Arm and m68k are the
only arches implementing custom API there.

Thanks,
Yury

> ---
> Changes in v2:
> - Remove rust_helper_ prefix from helpers.
> - Improve commit message.
> - The set of functions for which a helper is added is changed so that it
>   matches arch/arm/include/asm/bitops.h
> - Link to v1: https://lore.kernel.org/r/20251203-bitops-find-helper-v1-1-5193deb57766@google.com
> ---
>  rust/helpers/bitops.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/rust/helpers/bitops.c b/rust/helpers/bitops.c
> index 5d0861d29d3f0d705a014ae4601685828405f33b..e79ef9e6d98f969e2a0a2a6f62d9fcec3ef0fd72 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,44 @@ void rust_helper_clear_bit(unsigned long nr, volatile unsigned long *addr)
>  {
>  	clear_bit(nr, addr);
>  }
> +
> +/*
> + * The rust_helper_ prefix is intentionally omitted below so that the
> + * declarations in include/linux/find.h are compatible with these helpers.
> + *
> + * Note that the below #ifdefs mean that the helper is only created if C does
> + * not provide a definition.
> + */
> +#ifdef find_first_zero_bit
> +__rust_helper
> +unsigned long _find_first_zero_bit(const unsigned long *p, unsigned long size)
> +{
> +	return find_first_zero_bit(p, size);
> +}
> +#endif /* find_first_zero_bit */
> +
> +#ifdef find_next_zero_bit
> +__rust_helper
> +unsigned long _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_first_bit
> +__rust_helper
> +unsigned long _find_first_bit(const unsigned long *addr, unsigned long size)
> +{
> +	return find_first_bit(addr, size);
> +}
> +#endif /* find_first_bit */
> +
> +#ifdef find_next_bit
> +__rust_helper
> +unsigned long _find_next_bit(const unsigned long *addr, unsigned long size,
> +			     unsigned long offset)
> +{
> +	return find_next_bit(addr, size, offset);
> +}
> +#endif /* find_next_bit */
> 
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251203-bitops-find-helper-25ed1bbae700
> 
> Best regard

  reply	other threads:[~2026-01-05 17:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-05 10:44 [PATCH v2] rust: bitops: fix missing _find_* functions on 32-bit ARM Alice Ryhl
2026-01-05 17:03 ` Yury Norov [this message]
2026-01-05 17:08   ` Miguel Ojeda
2026-01-06  9:03   ` Alice Ryhl
2026-01-06 17:38     ` Yury Norov
2026-01-07  5:31       ` Dirk Behme
2026-01-06  8:48 ` Dirk Behme
2026-01-06 10:47 ` Andreas Hindborg

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=aVvu3zF2rYKR3XC0@yury \
    --to=yury.norov@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --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=stable@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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.