* [PATCH] rust: mem: add DropGuard
@ 2026-09-16 1:15 Mohamed Osama
2026-09-16 1:24 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Mohamed Osama @ 2026-09-16 1:15 UTC (permalink / raw)
To: rust-for-linux
Cc: linux-block, linux-serial, linux-kernel, Mohamed Osama, Gary Guo
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 | 116 +++++++++++++++++++++++++++++++
rust/kernel/serdev.rs | 8 +--
rust/kernel/sync/lock.rs | 5 +-
5 files changed, 138 insertions(+), 12 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..e098675ea195 100644
--- a/rust/kernel/mem.rs
+++ b/rust/kernel/mem.rs
@@ -4,6 +4,88 @@
use crate::prelude::*;
+use core::mem::ManuallyDrop;
+use core::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`.
+ #[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.
+ 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;
+
+ fn deref(&self) -> &T {
+ &self.inner
+ }
+}
+
+impl<T, F> DerefMut for DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ fn deref_mut(&mut self) -> &mut T {
+ &mut self.inner
+ }
+}
+
+impl<T, F> Drop for DropGuard<T, F>
+where
+ F: FnOnce(T),
+{
+ 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 +314,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..a400d241c2ad 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>>() });
});
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] rust: mem: add DropGuard
2026-09-16 1:15 [PATCH] rust: mem: add DropGuard Mohamed Osama
@ 2026-09-16 1:24 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-09-16 1:24 UTC (permalink / raw)
To: Mohamed Osama; +Cc: gary, linux-serial, ojeda
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] rust: missing #[inline] annotations on DropGuard methods
- [Low] serdev: compilation error due to method call syntax on associated function
- [Low] rust: non-compliant vertical import style in mem.rs
--
commit 3aca124644cba985e548a767bb3361277176f547
Author: Mohamed Osama <mohamed.osama189110@gmail.com>
rust: mem: add DropGuard
Add a DropGuard type to the Rust kernel memory module for running
cleanup code when a scope is left. It runs a FnOnce callback when dropped
and provides dismiss() to take the wrapped value without running the callback.
Users in gen_disk, serdev, and sync::lock are updated to use DropGuard.
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>
> diff --git a/rust/kernel/mem.rs b/rust/kernel/mem.rs
> index f2d4cdf87d00b..e098675ea1957 100644
> --- a/rust/kernel/mem.rs
> +++ b/rust/kernel/mem.rs
> @@ -4,6 +4,88 @@
>
> use crate::prelude::*;
>
> +use core::mem::ManuallyDrop;
> +use core::ops::{Deref, DerefMut};
[Severity: Low]
Does this deviate from the required vertical block formatting?
The newly added imports in rust/kernel/mem.rs are horizontal and
split across multiple statements. The Rust subsystem guidelines mandate that
commits touching imports should follow the vertical import style (e.g.,
grouping them vertically under a single use core::{...}; block).
> +
> +/// 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`.
> + #[must_use]
> + pub fn new(inner: T, f: F) -> Self {
[Severity: Low]
Should small wrapper functions in abstractions like the kernel crate use
#[inline]?
The DropGuard::new() constructor lacks this annotation. Missing #[inline]
annotations can prevent the compiler from optimizing away wrapper overhead
when calling small wrapper methods.
> + Self {
> + inner: ManuallyDrop::new(inner),
> + f: ManuallyDrop::new(f),
> + }
> + }
> +
> + /// Consumes the `DropGuard`, returning the wrapped value without
> + /// running the cleanup function.
> + pub fn dismiss(guard: Self) -> T {
[Severity: Low]
Is this trivial state-transition function missing the #[inline] annotation
as well?
The DropGuard::dismiss() function in rust/kernel/mem.rs lacks #[inline],
which may prevent optimization.
> + 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;
> +
> + fn deref(&self) -> &T {
[Severity: Low]
Should #[inline] be added to this trivial trait method?
The DropGuard::deref() method is a thin wrapper and missing #[inline].
> + &self.inner
> + }
> +}
> +
> +impl<T, F> DerefMut for DropGuard<T, F>
> +where
> + F: FnOnce(T),
> +{
> + fn deref_mut(&mut self) -> &mut T {
[Severity: Low]
Should #[inline] be added to deref_mut() for the same reason?
The DropGuard::deref_mut() method in rust/kernel/mem.rs is missing
#[inline].
> + &mut self.inner
> + }
> +}
> +
> +impl<T, F> Drop for DropGuard<T, F>
> +where
> + F: FnOnce(T),
> +{
> + fn drop(&mut self) {
[Severity: Low]
Does this drop implementation acting as a thin wrapper also need #[inline]?
The DropGuard::drop() method lacks #[inline], which may prevent optimization
when dropping the guard.
[ ... ]
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d6..a400d241c2ad6 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
[ ... ]
> @@ -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>>() });
> });
[Severity: Low]
Does this break the build when CONFIG_RUST_SERDEV is enabled?
The patch replaces ScopeGuard with DropGuard, but further down in
probe_callback() in rust/kernel/serdev.rs, there is an unmodified call to
private_data.dismiss():
rust/kernel/serdev.rs:probe_callback() {
...
result.map(|()| {
private_data.dismiss();
0
})
}
Since DropGuard::dismiss() is an associated function rather than a method
taking self, private_data.dismiss() is now invalid method call syntax and
fails to compile. Should this call be updated to
DropGuard::dismiss(private_data)?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916011500.529743-1-mohamed.osama189110@gmail.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-16 1:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-16 1:15 [PATCH] rust: mem: add DropGuard Mohamed Osama
2026-09-16 1:24 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.