`build_assert` relies on the compiler to optimize out its error path, lest build fails with the dreaded error:
ERROR: modpost: "rust_build_error" [path/to/module.ko] undefined!
It has been observed that very trivial code performing I/O accesses (sometimes even using an immediate value) would seemingly randomly fail with this error whenever `CLIPPY=1` was set. The same behavior was also observed until different, very similar conditions [1][2].
The cause, as pointed out by Gary Guo [3], appears to be that the failing function is eventually using `build_assert` with its argument, but is only annotated with `#[inline]`. This gives the compiler freedom to not inline the function, which it notably did when Clippy was active, triggering the error.
The fix is to annotate functions passing their argument to `build_assert` with `#[inline(always)]`, telling the compiler to be as aggressive as possible with their inlining. This is also the correct behavior as inlining is mandatory for correct behavior in these cases.
This series fixes all possible points of failure in the kernel crate, and adds documentation to `build_assert` explaining how to properly inline functions for which this behavior may arise.
[1] https://lore.kernel.org/all/DEEUYUOAEZU3.1J1HM2YQ10EX1@nvidia.com/ [2] https://lore.kernel.org/all/A1A280D4-836E-4D75-863E-30B1C276C80C@collabora.c... [3] https://lore.kernel.org/all/20251121143008.2f5acc33.gary@garyguo.net/
Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- Changes in v3: - Add "Fixes:" tags. - CC stable on fixup patches. - Link to v2: https://patch.msgid.link/20251128-io-build-assert-v2-0-a9ea9ce7d45d@nvidia.c...
Changes in v2: - Turn into a series and address other similar cases in the kernel crate. - Link to v1: https://patch.msgid.link/20251127-io-build-assert-v1-1-04237f2e5850@nvidia.c...
--- Alexandre Courbot (7): rust: build_assert: add instructions for use with function arguments rust: io: always inline functions using build_assert with arguments rust: cpufreq: always inline functions using build_assert with arguments rust: bits: always inline functions using build_assert with arguments rust: sync: refcount: always inline functions using build_assert with arguments rust: irq: always inline functions using build_assert with arguments rust: num: bounded: add missing comment for always inlined function
rust/kernel/bits.rs | 6 ++++-- rust/kernel/build_assert.rs | 7 ++++++- rust/kernel/cpufreq.rs | 2 ++ rust/kernel/io.rs | 9 ++++++--- rust/kernel/io/resource.rs | 2 ++ rust/kernel/irq/flags.rs | 2 ++ rust/kernel/num/bounded.rs | 1 + rust/kernel/sync/refcount.rs | 3 ++- 8 files changed, 25 insertions(+), 7 deletions(-) --- base-commit: ba65a4e7120a616d9c592750d9147f6dcafedffa change-id: 20251127-io-build-assert-3579a5bfb81c
Best regards,
`build_assert` relies on the compiler to optimize out its error path. Functions using it with its arguments must thus always be inlined, otherwise the error path of `build_assert` might not be optimized out, triggering a build error.
Cc: stable@vger.kernel.org Fixes: c6af9a1191d0 ("rust: cpufreq: Extend abstractions for driver registration") Acked-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- rust/kernel/cpufreq.rs | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index f968fbd22890..0879a79485f8 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -1015,6 +1015,8 @@ impl<T: Driver> Registration<T> { ..pin_init::zeroed() };
+ // Always inline to optimize out error path of `build_assert`. + #[inline(always)] const fn copy_name(name: &'static CStr) -> [c_char; CPUFREQ_NAME_LEN] { let src = name.to_bytes_with_nul(); let mut dst = [0; CPUFREQ_NAME_LEN];
`build_assert` relies on the compiler to optimize out its error path. Functions using it with its arguments must thus always be inlined, otherwise the error path of `build_assert` might not be optimized out, triggering a build error.
Cc: stable@vger.kernel.org Fixes: 746680ec6696 ("rust: irq: add flags module") Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- rust/kernel/irq/flags.rs | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/rust/kernel/irq/flags.rs b/rust/kernel/irq/flags.rs index adfde96ec47c..d26e25af06ee 100644 --- a/rust/kernel/irq/flags.rs +++ b/rust/kernel/irq/flags.rs @@ -96,6 +96,8 @@ pub(crate) fn into_inner(self) -> c_ulong { self.0 }
+ // Always inline to optimize out error path of `build_assert`. + #[inline(always)] const fn new(value: u32) -> Self { build_assert!(value as u64 <= c_ulong::MAX as u64); Self(value as c_ulong)
`build_assert` relies on the compiler to optimize out its error path. Functions using it with its arguments must thus always be inlined, otherwise the error path of `build_assert` might not be optimized out, triggering a build error.
Cc: stable@vger.kernel.org Fixes: ce30d94e6855 ("rust: add `io::{Io, IoRaw}` base types") Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- rust/kernel/io.rs | 9 ++++++--- rust/kernel/io/resource.rs | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 98e8b84e68d1..b64b11f75a35 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -142,7 +142,8 @@ macro_rules! define_read { /// Bound checks are performed on compile time, hence if the offset is not known at compile /// time, the build will fail. $(#[$attr])* - #[inline] + // Always inline to optimize out error path of `io_addr_assert`. + #[inline(always)] pub fn $name(&self, offset: usize) -> $type_name { let addr = self.io_addr_assert::<$type_name>(offset);
@@ -171,7 +172,8 @@ macro_rules! define_write { /// Bound checks are performed on compile time, hence if the offset is not known at compile /// time, the build will fail. $(#[$attr])* - #[inline] + // Always inline to optimize out error path of `io_addr_assert`. + #[inline(always)] pub fn $name(&self, value: $type_name, offset: usize) { let addr = self.io_addr_assert::<$type_name>(offset);
@@ -239,7 +241,8 @@ fn io_addr<U>(&self, offset: usize) -> Result<usize> { self.addr().checked_add(offset).ok_or(EINVAL) }
- #[inline] + // Always inline to optimize out error path of `build_assert`. + #[inline(always)] fn io_addr_assert<U>(&self, offset: usize) -> usize { build_assert!(Self::offset_valid::<U>(offset, SIZE));
diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs index 56cfde97ce87..b7ac9faf141d 100644 --- a/rust/kernel/io/resource.rs +++ b/rust/kernel/io/resource.rs @@ -226,6 +226,8 @@ impl Flags { /// Resource represents a memory region that must be ioremaped using `ioremap_np`. pub const IORESOURCE_MEM_NONPOSTED: Flags = Flags::new(bindings::IORESOURCE_MEM_NONPOSTED);
+ // Always inline to optimize out error path of `build_assert`. + #[inline(always)] const fn new(value: u32) -> Self { crate::build_assert!(value as u64 <= c_ulong::MAX as u64); Flags(value as c_ulong)
`build_assert` relies on the compiler to optimize out its error path. Functions using it with its arguments must thus always be inlined, otherwise the error path of `build_assert` might not be optimized out, triggering a build error.
Cc: stable@vger.kernel.org Fixes: cc84ef3b88f4 ("rust: bits: add support for bits/genmask macros") Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- rust/kernel/bits.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/bits.rs b/rust/kernel/bits.rs index 553d50265883..2daead125626 100644 --- a/rust/kernel/bits.rs +++ b/rust/kernel/bits.rs @@ -27,7 +27,8 @@ pub fn [<checked_bit_ $ty>](n: u32) -> Option<$ty> { /// /// This version is the default and should be used if `n` is known at /// compile time. - #[inline] + // Always inline to optimize out error path of `build_assert`. + #[inline(always)] pub const fn [<bit_ $ty>](n: u32) -> $ty { build_assert!(n < <$ty>::BITS); (1 as $ty) << n @@ -75,7 +76,8 @@ pub fn [<genmask_checked_ $ty>](range: RangeInclusive<u32>) -> Option<$ty> { /// This version is the default and should be used if the range is known /// at compile time. $(#[$genmask_ex])* - #[inline] + // Always inline to optimize out error path of `build_assert`. + #[inline(always)] pub const fn [<genmask_ $ty>](range: RangeInclusive<u32>) -> $ty { let start = *range.start(); let end = *range.end();
`build_assert` relies on the compiler to optimize out its error path. Functions using it with its arguments must thus always be inlined, otherwise the error path of `build_assert` might not be optimized out, triggering a build error.
Cc: stable@vger.kernel.org Fixes: bb38f35b35f9 ("rust: implement `kernel::sync::Refcount`") Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com --- rust/kernel/sync/refcount.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync/refcount.rs b/rust/kernel/sync/refcount.rs index 19236a5bccde..6c7ae8b05a0b 100644 --- a/rust/kernel/sync/refcount.rs +++ b/rust/kernel/sync/refcount.rs @@ -23,7 +23,8 @@ impl Refcount { /// Construct a new [`Refcount`] from an initial value. /// /// The initial value should be non-saturated. - #[inline] + // Always inline to optimize out error path of `build_assert`. + #[inline(always)] pub fn new(value: i32) -> Self { build_assert!(value >= 0, "initial value saturated"); // SAFETY: There are no safety requirements for this FFI call.
On Mon, 08 Dec 2025 11:46:58 +0900 Alexandre Courbot acourbot@nvidia.com wrote:
`build_assert` relies on the compiler to optimize out its error path, lest build fails with the dreaded error:
ERROR: modpost: "rust_build_error" [path/to/module.ko] undefined!It has been observed that very trivial code performing I/O accesses (sometimes even using an immediate value) would seemingly randomly fail with this error whenever `CLIPPY=1` was set. The same behavior was also observed until different, very similar conditions [1][2].
The cause, as pointed out by Gary Guo [3], appears to be that the failing function is eventually using `build_assert` with its argument, but is only annotated with `#[inline]`. This gives the compiler freedom to not inline the function, which it notably did when Clippy was active, triggering the error.
That's an interesting observation, so `#[inline]` is fine without clippy but `#[inline(always)]` is needed when Clippy is used?
I know Clippy would affect codegen but this might be a first concrete example that it actually creates (non-perf) issues that I've countered in practice.
The fix is to annotate functions passing their argument to `build_assert` with `#[inline(always)]`, telling the compiler to be as aggressive as possible with their inlining. This is also the correct behavior as inlining is mandatory for correct behavior in these cases.
Yeah, I suppose when you draw parallelism with C `BUILD_BUG` macro, there are a few users of that in other macros, which are kinda force-inlined.
This series fixes all possible points of failure in the kernel crate, and adds documentation to `build_assert` explaining how to properly inline functions for which this behavior may arise.
[1] https://lore.kernel.org/all/DEEUYUOAEZU3.1J1HM2YQ10EX1@nvidia.com/ [2] https://lore.kernel.org/all/A1A280D4-836E-4D75-863E-30B1C276C80C@collabora.c... [3] https://lore.kernel.org/all/20251121143008.2f5acc33.gary@garyguo.net/
Signed-off-by: Alexandre Courbot acourbot@nvidia.com
Changes in v3:
- Add "Fixes:" tags.
- CC stable on fixup patches.
- Link to v2: https://patch.msgid.link/20251128-io-build-assert-v2-0-a9ea9ce7d45d@nvidia.c...
Changes in v2:
- Turn into a series and address other similar cases in the kernel crate.
- Link to v1: https://patch.msgid.link/20251127-io-build-assert-v1-1-04237f2e5850@nvidia.c...
Alexandre Courbot (7): rust: build_assert: add instructions for use with function arguments rust: io: always inline functions using build_assert with arguments rust: cpufreq: always inline functions using build_assert with arguments rust: bits: always inline functions using build_assert with arguments rust: sync: refcount: always inline functions using build_assert with arguments rust: irq: always inline functions using build_assert with arguments rust: num: bounded: add missing comment for always inlined function
rust/kernel/bits.rs | 6 ++++-- rust/kernel/build_assert.rs | 7 ++++++- rust/kernel/cpufreq.rs | 2 ++ rust/kernel/io.rs | 9 ++++++--- rust/kernel/io/resource.rs | 2 ++ rust/kernel/irq/flags.rs | 2 ++ rust/kernel/num/bounded.rs | 1 + rust/kernel/sync/refcount.rs | 3 ++- 8 files changed, 25 insertions(+), 7 deletions(-)
base-commit: ba65a4e7120a616d9c592750d9147f6dcafedffa change-id: 20251127-io-build-assert-3579a5bfb81c
Best regards,
On Mon, 08 Dec 2025 11:47:01 +0900 Alexandre Courbot acourbot@nvidia.com wrote:
`build_assert` relies on the compiler to optimize out its error path. Functions using it with its arguments must thus always be inlined, otherwise the error path of `build_assert` might not be optimized out, triggering a build error.
Cc: stable@vger.kernel.org Fixes: c6af9a1191d0 ("rust: cpufreq: Extend abstractions for driver registration") Acked-by: Viresh Kumar viresh.kumar@linaro.org Reviewed-by: Daniel Almeida daniel.almeida@collabora.com Signed-off-by: Alexandre Courbot acourbot@nvidia.com
rust/kernel/cpufreq.rs | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index f968fbd22890..0879a79485f8 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -1015,6 +1015,8 @@ impl<T: Driver> Registration<T> { ..pin_init::zeroed() };
- // Always inline to optimize out error path of `build_assert`.
- #[inline(always)] const fn copy_name(name: &'static CStr) -> [c_char; CPUFREQ_NAME_LEN] { let src = name.to_bytes_with_nul(); let mut dst = [0; CPUFREQ_NAME_LEN];
This change is not needed as this is a private function only used in const-eval only.
I wonder if I should add another macro to assert that the function is only used in const eval instead? Do you think it might be useful to have something like:
#[const_only] const fn foo() {}
or
const fn foo() { const_only!(); }
? If so, I can send a patch that adds this feature.
Implementation-wise, this will behave similar to build_error, where a function is going to be added that is never-linked but has a body for const eval.
Best, Gary
linux-stable-mirror@lists.linaro.org
-
Alexandre Courbot -
Gary Guo