From ece207a83e464af710d641f29e32b7a144c48e79 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Thu, 26 Sep 2024 14:47:51 +0200 Subject: rust: kernel: sort Rust modules Rust modules are intended to be sorted, thus do so. This makes `rustfmtcheck` to pass again. Fixes: 570172569238 ("Merge tag 'rust-6.12' of https://github.com/Rust-for-Linux/linux") Reviewed-by: Alice Ryhl Link: https://lore.kernel.org/r/20240926124751.345471-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'rust/kernel') diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 22a3bfa5a9e9..b5f4b3ce6b48 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -44,8 +44,8 @@ pub mod net; pub mod page; pub mod prelude; pub mod print; -pub mod sizes; pub mod rbtree; +pub mod sizes; mod static_assert; #[doc(hidden)] pub mod std_vendor; -- cgit v1.2.3 From a8ee30f45d5d57467ddb7877ed6914d0eba0af7f Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Sun, 15 Sep 2024 14:41:28 +0000 Subject: rust: sync: require `T: Sync` for `LockedBy::access` The `LockedBy::access` method only requires a shared reference to the owner, so if we have shared access to the `LockedBy` from several threads at once, then two threads could call `access` in parallel and both obtain a shared reference to the inner value. Thus, require that `T: Sync` when calling the `access` method. An alternative is to require `T: Sync` in the `impl Sync for LockedBy`. This patch does not choose that approach as it gives up the ability to use `LockedBy` with `!Sync` types, which is okay as long as you only use `access_mut`. Cc: stable@vger.kernel.org Fixes: 7b1f55e3a984 ("rust: sync: introduce `LockedBy`") Signed-off-by: Alice Ryhl Suggested-by: Boqun Feng Reviewed-by: Gary Guo Link: https://lore.kernel.org/r/20240915-locked-by-sync-fix-v2-1-1a8d89710392@google.com Signed-off-by: Miguel Ojeda --- rust/kernel/sync/locked_by.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/sync/locked_by.rs b/rust/kernel/sync/locked_by.rs index babc731bd5f6..ce2ee8d87865 100644 --- a/rust/kernel/sync/locked_by.rs +++ b/rust/kernel/sync/locked_by.rs @@ -83,8 +83,12 @@ pub struct LockedBy { // SAFETY: `LockedBy` can be transferred across thread boundaries iff the data it protects can. unsafe impl Send for LockedBy {} -// SAFETY: `LockedBy` serialises the interior mutability it provides, so it is `Sync` as long as the -// data it protects is `Send`. +// SAFETY: If `T` is not `Sync`, then parallel shared access to this `LockedBy` allows you to use +// `access_mut` to hand out `&mut T` on one thread at the time. The requirement that `T: Send` is +// sufficient to allow that. +// +// If `T` is `Sync`, then the `access` method also becomes available, which allows you to obtain +// several `&T` from several threads at once. However, this is okay as `T` is `Sync`. unsafe impl Sync for LockedBy {} impl LockedBy { @@ -118,7 +122,10 @@ impl LockedBy { /// /// Panics if `owner` is different from the data protected by the lock used in /// [`new`](LockedBy::new). - pub fn access<'a>(&'a self, owner: &'a U) -> &'a T { + pub fn access<'a>(&'a self, owner: &'a U) -> &'a T + where + T: Sync, + { build_assert!( size_of::() > 0, "`U` cannot be a ZST because `owner` wouldn't be unique" @@ -127,7 +134,10 @@ impl LockedBy { panic!("mismatched owners"); } - // SAFETY: `owner` is evidence that the owner is locked. + // SAFETY: `owner` is evidence that there are only shared references to the owner for the + // duration of 'a, so it's not possible to use `Self::access_mut` to obtain a mutable + // reference to the inner value that aliases with this shared reference. The type is `Sync` + // so there are no other requirements. unsafe { &*self.data.get() } } -- cgit v1.2.3 From 05cef2c4a421ca09ab9761a95e61423e59e5bfb1 Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Fri, 27 Sep 2024 18:44:14 +0200 Subject: rust: kunit: use C-string literals to clean warning Starting with upstream Rust commit a5e3a3f9b6bd ("move `manual_c_str_literals` to complexity"), to be released in Rust 1.83.0 [1], Clippy now warns on `manual_c_str_literals` by default, e.g.: error: manually constructing a nul-terminated string --> rust/kernel/kunit.rs:21:13 | 21 | b"\x013%pA\0".as_ptr() as _, | ^^^^^^^^^^^^^ help: use a `c""` literal: `c"\x013%pA"` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals = note: `-D clippy::manual-c-str-literals` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::manual_c_str_literals)]` Apply the suggestion to clean up the warnings. Link: https://github.com/rust-lang/rust-clippy/pull/13263 [1] Reviewed-by: Trevor Gross Reviewed-by: Benno Lossin Link: https://lore.kernel.org/r/20240927164414.560906-1-ojeda@kernel.org Signed-off-by: Miguel Ojeda --- rust/kernel/kunit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs index 0ba77276ae7e..824da0e9738a 100644 --- a/rust/kernel/kunit.rs +++ b/rust/kernel/kunit.rs @@ -18,7 +18,7 @@ pub fn err(args: fmt::Arguments<'_>) { #[cfg(CONFIG_PRINTK)] unsafe { bindings::_printk( - b"\x013%pA\0".as_ptr() as _, + c"\x013%pA".as_ptr() as _, &args as *const _ as *const c_void, ); } @@ -34,7 +34,7 @@ pub fn info(args: fmt::Arguments<'_>) { #[cfg(CONFIG_PRINTK)] unsafe { bindings::_printk( - b"\x016%pA\0".as_ptr() as _, + c"\x016%pA".as_ptr() as _, &args as *const _ as *const c_void, ); } -- cgit v1.2.3 From cc4332afb5631b0e9d2ce5699b7f4b7caf743526 Mon Sep 17 00:00:00 2001 From: Guilherme Giacomo Simoes Date: Tue, 1 Oct 2024 17:56:03 -0300 Subject: rust: device: change the from_raw() function The function Device::from_raw() increments a refcount by a call to bindings::get_device(ptr). This can be confused because usually from_raw() functions don't increment a refcount. Hence, rename Device::from_raw() to avoid confuion with other "from_raw" semantics. The new name of function should be "get_device" to be consistent with the function get_device() already exist in .c files. This function body also changed, because the `into()` will convert the `&'a Device` into `ARef` and also call `inc_ref` from the `AlwaysRefCounted` trait implemented for Device. Signed-off-by: Guilherme Giacomo Simoes Acked-by: Danilo Krummrich Closes: https://github.com/Rust-for-Linux/linux/issues/1088 Reviewed-by: Boqun Feng Link: https://lore.kernel.org/r/20241001205603.106278-1-trintaeoitogc@gmail.com Signed-off-by: Greg Kroah-Hartman --- rust/kernel/device.rs | 15 +++------------ rust/kernel/firmware.rs | 2 +- 2 files changed, 4 insertions(+), 13 deletions(-) (limited to 'rust/kernel') diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs index 851018eef885..c8199ee079ef 100644 --- a/rust/kernel/device.rs +++ b/rust/kernel/device.rs @@ -51,18 +51,9 @@ impl Device { /// /// It must also be ensured that `bindings::device::release` can be called from any thread. /// While not officially documented, this should be the case for any `struct device`. - pub unsafe fn from_raw(ptr: *mut bindings::device) -> ARef { - // SAFETY: By the safety requirements, ptr is valid. - // Initially increase the reference count by one to compensate for the final decrement once - // this newly created `ARef` instance is dropped. - unsafe { bindings::get_device(ptr) }; - - // CAST: `Self` is a `repr(transparent)` wrapper around `bindings::device`. - let ptr = ptr.cast::(); - - // SAFETY: `ptr` is valid by the safety requirements of this function. By the above call to - // `bindings::get_device` we also own a reference to the underlying `struct device`. - unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(ptr)) } + pub unsafe fn get_device(ptr: *mut bindings::device) -> ARef { + // SAFETY: By the safety requirements ptr is valid + unsafe { Self::as_ref(ptr) }.into() } /// Obtain the raw `struct device *`. diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs index dee5b4b18aec..13a374a5cdb7 100644 --- a/rust/kernel/firmware.rs +++ b/rust/kernel/firmware.rs @@ -44,7 +44,7 @@ impl FwFunc { /// /// # fn no_run() -> Result<(), Error> { /// # // SAFETY: *NOT* safe, just for the example to get an `ARef` instance -/// # let dev = unsafe { Device::from_raw(core::ptr::null_mut()) }; +/// # let dev = unsafe { Device::get_device(core::ptr::null_mut()) }; /// /// let fw = Firmware::request(c_str!("path/to/firmware.bin"), &dev)?; /// let blob = fw.data(); -- cgit v1.2.3