only lock params if sysfs enabled

adamrk · adamrk · commit 2e1d96b6891c · 2021-01-31T22:25:23.000+01:00
diff --git a/drivers/char/rust_example.rs b/drivers/char/rust_example.rs
@@ -55,21 +55,36 @@ struct RustExample {
     _dev: Pin<Box<miscdev::Registration>>,
 }
 
+#[cfg(CONFIG_SYSFS)]
+fn print_params() {
+    let lock = THIS_MODULE.kernel_param_lock();
+    println!("Parameters:");
+    println!("  my_bool:    {}", my_bool.nonlocking_read());
+    println!("  my_i32:     {}", my_i32.locking_read(&lock));
+    println!(
+        "  my_str:     {:?}",
+        core::str::from_utf8(my_str.locking_read(&lock))
+    );
+    println!("  my_invbool: {}", my_invbool.locking_read(&lock));
+}
+
+#[cfg(not(CONFIG_SYSFS))]
+fn print_params() {
+    println!("Parameters:");
+    println!("  my_bool:    {}", my_bool.nonlocking_read());
+    println!("  my_i32:     {}", my_i32.nonlocking_read());
+    println!(
+        "  my_str:     {:?}",
+        core::str::from_utf8(my_str.nonlocking_read())
+    );
+    println!("  my_invbool: {}", my_invbool.nonlocking_read());
+}
+
 impl KernelModule for RustExample {
     fn init() -> KernelResult<Self> {
         println!("Rust Example (init)");
         println!("Am I built-in? {}", !cfg!(MODULE));
-        {
-            let lock = THIS_MODULE.kernel_param_lock();
-            println!("Parameters:");
-            println!("  my_bool:    {}", my_bool.read());
-            println!("  my_i32:     {}", my_i32.locked_read(&lock));
-            println!(
-                "  my_str:     {:?}",
-                core::str::from_utf8(my_str.locked_read(&lock))
-            );
-            println!("  my_invbool: {}", my_invbool.locked_read(&lock));
-        }
+        print_params();
 
         // Including this large variable on the stack will trigger
         // stack probing on the supported archs.
diff --git a/drivers/char/rust_example_2.rs b/drivers/char/rust_example_2.rs
@@ -40,21 +40,36 @@ struct RustExample2 {
     message: String,
 }
 
+#[cfg(CONFIG_SYSFS)]
+fn print_params() {
+    let lock = THIS_MODULE.kernel_param_lock();
+    println!("[2] Parameters:");
+    println!("[2]   my_bool:    {}", my_bool.nonlocking_read());
+    println!("[2]   my_i32:     {}", my_i32.locking_read(&lock));
+    println!(
+        "[2]   my_str:     {:?}",
+        core::str::from_utf8(my_str.locking_read(&lock))
+    );
+    println!("[2]   my_invbool: {}", my_invbool.locking_read(&lock));
+}
+
+#[cfg(not(CONFIG_SYSFS))]
+fn print_params() {
+    println!("[2] Parameters:");
+    println!("[2]   my_bool:    {}", my_bool.nonlocking_read());
+    println!("[2]   my_i32:     {}", my_i32.nonlocking_read());
+    println!(
+        "[2]   my_str:     {:?}",
+        core::str::from_utf8(my_str.nonlocking_read())
+    );
+    println!("[2]   my_invbool: {}", my_invbool.nonlocking_read());
+}
+
 impl KernelModule for RustExample2 {
     fn init() -> KernelResult<Self> {
         println!("[2] Rust Example (init)");
         println!("[2] Am I built-in? {}", !cfg!(MODULE));
-        {
-            let lock = THIS_MODULE.kernel_param_lock();
-            println!("[2] Parameters:");
-            println!("[2]   my_bool:    {}", my_bool.read());
-            println!("[2]   my_i32:     {}", my_i32.locked_read(&lock));
-            println!(
-                "[2]   my_str:     {:?}",
-                core::str::from_utf8(my_str.locked_read(&lock))
-            );
-            println!("[2]   my_invbool: {}", my_invbool.locked_read(&lock));
-        }
+        print_params();
 
         // Including this large variable on the stack will trigger
         // stack probing on the supported archs.
diff --git a/drivers/char/rust_example_3.rs b/drivers/char/rust_example_3.rs
@@ -40,21 +40,36 @@ struct RustExample3 {
     message: String,
 }
 
+#[cfg(CONFIG_SYSFS)]
+fn print_params() {
+    let lock = THIS_MODULE.kernel_param_lock();
+    println!("[3] Parameters:");
+    println!("[3]   my_bool:    {}", my_bool.nonlocking_read());
+    println!("[3]   my_i32:     {}", my_i32.locking_read(&lock));
+    println!(
+        "[3]   my_str:     {:?}",
+        core::str::from_utf8(my_str.locking_read(&lock))
+    );
+    println!("[3]   my_invbool: {}", my_invbool.locking_read(&lock));
+}
+
+#[cfg(not(CONFIG_SYSFS))]
+fn print_params() {
+    println!("[3] Parameters:");
+    println!("[3]   my_bool:    {}", my_bool.nonlocking_read());
+    println!("[3]   my_i32:     {}", my_i32.nonlocking_read());
+    println!(
+        "[3]   my_str:     {:?}",
+        core::str::from_utf8(my_str.nonlocking_read())
+    );
+    println!("[3]   my_invbool: {}", my_invbool.nonlocking_read());
+}
+
 impl KernelModule for RustExample3 {
     fn init() -> KernelResult<Self> {
         println!("[3] Rust Example (init)");
         println!("[3] Am I built-in? {}", !cfg!(MODULE));
-        {
-            let lock = THIS_MODULE.kernel_param_lock();
-            println!("[3] Parameters:");
-            println!("[3]   my_bool:    {}", my_bool.read());
-            println!("[3]   my_i32:     {}", my_i32.locked_read(&lock));
-            println!(
-                "[3]   my_str:     {:?}",
-                core::str::from_utf8(my_str.locked_read(&lock))
-            );
-            println!("[3]   my_invbool: {}", my_invbool.locked_read(&lock));
-        }
+        print_params();
 
         // Including this large variable on the stack will trigger
         // stack probing on the supported archs.
diff --git a/drivers/char/rust_example_4.rs b/drivers/char/rust_example_4.rs
@@ -40,21 +40,36 @@ struct RustExample4 {
     message: String,
 }
 
+#[cfg(CONFIG_SYSFS)]
+fn print_params() {
+    let lock = THIS_MODULE.kernel_param_lock();
+    println!("[4] Parameters:");
+    println!("[4]   my_bool:    {}", my_bool.nonlocking_read());
+    println!("[4]   my_i32:     {}", my_i32.locking_read(&lock));
+    println!(
+        "[4]   my_str:     {:?}",
+        core::str::from_utf8(my_str.locking_read(&lock))
+    );
+    println!("[4]   my_invbool: {}", my_invbool.locking_read(&lock));
+}
+
+#[cfg(not(CONFIG_SYSFS))]
+fn print_params() {
+    println!("[4] Parameters:");
+    println!("[4]   my_bool:    {}", my_bool.nonlocking_read());
+    println!("[4]   my_i32:     {}", my_i32.nonlocking_read());
+    println!(
+        "[4]   my_str:     {:?}",
+        core::str::from_utf8(my_str.nonlocking_read())
+    );
+    println!("[4]   my_invbool: {}", my_invbool.nonlocking_read());
+}
+
 impl KernelModule for RustExample4 {
     fn init() -> KernelResult<Self> {
         println!("[4] Rust Example (init)");
         println!("[4] Am I built-in? {}", !cfg!(MODULE));
-        {
-            let lock = THIS_MODULE.kernel_param_lock();
-            println!("[4] Parameters:");
-            println!("[4]   my_bool:    {}", my_bool.read());
-            println!("[4]   my_i32:     {}", my_i32.locked_read(&lock));
-            println!(
-                "[4]   my_str:     {:?}",
-                core::str::from_utf8(my_str.locked_read(&lock))
-            );
-            println!("[4]   my_invbool: {}", my_invbool.locked_read(&lock));
-        }
+        print_params();
 
         // Including this large variable on the stack will trigger
         // stack probing on the supported archs.
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
@@ -55,6 +55,7 @@ impl ThisModule {
         ThisModule(ptr)
     }
 
+    #[cfg(CONFIG_SYSFS)]
     pub fn kernel_param_lock(&self) -> KParamGuard<'_> {
         // SAFETY: `kernel_param_lock` will check if the pointer is null and use the built-in mutex
         // in that case.
@@ -65,10 +66,12 @@ impl ThisModule {
 
 /// Scoped lock on the kernel parameters of `ThisModule`. Lock will be released
 /// when this struct is dropped.
+#[cfg(CONFIG_SYSFS)]
 pub struct KParamGuard<'a> {
     this_module: &'a ThisModule
 }
 
+#[cfg(CONFIG_SYSFS)]
 impl<'a> Drop for KParamGuard<'a> {
     fn drop(&mut self) {
         // SAFETY: `kernel_param_lock` will check if the pointer is null and use the built-in mutex
diff --git a/rust/module.rs b/rust/module.rs
@@ -199,7 +199,15 @@ fn permissions_are_readonly(perms: &str) -> bool {
 ///
 /// impl KernelModule for MyKernelModule {
 ///     fn init() -> KernelResult<Self> {
-///         println!("i32 param is:  {}", my_i32.read());
+///         // If `CONFIG_SYSFS` is enabled and the parameter is writeable, it must
+///         // the kparam lock must be taken to read the parameter:
+///         {
+///             let lock = THIS_MODULE.kernel_param_lock();
+///             println!("i32 param is:  {}", my_i32.nonlocking_read());
+///         }
+///         // If `CONFIG_SYSFS` is not enabled or the parameter is read only, it
+///         // can be read without locking the kernel parameters:
+///         println!("i32 param is:  {}", my_i32.nonlocking_read());
 ///         Ok(MyKernelModule)
 ///     }
 /// }
@@ -297,10 +305,10 @@ pub fn module(ts: TokenStream) -> TokenStream {
             "str" => format!("b\"{}\0\" as *const _ as *mut kernel::c_types::c_char", param_default),
             _ => param_default,
         };
-        let read_func = match (param_type.as_ref(), permissions_are_readonly(&param_permissions)) {
-            ("str", false) => format!(
+        let locking_read_func = match param_type.as_ref() {
+            "str" => format!(
                 "
-                    fn locked_read<'par: 'val, 'lck: 'val, 'val>(&'par self, lock: &'lck kernel::KParamGuard) -> &'val [u8] {{
+                    fn locking_read<'par: 'val, 'lck: 'val, 'val>(&'par self, lock: &'lck kernel::KParamGuard) -> &'val [u8] {{
                         // SAFETY: The pointer is provided either in `param_default` when building the module,
                         // or by the kernel through `param_set_charp`. Both will be valid C strings.
                         // Parameters are locked by `KParamGuard`.
@@ -312,12 +320,25 @@ pub fn module(ts: TokenStream) -> TokenStream {
                 name = name,
                 param_name = param_name,
             ),
-            ("str", true) => format!(
+            _ => format!(
                 "
-                    fn read(&self) -> &[u8] {{
+                    // SAFETY: Parameters are locked by `KParamGuard`.
+                    fn locking_read<'par: 'val, 'lck: 'val, 'val>(&'par self, lock: &'lck kernel::KParamGuard) -> &'val {param_type_internal} {{
+                        unsafe {{ &__{name}_{param_name}_value }}
+                    }}
+                ",
+                name = name,
+                param_name = param_name,
+                param_type_internal = param_type_internal,
+            ),
+        };
+        let nonlocking_read_func = match param_type.as_ref() {
+            "str" => format!(
+                "
+                    fn nonlocking_read(&self) -> &[u8] {{
                         // SAFETY: The pointer is provided either in `param_default` when building the module,
                         // or by the kernel through `param_set_charp`. Both will be valid C strings.
-                        // Parameters do not need to be locked because they are read only.
+                        // Parameters do not need to be locked because they are read only or sysfs is not enabled.
                         unsafe {{
                             kernel::c_types::c_string_bytes(__{name}_{param_name}_value)
                         }}
@@ -326,28 +347,36 @@ pub fn module(ts: TokenStream) -> TokenStream {
                 name = name,
                 param_name = param_name,
             ),
-            (_, false) => format!(
+            _ => format!(
                 "
-                    // SAFETY: Parameters are locked by `KParamGuard`.
-                    fn locked_read<'par: 'val, 'lck: 'val, 'val>(&'par self, lock: &'lck kernel::KParamGuard) -> &'val {param_type_internal} {{
+                    // SAFETY: Parameters do not need to be locked because they are read only or sysfs is not enabled.
+                    fn nonlocking_read(&self) -> &{param_type_internal} {{
                         unsafe {{ &__{name}_{param_name}_value }}
                     }}
                 ",
                 name = name,
                 param_name = param_name,
                 param_type_internal = param_type_internal,
             ),
-            (_, true) => format!(
+        };
+        let read_func = if permissions_are_readonly(&param_permissions) {
+            format!(
                 "
-                    // SAFETY: Parameters do not need to be locked because they are read only.
-                    fn read(&self) -> &{param_type_internal} {{
-                        unsafe {{ &__{name}_{param_name}_value }}
-                    }}
+                    {nonlocking_read_func}
                 ",
-                name = name,
-                param_name = param_name,
-                param_type_internal = param_type_internal,
-            ),
+                nonlocking_read_func = nonlocking_read_func
+            )
+        } else {
+            format!(
+                "
+                    #[cfg(CONFIG_SYSFS)]
+                    {locking_read_func}
+                    #[cfg(not(CONFIG_SYSFS))]
+                    {nonlocking_read_func}
+                ",
+                locking_read_func = locking_read_func,
+                nonlocking_read_func = nonlocking_read_func,
+            )
         };
         let kparam = format!(
             "
