[v1] rust: module_param: return value by copy from `value`

[PATCH] rust: module_param: return value by copy from `value`

Posted by Andreas Hindborg 1 week, 3 days ago

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`.

Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
This change was suggested during review of the rust null block series [1].

[1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
---
 rust/kernel/module_param.rs  | 15 ++++++++++++++-
 samples/rust/rust_minimal.rs |  2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 6a8a7a875643..701210206bba 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
     }
 
     /// 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 {
+    pub fn value_ref(&self) -> &T {
         self.value.as_ref().unwrap_or(&self.default)
     }
 
@@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
     }
 }
 
+impl<T: Copy> ModuleParamAccess<T> {
+    /// 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.
+    pub fn value(&self) -> T {
+        self.value.copy().unwrap_or(self.default)
+    }
+}
+
 #[doc(hidden)]
 /// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
 ///
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 8eb9583571d7..60d03df6cd80 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -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,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Re: [PATCH] rust: module_param: return value by copy from `value`

Posted by Gary Guo 1 week, 2 days ago

On Fri May 29, 2026 at 9:41 AM BST, Andreas Hindborg wrote:
> 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`.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> This change was suggested during review of the rust null block series [1].
>
> [1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
> ---
>  rust/kernel/module_param.rs  | 15 ++++++++++++++-
>  samples/rust/rust_minimal.rs |  2 +-
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643..701210206bba 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
>      }
>  
>      /// 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 {
> +    pub fn value_ref(&self) -> &T {
>          self.value.as_ref().unwrap_or(&self.default)
>      }
>  
> @@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
>      }
>  }
>  
> +impl<T: Copy> ModuleParamAccess<T> {

Instead of adding a new impl block, this should just be moved to immediately
after (or before) `value_ref` with a `where T: Copy` bound instead.

This makes related code close together for readers of the code.

Thanks,
Gary

> +    /// 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.
> +    pub fn value(&self) -> T {
> +        self.value.copy().unwrap_or(self.default)
> +    }
> +}
> +
>  #[doc(hidden)]
>  /// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
>  ///
> diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
> index 8eb9583571d7..60d03df6cd80 100644
> --- a/samples/rust/rust_minimal.rs
> +++ b/samples/rust/rust_minimal.rs
> @@ -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,

Re: [PATCH] rust: module_param: return value by copy from `value`

Posted by Andreas Hindborg 1 week ago

"Gary Guo" <gary@garyguo.net> writes:

> On Fri May 29, 2026 at 9:41 AM BST, Andreas Hindborg wrote:
>> 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`.
>>
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> This change was suggested during review of the rust null block series [1].
>>
>> [1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
>> ---
>>  rust/kernel/module_param.rs  | 15 ++++++++++++++-
>>  samples/rust/rust_minimal.rs |  2 +-
>>  2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
>> index 6a8a7a875643..701210206bba 100644
>> --- a/rust/kernel/module_param.rs
>> +++ b/rust/kernel/module_param.rs
>> @@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
>>      }
>>  
>>      /// 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 {
>> +    pub fn value_ref(&self) -> &T {
>>          self.value.as_ref().unwrap_or(&self.default)
>>      }
>>  
>> @@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
>>      }
>>  }
>>  
>> +impl<T: Copy> ModuleParamAccess<T> {
>
> Instead of adding a new impl block, this should just be moved to immediately
> after (or before) `value_ref` with a `where T: Copy` bound instead.
>
> This makes related code close together for readers of the code.

Will do.


Best regards,
Andreas Hindborg