rust/kernel/Kconfig.test | 10 +++ rust/kernel/block/mq/gen_disk.rs | 11 +-- rust/kernel/mem.rs | 123 +++++++++++++++++++++++++++++++ rust/kernel/serdev.rs | 10 +-- rust/kernel/sync/lock.rs | 5 +- 5 files changed, 146 insertions(+), 13 deletions(-)
Add a `DropGuard` type to the Rust kernel memory module for running
cleanup code when a scope is left.
`DropGuard` runs a `FnOnce` callback when dropped and provides
`dismiss()` to take the wrapped value without running the callback.
Replace the current `ScopeGuard` users in `gen_disk`, `serdev`, and
`sync::lock` with `DropGuard`. Keep `ScopeGuard` since it is still used
elsewhere.
Add KUnit tests for cleanup on drop and `dismiss()`.
Tested with:
- `make LLVM=1 -j$(nproc)`
- KUnit: 8 tests passed
- `make LLVM=1 rustfmtcheck`
- `git diff --check`
- `checkpatch.pl`
- `make LLVM=1 rustdoc`
Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://github.com/Rust-for-Linux/linux/issues/1255
Signed-off-by: Mohamed Osama <mohamed.osama189110@gmail.com>
---
rust/kernel/Kconfig.test | 10 +++
rust/kernel/block/mq/gen_disk.rs | 11 +--
rust/kernel/mem.rs | 123 +++++++++++++++++++++++++++++++
rust/kernel/serdev.rs | 10 +--
rust/kernel/sync/lock.rs | 5 +-
5 files changed, 146 insertions(+), 13 deletions(-)
diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
index e6a5c7a795f0..011c72f14e2c 100644
--- a/rust/kernel/Kconfig.test
+++ b/rust/kernel/Kconfig.test
@@ -33,6 +33,16 @@ config RUST_KVEC_KUNIT_TEST
If unsure, say N.
+config RUST_DROP_GUARD_KUNIT_TEST
+ bool "KUnit tests for Rust DropGuard API" if !KUNIT_ALL_TESTS
+ default KUNIT_ALL_TESTS
+ help
+ This option enables KUnit tests for the Rust DropGuard API.
+ These are only for development and testing, not for regular
+ kernel use cases.
+
+ If unsure, say N.
+
config RUST_BITMAP_KUNIT_TEST
bool "KUnit tests for Rust bitmap API" if !KUNIT_ALL_TESTS
default KUNIT_ALL_TESTS
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index fc97dd873974..d9019fbbb361 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -10,11 +10,12 @@
block::mq::{Operations, TagSet},
error::{self, from_err_ptr, Result},
fmt::{self, Write},
+ mem::DropGuard,
prelude::*,
static_lock_class,
str::NullTerminatedFormatter,
sync::Arc,
- types::{ForeignOwnable, ScopeGuard},
+ types::ForeignOwnable,
};
/// A builder for [`GenDisk`].
@@ -102,7 +103,7 @@ pub fn build<T: Operations>(
queue_data: T::QueueData,
) -> Result<GenDisk<T>> {
let data = queue_data.into_foreign();
- let recover_data = ScopeGuard::new(|| {
+ let recover_data = DropGuard::new((), |_| {
// SAFETY: T::QueueData was created by the call to `into_foreign()` above
drop(unsafe { T::QueueData::from_foreign(data) });
});
@@ -150,7 +151,7 @@ pub fn build<T: Operations>(
// SAFETY: `gendisk` is a valid pointer as we initialized it above
unsafe { (*gendisk).fops = &TABLE };
- let cleanup_failure = ScopeGuard::new_with_data((gendisk, data), |(gendisk, data)| {
+ let cleanup_failure = DropGuard::new((gendisk, data), |(gendisk, data)| {
// SAFETY: `gendisk` came from `__blk_mq_alloc_disk()` above and
// has not been added to the VFS on this cleanup path.
unsafe { bindings::put_disk(gendisk) };
@@ -161,7 +162,7 @@ pub fn build<T: Operations>(
// The failure guard now owns both pieces of cleanup; the early guard
// must not run on this path anymore.
- recover_data.dismiss();
+ DropGuard::dismiss(recover_data);
let mut writer = NullTerminatedFormatter::new(
// SAFETY: `gendisk` points to a valid and initialized instance. We
@@ -185,7 +186,7 @@ pub fn build<T: Operations>(
},
)?;
- cleanup_failure.dismiss();
+ DropGuard::dismiss(cleanup_failure);
// INVARIANT: `gendisk` was initialized above.
// INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
diff --git a/rust/kernel/mem.rs b/rust/kernel/mem.rs
index f2d4cdf87d00..17807c8e67ac 100644
--- a/rust/kernel/mem.rs
+++ b/rust/kernel/mem.rs
@@ -4,6 +4,95 @@
use crate::prelude::*;
+use core::{
+ mem::ManuallyDrop,
+ ops::{Deref, DerefMut},
+};
+
+/// Wraps a value and runs a closure when dropped.
+///
+/// This is useful for running cleanup code when leaving a scope.
+///
+/// The [`DropGuard::dismiss`] function can be used to take ownership of the wrapped
+/// value without running the cleanup function.
+#[doc(alias = "ScopeGuard")]
+#[doc(alias = "defer")]
+pub struct DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ inner: ManuallyDrop<T>,
+ f: ManuallyDrop<F>,
+}
+
+impl<T, F> DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ /// Creates a new `DropGuard`.
+ #[inline]
+ #[must_use]
+ pub fn new(inner: T, f: F) -> Self {
+ Self {
+ inner: ManuallyDrop::new(inner),
+ f: ManuallyDrop::new(f),
+ }
+ }
+
+ /// Consumes the `DropGuard`, returning the wrapped value without
+ /// running the cleanup function.
+ #[inline]
+ pub fn dismiss(guard: Self) -> T {
+ let mut guard = ManuallyDrop::new(guard);
+
+ // SAFETY: We have taken ownership of the guard and prevent its destructor from running.
+ let value = unsafe { ManuallyDrop::take(&mut guard.inner) };
+
+ // SAFETY: We have taken ownership of the guard.
+ unsafe { ManuallyDrop::drop(&mut guard.f) };
+
+ value
+ }
+}
+
+impl<T, F> Deref for DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ type Target = T;
+
+ #[inline]
+ fn deref(&self) -> &T {
+ &self.inner
+ }
+}
+
+impl<T, F> DerefMut for DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ #[inline]
+ fn deref_mut(&mut self) -> &mut T {
+ &mut self.inner
+ }
+}
+
+impl<T, F> Drop for DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ #[inline]
+ fn drop(&mut self) {
+ // SAFETY: `DropGuard` is in the process of being dropped.
+ let inner = unsafe { ManuallyDrop::take(&mut self.inner) };
+
+ // SAFETY: `DropGuard` is in the process of being dropped.
+ let f = unsafe { ManuallyDrop::take(&mut self.f) };
+
+ f(inner);
+ }
+}
+
/// Transmute between two types.
///
/// Use this instead of [`core::mem::transmute`] when it is known that sizes are identical but this
@@ -232,3 +321,37 @@ unsafe impl AsReprMut for $signed {}
// `usize` is not normalized to particular integer for portability.
usize isize,
}
+
+#[cfg(CONFIG_RUST_DROP_GUARD_KUNIT_TEST)]
+#[macros::kunit_tests(rust_drop_guard)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn test_drop_runs_cleanup() {
+ let mut cleaned = false;
+
+ {
+ let _guard = DropGuard::new(42, |value| {
+ assert_eq!(value, 42);
+ cleaned = true;
+ });
+ }
+
+ assert!(cleaned);
+ }
+
+ #[test]
+ fn test_dismiss_returns_value_without_cleanup() {
+ let mut cleaned = false;
+
+ let guard = DropGuard::new(42, |_| {
+ cleaned = true;
+ });
+
+ let value = DropGuard::dismiss(guard);
+
+ assert_eq!(value, 42);
+ assert!(!cleaned);
+ }
+}
diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..dd43b159b461 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -13,6 +13,7 @@
to_result,
VTABLE_DEFAULT_ERROR, //
},
+ mem::DropGuard,
new_mutex,
of,
prelude::*,
@@ -21,10 +22,7 @@
Mutex, //
},
time::Jiffies,
- types::{
- Opaque,
- ScopeGuard, //
- }, //
+ types::Opaque, //
};
use core::{
@@ -174,7 +172,7 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
}))?;
// SAFETY: We just set drvdata to `PrivateData<'_, T>`.
let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
- let private_data = ScopeGuard::new_with_data(private_data, |_| {
+ let private_data = DropGuard::new(private_data, |_| {
// SAFETY: We just set drvdata to `PrivateData<'_, T>`.
drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
});
@@ -204,7 +202,7 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
drop(active);
result.map(|()| {
- private_data.dismiss();
+ DropGuard::dismiss(private_data);
0
})
})
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 10b6b5e9b024..15f9cbe76c8d 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -7,8 +7,9 @@
use super::LockClassKey;
use crate::{
+ mem::DropGuard,
str::{CStr, CStrExt as _},
- types::{NotThreadSafe, Opaque, ScopeGuard},
+ types::{NotThreadSafe, Opaque},
};
use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
use pin_init::{pin_data, pin_init, PinInit, Wrapper};
@@ -242,7 +243,7 @@ pub(crate) fn do_unlocked<U>(&mut self, cb: impl FnOnce() -> U) -> U {
// SAFETY: The caller owns the lock, so it is safe to unlock it.
unsafe { B::unlock(self.lock.state.get(), &self.state) };
- let _relock = ScopeGuard::new(||
+ let _relock = DropGuard::new((), |_|
// SAFETY: The lock was just unlocked above and is being relocked now.
unsafe { B::relock(self.lock.state.get(), &mut self.state) });
--
2.43.0© 2016 - 2026 Red Hat, Inc.