* [PATCH v3 1/7] rust: build_assert: add instructions for use with function arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
@ 2025-12-08 2:46 ` Alexandre Courbot
2025-12-08 2:47 ` [PATCH v3 2/7] rust: io: always inline functions using build_assert with arguments Alexandre Courbot
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:46 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot
`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 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.
Add a paragraph instructing to annotate such functions with
`#[inline(always)]` in `build_assert`'s documentation, and split its
example to illustrate.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/build_assert.rs | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/build_assert.rs b/rust/kernel/build_assert.rs
index 6331b15d7c4d..f8124dbc663f 100644
--- a/rust/kernel/build_assert.rs
+++ b/rust/kernel/build_assert.rs
@@ -61,8 +61,13 @@ macro_rules! build_error {
/// build_assert!(N > 1); // Build-time check
/// assert!(N > 1); // Run-time check
/// }
+/// ```
///
-/// #[inline]
+/// When a condition depends on a function argument, the function must be annotated with
+/// `#[inline(always)]`. Without this attribute, the compiler may choose to not inline the
+/// function, preventing it from optimizing out the error path.
+/// ```
+/// #[inline(always)]
/// fn bar(n: usize) {
/// // `static_assert!(n > 1);` is not allowed
/// build_assert!(n > 1); // Build-time check
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 2/7] rust: io: always inline functions using build_assert with arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
2025-12-08 2:46 ` [PATCH v3 1/7] rust: build_assert: add instructions for " Alexandre Courbot
@ 2025-12-08 2:47 ` Alexandre Courbot
2025-12-08 2:47 ` [PATCH v3 3/7] rust: cpufreq: " Alexandre Courbot
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:47 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot, stable
`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)
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
2025-12-08 2:46 ` [PATCH v3 1/7] rust: build_assert: add instructions for " Alexandre Courbot
2025-12-08 2:47 ` [PATCH v3 2/7] rust: io: always inline functions using build_assert with arguments Alexandre Courbot
@ 2025-12-08 2:47 ` Alexandre Courbot
2025-12-08 13:55 ` Gary Guo
2025-12-08 2:47 ` [PATCH v3 4/7] rust: bits: " Alexandre Courbot
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:47 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot, stable
`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];
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-08 2:47 ` [PATCH v3 3/7] rust: cpufreq: " Alexandre Courbot
@ 2025-12-08 13:55 ` Gary Guo
2025-12-09 0:52 ` Alexandre Courbot
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Gary Guo @ 2025-12-08 13:55 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Rafael J. Wysocki, Viresh Kumar,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-08 13:55 ` Gary Guo
@ 2025-12-09 0:52 ` Alexandre Courbot
2025-12-09 12:02 ` Gary Guo
2025-12-09 1:01 ` Miguel Ojeda
2025-12-15 5:06 ` Viresh Kumar
2 siblings, 1 reply; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-09 0:52 UTC (permalink / raw)
To: Gary Guo, Alexandre Courbot
Cc: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Rafael J. Wysocki, Viresh Kumar,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
On Mon Dec 8, 2025 at 10:55 PM JST, Gary Guo wrote:
> 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.
... for now. :)
>
> 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.
It could be useful in the general sense, but for this particular case
the rule "if you do build_assert on a function argument, then always
inline it" also covers us in case `copy_name` gets used outside of const
context, so isn't it the preferable workaround?
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-09 0:52 ` Alexandre Courbot
@ 2025-12-09 12:02 ` Gary Guo
0 siblings, 0 replies; 17+ messages in thread
From: Gary Guo @ 2025-12-09 12:02 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Rafael J. Wysocki, Viresh Kumar,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
On Tue, 09 Dec 2025 09:52:13 +0900
"Alexandre Courbot" <acourbot@nvidia.com> wrote:
> On Mon Dec 8, 2025 at 10:55 PM JST, Gary Guo wrote:
> > 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.
>
> ... for now. :)
>
> >
> > 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.
>
> It could be useful in the general sense, but for this particular case
> the rule "if you do build_assert on a function argument, then always
> inline it" also covers us in case `copy_name` gets used outside of const
> context, so isn't it the preferable workaround?
In this particular case the `copy_name` shouldn't be used at all
outside const eval. It's specificially for building a table during
const eval. It's a bug if it's outside, hence I think
`#[inline(always)]` adds confusion to the reader of this code.
I get that you want to have a general rule of "if you're using
something with `build_assert!`, then use `#[inline(always)]`", but I
think applying that rule here is detrimental.
Hence I suggested adding a marker to indicate const-eval only function,
so we can either say const-eval-only functions are fine without inline
markers, or perhaps just use normal panicking-assertion inside these
functions (as `build_assert!` behave identical to just `assert!` in
const-eval).
Best,
Gary
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-08 13:55 ` Gary Guo
2025-12-09 0:52 ` Alexandre Courbot
@ 2025-12-09 1:01 ` Miguel Ojeda
2025-12-15 5:06 ` Viresh Kumar
2 siblings, 0 replies; 17+ messages in thread
From: Miguel Ojeda @ 2025-12-09 1:01 UTC (permalink / raw)
To: Gary Guo
Cc: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland,
rust-for-linux, linux-kernel, linux-pm, stable
On Mon, Dec 8, 2025 at 2:55 PM Gary Guo <gary@garyguo.net> wrote:
>
> ? If so, I can send a patch that adds this feature.
Sounds like `consteval` in C++20, which is useful from time to time.
If we add it, then the attribute form may make a bit more "sense"
conceptually (and we already also added the `export` one).
Cheers,
Miguel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-08 13:55 ` Gary Guo
2025-12-09 0:52 ` Alexandre Courbot
2025-12-09 1:01 ` Miguel Ojeda
@ 2025-12-15 5:06 ` Viresh Kumar
2025-12-15 11:14 ` Gary Guo
2 siblings, 1 reply; 17+ messages in thread
From: Viresh Kumar @ 2025-12-15 5:06 UTC (permalink / raw)
To: Gary Guo
Cc: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
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.
I already applied this from V2, should I drop this change ?
--
viresh
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-15 5:06 ` Viresh Kumar
@ 2025-12-15 11:14 ` Gary Guo
2025-12-16 6:48 ` Viresh Kumar
0 siblings, 1 reply; 17+ messages in thread
From: Gary Guo @ 2025-12-15 11:14 UTC (permalink / raw)
To: Viresh Kumar
Cc: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
On Mon, 15 Dec 2025 10:36:55 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 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.
>
> I already applied this from V2, should I drop this change ?
>
Thinking again about this I think `#[inline(always)]` is fine to keep as
it can also be used to indicate "this function shall never be codegenned".
However I do still think the comment is confusing per-se as there is no
"optimization" for this function at all.
RE: the patch I am fine either without this patch picked or having this
patch in and fix the comment later.
Best,
Gary
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/7] rust: cpufreq: always inline functions using build_assert with arguments
2025-12-15 11:14 ` Gary Guo
@ 2025-12-16 6:48 ` Viresh Kumar
0 siblings, 0 replies; 17+ messages in thread
From: Viresh Kumar @ 2025-12-16 6:48 UTC (permalink / raw)
To: Gary Guo
Cc: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, Daniel Almeida,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
On 15-12-25, 11:14, Gary Guo wrote:
> Thinking again about this I think `#[inline(always)]` is fine to keep as
> it can also be used to indicate "this function shall never be codegenned".
>
> However I do still think the comment is confusing per-se as there is no
> "optimization" for this function at all.
>
> RE: the patch I am fine either without this patch picked or having this
> patch in and fix the comment later.
Thanks Gary. I will keep the patch then and apply add-ons later.
--
viresh
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 4/7] rust: bits: always inline functions using build_assert with arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
` (2 preceding siblings ...)
2025-12-08 2:47 ` [PATCH v3 3/7] rust: cpufreq: " Alexandre Courbot
@ 2025-12-08 2:47 ` Alexandre Courbot
2025-12-08 2:47 ` [PATCH v3 5/7] rust: sync: refcount: " Alexandre Courbot
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:47 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot, stable
`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();
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 5/7] rust: sync: refcount: always inline functions using build_assert with arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
` (3 preceding siblings ...)
2025-12-08 2:47 ` [PATCH v3 4/7] rust: bits: " Alexandre Courbot
@ 2025-12-08 2:47 ` Alexandre Courbot
2025-12-08 2:47 ` [PATCH v3 6/7] rust: irq: " Alexandre Courbot
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:47 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot, stable
`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.
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 6/7] rust: irq: always inline functions using build_assert with arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
` (4 preceding siblings ...)
2025-12-08 2:47 ` [PATCH v3 5/7] rust: sync: refcount: " Alexandre Courbot
@ 2025-12-08 2:47 ` Alexandre Courbot
2025-12-08 2:47 ` [PATCH v3 7/7] rust: num: bounded: add missing comment for always inlined function Alexandre Courbot
2025-12-08 13:49 ` [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Gary Guo
7 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:47 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot, stable
`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)
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v3 7/7] rust: num: bounded: add missing comment for always inlined function
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
` (5 preceding siblings ...)
2025-12-08 2:47 ` [PATCH v3 6/7] rust: irq: " Alexandre Courbot
@ 2025-12-08 2:47 ` Alexandre Courbot
2025-12-08 13:49 ` [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Gary Guo
7 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-08 2:47 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Rafael J. Wysocki,
Viresh Kumar, Will Deacon, Peter Zijlstra, Mark Rutland
Cc: rust-for-linux, linux-kernel, linux-pm, Alexandre Courbot
This code is always inlined to avoid a build error if the error path of
`build_assert` cannot be optimized out. Add a comment justifying the
`#[inline(always)]` property to avoid it being taken away by mistake.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/num/bounded.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
index f870080af8ac..6dfcf9c1a55a 100644
--- a/rust/kernel/num/bounded.rs
+++ b/rust/kernel/num/bounded.rs
@@ -363,6 +363,7 @@ pub fn try_new(value: T) -> Option<Self> {
/// assert_eq!(Bounded::<u8, 1>::from_expr(1).get(), 1);
/// assert_eq!(Bounded::<u16, 8>::from_expr(0xff).get(), 0xff);
/// ```
+ // Always inline to optimize out error path of `build_assert`.
#[inline(always)]
pub fn from_expr(expr: T) -> Self {
crate::build_assert!(
--
2.52.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments
2025-12-08 2:46 [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Alexandre Courbot
` (6 preceding siblings ...)
2025-12-08 2:47 ` [PATCH v3 7/7] rust: num: bounded: add missing comment for always inlined function Alexandre Courbot
@ 2025-12-08 13:49 ` Gary Guo
2025-12-09 0:54 ` Alexandre Courbot
7 siblings, 1 reply; 17+ messages in thread
From: Gary Guo @ 2025-12-08 13:49 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Rafael J. Wysocki, Viresh Kumar,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
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.com/
> [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.com
>
> 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.com
>
> ---
> 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,
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments
2025-12-08 13:49 ` [PATCH v3 0/7] rust: build_assert: document and fix use with function arguments Gary Guo
@ 2025-12-09 0:54 ` Alexandre Courbot
0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Courbot @ 2025-12-09 0:54 UTC (permalink / raw)
To: Gary Guo, Alexandre Courbot
Cc: Danilo Krummrich, Alice Ryhl, Daniel Almeida, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Rafael J. Wysocki, Viresh Kumar,
Will Deacon, Peter Zijlstra, Mark Rutland, rust-for-linux,
linux-kernel, linux-pm, stable
On Mon Dec 8, 2025 at 10:49 PM JST, Gary Guo wrote:
> 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?
Precisely. And sometimes just moving the code invoking the
not-always-inlined function that invokes build_assert is enough to fix
it, so I don't know of an accurately reproducible pattern. It also
occurs pretty rarely.
^ permalink raw reply [flat|nested] 17+ messages in thread