rust: module_param: return value by copy from `value`
Subsystems (2)
Andreas Hindborg
reviewerPATCH v2 1/1 rust: module_param: return value by copy from `value`
For Copy parameter types it is more ergonomic to retrieve the parameter value by copy than through a shared reference. Change ModuleParamAccess::value to return T by copy when T: Copy, and rename the previous reference-returning accessor to value_ref. Update the in-tree caller in rust_minimal.
---
This change was suggested during review of the rust null block series [1].
[1] https://lore.kernel.org/r/[email protected]
---
Changes in v2:
- Move value into existing impl block (Gary).
- Add inline attributes (Sashiko).
- Link to v1: https://msgid.link/[email protected]
--- rust/kernel/module_param.rs | 18 +++++++++++++++++- samples/rust/rust_minimal.rs | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-)
rust/kernel/module_param.rs +17 -1
@@ -131,10 +131,26 @@ pub const fn new(default: T) -> Self { } } +/// Get a copy of the parameter value. +/// +/// Returns the value supplied at module load time, or the default value +/// if the parameter has not been set. +#[inline] +pub fn value (&self) ->T +where +T :Copy , + { + self.value.copy ().unwrap_or (self.default) + } +/// Get a shared reference to the parameter value. +/// +/// Returns a reference to the value supplied at module load time, or a +/// reference to the default value if the parameter has not been set. // Note: When sysfs access to parameters are enabled, we have to pass in a // held lock guard here. -pub fn value (&self) -> &T { +#[inline] +pub fn value_ref (&self) -> &T { self.value.as_ref ().unwrap_or (&self.default) }
samples/rust/rust_minimal.rs +1 -1
@@ -28,7 +28,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {pr_info! ("Am I built-in? {}\n" , !cfg! (MODULE ));pr_info! ("test_parameter: {}\n" , - *module_parameters::test_parameter.value () + module_parameters::test_parameter.value () );let mut numbers =KVec ::new ();
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260529-modules-value-ref-e95a7ab94fdb
Best regards,
Signature · 1 line
Andreas Hindborg <[email protected]>
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Casting a pointer of ModuleParamAccess<T> to SetOnce<T> invokes undefined behavior due to missing #[repr(C)] on ModuleParamAccess. --
commit 9dc35dfddf4037b5b314ed7586fa775219811371
Author: Andreas Hindborg <[email protected]>
rust: module_param: return value by copy from value
This commit updates parameter access to be more ergonomic for Copy types by returning the parameter value by copy rather than through a shared reference. It changes ModuleParamAccess::value to return T by copy when
T: Copy, and renames the previous reference-returning accessor to value_ref. The in-tree caller in rust_minimal is updated to reflect this change.
rust/kernel/module_param.rs
Quoted 4 lines
rust/kernel/module_param.rs +0
-0
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but casting the pointer to SetOnce<T> appears to rely on an unspecified memory layout.
In rust/kernel/module_param.rs:ModuleParamAccess:
pub struct ModuleParamAccess<T> {
value: SetOnce<T>,
default: T,
}
Since ModuleParamAccess lacks a #[repr(C)] annotation, the compiler is free to reorder its fields. If the default field is placed before value to minimize padding, would the cast in set_param() overwrite the default field instead?
rust/kernel/module_param.rs:set_param() {
...
// SAFETY: By function safety requirements, this access is safe.
let container = unsafe {
&*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>())
};
container
.populate(new_value)
...
}Should ModuleParamAccess be marked with #[repr(C)], or should the pointer be cast back to ModuleParamAccess<T> before accessing the value field?
Quoted 6 lines
rust/kernel/module_param.rs +2
-0@@ -131,10 +131,26 @@ pub const fn new(default: T) -> Self { } } +/// Get a copy of the parameter value. +///
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Quoted 8 lines · Andreas Hindborg
For
Copyparameter types it is more ergonomic to retrieve the parameter value by copy than through a shared reference. ChangeModuleParamAccess::valueto returnTby copy whenT: Copy, and rename the previous reference-returning accessor tovalue_ref. Update the in-tree caller inrust_minimal.Suggested-by Alice Ryhl <[email protected]>Signed-off-by Andreas Hindborg <[email protected]>
Quoted 13 lines
---
This change was suggested during review of the rust null block series [1].[1] https://lore.kernel.org/r/[email protected]
---
Changes in v2:
- Movevalueinto existing impl block (Gary).
- Add inline attributes (Sashiko).
- Link to v1: https://msgid.link/[email protected]--- rust/kernel/module_param.rs | 18 +++++++++++++++++- samples/rust/rust_minimal.rs | 2 +- 2 files changed, 18 insertions(+), 2 deletions(-)
Quoted 8 lines · Andreas Hindborg
For
Copyparameter types it is more ergonomic to retrieve the parameter value by copy than through a shared reference. ChangeModuleParamAccess::valueto returnTby copy whenT: Copy, and rename the previous reference-returning accessor tovalue_ref. Update the in-tree caller inrust_minimal.Suggested-by Alice Ryhl <[email protected]>Signed-off-by Andreas Hindborg <[email protected]>
@Miguel, please let me know if I should take this patch on modules-next, or if you'd prefer for it to go through the Rust tree.
Signature · 2 lines
Thanks, Petr
Quoted 3 lines · Petr Pavlu
@Miguel, please let me know if I should take this patch on modules-next, or if you'd prefer for it to go through the Rust tree.
Please feel free to take it through modules-next, of course.
Thanks!
Cheers,
Miguel
Petr Pavlu
maintainerQuoted 6 lines · Miguel Ojeda
Quoted 3 lines · Petr Pavlu
@Miguel, please let me know if I should take this patch on modules-next, or if you'd prefer for it to go through the Rust tree.
Please feel free to take it through modules-next, of course.
Thanks for the confirmation. Queued now on modules-next for v7.3-rc1.
-- Petr
Alice Ryhl
reviewerQuoted 9 lines · Petr Pavlu
Quoted 6 lines · Miguel Ojeda
Quoted 3 lines · Petr Pavlu
@Miguel, please let me know if I should take this patch on modules-next, or if you'd prefer for it to go through the Rust tree.
Please feel free to take it through modules-next, of course.
Thanks for the confirmation. Queued now on modules-next for v7.3-rc1.
I'm seeing this error on x86 with latest nightly rust compiler:
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramaEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_()rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_() falls through to next function _RINvNvNtCs65fqAZscXCv_6kernel6devres16register_foreign8callbackINtNtCsbtRzKR1rrUg_4core3pin3PinINtNtNtB6_5alloc4kbox3BoxNtNtNtB6_3pci3irq21IrqVectorRegistrationNtNtB1A_9allocator7KmallocEEEB6_() rust/kernel.o: error: objtool: _RNvMNtCs65fqAZscXCv_6kernel6deviceNtB2_6Device10get_device() falls through to next function _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_()rust/kernel.o: error: objtool: _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_() falls through to next function _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE21devres_node_free_nodeB4_()
I understand this is probably due to a new function in Rust stdlib that is no_return, so probably not caused by this patch even if this is triggering it.
Alice
Quoted 4 lines · Alice Ryhl
I understand this is probably due to a new function in Rust stdlib that is
no_return, so probably not caused by this patch even if this is triggering it.
I can take a look and perhaps push a patch via rust-fixes, which would make it fixed by the time this one lands in the merge window.
Cheers,
Miguel
Petr Pavlu
maintainerQuoted 25 lines · Alice Ryhl
Quoted 9 lines · Petr Pavlu
Quoted 6 lines · Miguel Ojeda
Quoted 3 lines · Petr Pavlu
@Miguel, please let me know if I should take this patch on modules-next, or if you'd prefer for it to go through the Rust tree.
Please feel free to take it through modules-next, of course.
Thanks for the confirmation. Queued now on modules-next for v7.3-rc1.
I'm seeing this error on x86 with latest nightly rust compiler:
rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramaEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_()falls through to next function_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_()rust/kernel.o: error: objtool:_RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_()falls through to next function_RINvNvNtCs65fqAZscXCv_6kernel6devres16register_foreign8callbackINtNtCsbtRzKR1rrUg_4core3pin3PinINtNtNtB6_5alloc4kbox3BoxNtNtNtB6_3pci3irq21IrqVectorRegistrationNtNtB1A_9allocator7KmallocEEEB6_()rust/kernel.o: error: objtool:_RNvMNtCs65fqAZscXCv_6kernel6deviceNtB2_6Device10get_device()falls through to next function_RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_()rust/kernel.o: error: objtool:_RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_()falls through to next function_RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE21devres_node_free_nodeB4_()
Quoted 3 lines
I understand this is probably due to a new function in Rust stdlib that is
no_return, so probably not caused by this patch even if this is triggering it.
I can't immediately reproduce this issue. I tried with linux-next (next-20260710), rustc 1.99.0-nightly (af3d95584 2026-07-09) and x86_64_defconfig with Rust enabled. What am I missing? Could you send me your config and the produced rust/kernel.o off-list?
Signature · 2 lines
Thanks, Petr
Quoted 5 lines · Petr Pavlu
I can't immediately reproduce this issue. I tried with linux-next (next-20260710), rustc 1.99.0-nightly (
af3d955842026-07-09) andx86_64_defconfigwith Rust enabled. What am I missing? Could you send me your config and the producedrust/kernel.ooff-list?
I can reproduce it independently of this patch.
It looks like panic_null_reference_constructed, added the other day -- I will send the patch.
Cheers,
Miguel
Quoted 3 lines · Miguel Ojeda
It looks like
panic_null_reference_constructed, added the other day -- I will send the patch.
https://lore.kernel.org/rust-for-linux/[email protected]/
Cheers,
Miguel