Re: [PATCH RFC 07/11] rust: sync: Implement dynamic lockdep class creation

Martin Rodriguez Reboredo
2023-07-14T20:38:20Z

On 7/14/23 06:13, Asahi Lina wrote:
> Using macros to create lock classes all over the place is unergonomic,
> and makes it impossible to add new features that require lock classes to
> code such as Arc<> without changing all callers.
>
> Rust has the ability to track the caller's identity by file/line/column
> number, and we can use that to dynamically generate lock classes
> instead.
>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
> [...]
> +
> +const LOCK_CLASS_BUCKETS: usize = 1024;
> +
> +#[track_caller]
> +fn caller_lock_class_inner() -> Result<&'static DynLockClassKey> {
> + // This is just a hack to make the below static array initialization work.
> + #[allow(clippy::declare_interior_mutable_const)]
> + const ATOMIC_PTR: AtomicPtr<Mutex<Vec<&'static DynLockClassKey>>> =
> + AtomicPtr::new(core::ptr::null_mut());
> +
> + #[allow(clippy::complexity)]
> + static LOCK_CLASSES: [AtomicPtr<Mutex<Vec<&'static DynLockClassKey>>>; LOCK_CLASS_BUCKETS] =
> + [ATOMIC_PTR; LOCK_CLASS_BUCKETS];
> +
> + let loc = core::panic::Location::caller();
> + let loc_key = LocationKey::new(loc);
> +
> + let index = (loc_key.hash % (LOCK_CLASS_BUCKETS as u64)) as usize;
> + let slot = &LOCK_CLASSES[index];
> +
> + let mut ptr = slot.load(Ordering::Relaxed);
> + if ptr.is_null() {
> + let new_element = Box::pin_init(new_mutex!(Vec::new()))?;
> +
> + if let Err(e) = slot.compare_exchange(
> + core::ptr::null_mut(),
> + // SAFETY: We never move out of this Box
> + Box::into_raw(unsafe { Pin::into_inner_unchecked(new_element) }),
> + Ordering::Relaxed,
> + Ordering::Relaxed,
> + ) {
> + // SAFETY: We just got this pointer from `into_raw()`
> + unsafe { Box::from_raw(e) };
> + }
> +
> + ptr = slot.load(Ordering::Relaxed);
> + assert!(!ptr.is_null());
> + }
> +
> + // SAFETY: This mutex was either just created above or previously allocated,
> + // and we never free these objects so the pointer is guaranteed to be valid.
> + let mut guard = unsafe { (*ptr).lock() };
> +
> + for i in guard.iter() {
> + if i.loc == loc_key {
> + return Ok(i);
> + }
> + }
> +
> + // We immediately leak the class, so it becomes 'static
> + let new_class = Box::leak(Box::try_new(DynLockClassKey {
> + key: Opaque::zeroed(),
> + loc: loc_key,
> + name: CString::try_from_fmt(fmt!("{}:{}:{}", loc.file(), loc.line(), loc.column()))?,
> + })?);
> +
> + // SAFETY: This is safe to call with a pointer to a dynamically allocated lockdep key,
> + // and we never free the objects so it is safe to never unregister the key.
> + unsafe { bindings::lockdep_register_key(new_class.key.get()) };
> +
> + guard.try_push(new_class)?;
> +
> + Ok(new_class)
> +}
> +
> [...]

Is there any problem if we have many `DynLockClassKey`s leaked or not?