rust: module_param: return copy from value() for Copy types

[PATCH] rust: module_param: return copy from value() for Copy types

Posted by Andreas Hindborg 1 week, 4 days ago

Rename the existing `value()` method to `value_ref()` which returns a
shared reference to the parameter value, and add a new `value()`
method on `ModuleParamAccess<T>` where `T: Copy` that returns the
value by copy.

This provides a more ergonomic API for the common case where the
parameter type implements `Copy`, avoiding the need to explicitly
dereference the return value at call sites.

Currently `value_ref()` has no in-tree callers, but it will be needed
when support for non-`Copy` parameter types such as arrays and
strings is added.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
This change was suggested at [1].

Link: https://lore.kernel.org/r/87cy13swpw.fsf@t14s.mail-host-address-is-not-set [1]
---
 rust/kernel/module_param.rs  | 11 ++++++++++-
 samples/rust/rust_minimal.rs |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 6a8a7a875643..5dcfe2ba87a1 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -134,7 +134,7 @@ pub const fn new(default: T) -> Self {
     /// Get a shared reference to the parameter value.
     // 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 +146,15 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
     }
 }
 
+impl<T: Copy> ModuleParamAccess<T> {
+    /// Get a copy of the parameter value.
+    // Note: When sysfs access to parameters are enabled, we have to pass in a
+    // held lock guard here.
+    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: c369299895a591d96745d6492d4888259b004a9e
change-id: 20260323-module-value-ref-5884b5ae6b2a

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Re: [PATCH] rust: module_param: return copy from value() for Copy types

Posted by Gary Guo 1 week, 4 days ago

On Mon Mar 23, 2026 at 12:47 PM GMT, Andreas Hindborg wrote:
> Rename the existing `value()` method to `value_ref()` which returns a
> shared reference to the parameter value, and add a new `value()`
> method on `ModuleParamAccess<T>` where `T: Copy` that returns the
> value by copy.
>
> This provides a more ergonomic API for the common case where the
> parameter type implements `Copy`, avoiding the need to explicitly
> dereference the return value at call sites.
>
> Currently `value_ref()` has no in-tree callers, but it will be needed
> when support for non-`Copy` parameter types such as arrays and
> strings is added.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> This change was suggested at [1].
>
> Link: https://lore.kernel.org/r/87cy13swpw.fsf@t14s.mail-host-address-is-not-set [1]
> ---
>  rust/kernel/module_param.rs  | 11 ++++++++++-
>  samples/rust/rust_minimal.rs |  2 +-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643..5dcfe2ba87a1 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -134,7 +134,7 @@ pub const fn new(default: T) -> Self {
>      /// Get a shared reference to the parameter value.
>      // 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 +146,15 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
>      }
>  }
>  
> +impl<T: Copy> ModuleParamAccess<T> {
> +    /// Get a copy of the parameter value.
> +    // Note: When sysfs access to parameters are enabled, we have to pass in a
> +    // held lock guard here.
> +    pub fn value(&self) -> T {

It's better to keep this close to `value_ref` in the same impl block. The `T:
Copy` bound doesn't need to be on the impl block, it can be on the item itself
with

    pub fn value(&self) -> T where T: Copy

Best,
Gary

> +        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: c369299895a591d96745d6492d4888259b004a9e
> change-id: 20260323-module-value-ref-5884b5ae6b2a
>
> Best regards,

Re: [PATCH] rust: module_param: return copy from value() for Copy types

Posted by Andreas Hindborg 1 week, 4 days ago

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

> On Mon Mar 23, 2026 at 12:47 PM GMT, Andreas Hindborg wrote:
>> Rename the existing `value()` method to `value_ref()` which returns a
>> shared reference to the parameter value, and add a new `value()`
>> method on `ModuleParamAccess<T>` where `T: Copy` that returns the
>> value by copy.
>>
>> This provides a more ergonomic API for the common case where the
>> parameter type implements `Copy`, avoiding the need to explicitly
>> dereference the return value at call sites.
>>
>> Currently `value_ref()` has no in-tree callers, but it will be needed
>> when support for non-`Copy` parameter types such as arrays and
>> strings is added.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> This change was suggested at [1].
>>
>> Link: https://lore.kernel.org/r/87cy13swpw.fsf@t14s.mail-host-address-is-not-set [1]
>> ---
>>  rust/kernel/module_param.rs  | 11 ++++++++++-
>>  samples/rust/rust_minimal.rs |  2 +-
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
>> index 6a8a7a875643..5dcfe2ba87a1 100644
>> --- a/rust/kernel/module_param.rs
>> +++ b/rust/kernel/module_param.rs
>> @@ -134,7 +134,7 @@ pub const fn new(default: T) -> Self {
>>      /// Get a shared reference to the parameter value.
>>      // 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 +146,15 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
>>      }
>>  }
>>
>> +impl<T: Copy> ModuleParamAccess<T> {
>> +    /// Get a copy of the parameter value.
>> +    // Note: When sysfs access to parameters are enabled, we have to pass in a
>> +    // held lock guard here.
>> +    pub fn value(&self) -> T {
>
> It's better to keep this close to `value_ref` in the same impl block. The `T:
> Copy` bound doesn't need to be on the impl block, it can be on the item itself
> with
>
>     pub fn value(&self) -> T where T: Copy

Cool, I'll do that.


Best regards,
Andreas Hindborg

Re: [PATCH] rust: module_param: return copy from value() for Copy types

Posted by Alice Ryhl 1 week, 4 days ago

On Mon, Mar 23, 2026 at 1:48 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Rename the existing `value()` method to `value_ref()` which returns a
> shared reference to the parameter value, and add a new `value()`
> method on `ModuleParamAccess<T>` where `T: Copy` that returns the
> value by copy.
>
> This provides a more ergonomic API for the common case where the
> parameter type implements `Copy`, avoiding the need to explicitly
> dereference the return value at call sites.
>
> Currently `value_ref()` has no in-tree callers, but it will be needed
> when support for non-`Copy` parameter types such as arrays and
> strings is added.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>