From 98f5852ba1f7bc80f0f575fa9175ceb19aa5c8d5 Mon Sep 17 00:00:00 2001 From: Asahi Lina Date: Sat, 27 May 2023 21:22:03 +0900 Subject: [PATCH] rust: sync: arc: Add lockdep integration Signed-off-by: Asahi Lina --- lib/Kconfig.debug | 8 +++++ rust/kernel/init.rs | 6 ++++ rust/kernel/sync/arc.rs | 71 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 82 insertions(+), 3 deletions(-) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 39d1d93164bd09..5e5919cfa1cab3 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2880,6 +2880,14 @@ config RUST_BUILD_ASSERT_ALLOW If unsure, say N. +config RUST_EXTRA_LOCKDEP + bool "Extra lockdep checking" + depends on RUST && PROVE_LOCKING + help + Enabled additional lockdep integration with certain Rust types. + + If unsure, say N. + endmenu # "Rust" endmenu # Kernel hacking diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs index 2b7fa770efa2cb..b2bee8660704e5 100644 --- a/rust/kernel/init.rs +++ b/rust/kernel/init.rs @@ -1489,6 +1489,7 @@ pub trait InPlaceInit: Sized { /// type. /// /// If `T: !Unpin` it will not be able to move afterwards. + #[track_caller] fn try_pin_init(init: impl PinInit) -> Result, E> where E: From; @@ -1497,6 +1498,7 @@ pub trait InPlaceInit: Sized { /// type. /// /// If `T: !Unpin` it will not be able to move afterwards. + #[track_caller] fn pin_init(init: impl PinInit) -> error::Result> where Error: From, @@ -1509,11 +1511,13 @@ pub trait InPlaceInit: Sized { } /// Use the given initializer to in-place initialize a `T`. + #[track_caller] fn try_init(init: impl Init) -> Result where E: From; /// Use the given initializer to in-place initialize a `T`. + #[track_caller] fn init(init: impl Init) -> error::Result where Error: From, @@ -1558,6 +1562,7 @@ impl InPlaceInit for Box { impl InPlaceInit for UniqueArc { #[inline] + #[track_caller] fn try_pin_init(init: impl PinInit) -> Result, E> where E: From, @@ -1572,6 +1577,7 @@ impl InPlaceInit for UniqueArc { } #[inline] + #[track_caller] fn try_init(init: impl Init) -> Result where E: From, diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index e6d20624246529..379f742638355a 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -34,6 +34,9 @@ use core::{ }; use macros::pin_data; +#[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] +use crate::sync::lockdep::LockdepMap; + mod std_vendor; /// A reference-counted pointer to an instance of `T`. @@ -127,6 +130,17 @@ pub struct Arc { _p: PhantomData>, } +#[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] +#[pin_data] +#[repr(C)] +struct ArcInner { + refcount: Opaque, + lockdep_map: LockdepMap, + data: T, +} + +// FIXME: pin_data does not work well with cfg attributes within the struct definition. +#[cfg(not(CONFIG_RUST_EXTRA_LOCKDEP))] #[pin_data] #[repr(C)] struct ArcInner { @@ -157,11 +171,14 @@ unsafe impl Sync for Arc {} impl Arc { /// Constructs a new reference counted instance of `T`. + #[track_caller] pub fn try_new(contents: T) -> Result { // INVARIANT: The refcount is initialised to a non-zero value. let value = ArcInner { // SAFETY: There are no safety requirements for this FFI call. refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }), + #[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] + lockdep_map: LockdepMap::new(), data: contents, }; @@ -176,6 +193,7 @@ impl Arc { /// /// If `T: !Unpin` it will not be able to move afterwards. #[inline] + #[track_caller] pub fn pin_init(init: impl PinInit) -> error::Result where Error: From, @@ -187,6 +205,7 @@ impl Arc { /// /// This is equivalent to [`pin_init`], since an [`Arc`] is always pinned. #[inline] + #[track_caller] pub fn init(init: impl Init) -> error::Result where Error: From, @@ -279,15 +298,46 @@ impl Drop for Arc { // freed/invalid memory as long as it is never dereferenced. let refcount = unsafe { self.ptr.as_ref() }.refcount.get(); + // SAFETY: By the type invariant, there is necessarily a reference to the object. + // We cannot hold the map lock across the reference decrement, as we might race + // another thread. Therefore, we lock and immediately drop the guard here. This + // only serves to inform lockdep of the dependency up the call stack. + #[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] + unsafe { self.ptr.as_ref() }.lockdep_map.lock(); + // INVARIANT: If the refcount reaches zero, there are no other instances of `Arc`, and // this instance is being dropped, so the broken invariant is not observable. // SAFETY: Also by the type invariant, we are allowed to decrement the refcount. let is_zero = unsafe { bindings::refcount_dec_and_test(refcount) }; + if is_zero { // The count reached zero, we must free the memory. - // - // SAFETY: The pointer was initialised from the result of `Box::leak`. - unsafe { Box::from_raw(self.ptr.as_ptr()) }; + + // SAFETY: If we get this far, we had the last reference to the object. + // That means we are responsible for freeing it, so we can safely lock + // the fake lock again. This wraps dropping the inner object, which + // informs lockdep of the dependencies down the call stack. + #[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] + let guard = unsafe { self.ptr.as_ref() }.lockdep_map.lock(); + + // SAFETY: The pointer was initialised from the result of `Box::leak`, + // and the value is valid. + unsafe { core::ptr::drop_in_place(&mut self.ptr.as_mut().data) }; + + // We need to drop the lock guard before freeing the LockdepMap itself + #[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] + core::mem::drop(guard); + + // SAFETY: The pointer was initialised from the result of `Box::leak`, + // and the lockdep map is valid. + #[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] + unsafe { + core::ptr::drop_in_place(&mut self.ptr.as_mut().lockdep_map) + }; + + // SAFETY: The pointer was initialised from the result of `Box::leak`, and + // a ManuallyDrop is compatible. We already dropped the contents above. + unsafe { Box::from_raw(self.ptr.as_ptr() as *mut ManuallyDrop>) }; } } } @@ -499,6 +549,7 @@ pub struct UniqueArc { impl UniqueArc { /// Tries to allocate a new [`UniqueArc`] instance. + #[track_caller] pub fn try_new(value: T) -> Result { Ok(Self { // INVARIANT: The newly-created object has a ref-count of 1. @@ -507,13 +558,27 @@ impl UniqueArc { } /// Tries to allocate a new [`UniqueArc`] instance whose contents are not initialised yet. + #[track_caller] pub fn try_new_uninit() -> Result>, AllocError> { // INVARIANT: The refcount is initialised to a non-zero value. + #[cfg(CONFIG_RUST_EXTRA_LOCKDEP)] + let inner = { + let map = LockdepMap::new(); + Box::try_init::(try_init!(ArcInner { + // SAFETY: There are no safety requirements for this FFI call. + refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }), + lockdep_map: map, + data <- init::uninit::(), + }? AllocError))? + }; + // FIXME: try_init!() does not work with cfg attributes. + #[cfg(not(CONFIG_RUST_EXTRA_LOCKDEP))] let inner = Box::try_init::(try_init!(ArcInner { // SAFETY: There are no safety requirements for this FFI call. refcount: Opaque::new(unsafe { bindings::REFCOUNT_INIT(1) }), data <- init::uninit::(), }? AllocError))?; + Ok(UniqueArc { // INVARIANT: The newly-created object has a ref-count of 1. // SAFETY: The pointer from the `Box` is valid.