Re: [PATCH v2 3/3] rust: sync: Add SpinLockIrq

[Benno Lossin <benno.lossin@proton.me>]

On 01.08.24 00:35, Lyude Paul wrote:
> +unsafe impl super::Backend for SpinLockIrqBackend {
> +    type State = bindings::spinlock_t;
> +    type GuardState = ();
> +    type Context<'a> = IrqDisabled<'a>;
> +
> +    unsafe fn init(
> +        ptr: *mut Self::State,
> +        name: *const core::ffi::c_char,
> +        key: *mut bindings::lock_class_key,
> +    ) {
> +        // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
> +        // `key` are valid for read indefinitely.
> +        unsafe { bindings::__spin_lock_init(ptr, name, key) }
> +    }
> +
> +    unsafe fn lock(ptr: *mut Self::State) -> Self::GuardState {
> +        // SAFETY: The safety requirements of this function ensure that `ptr` points to valid
> +        // memory, and that it has been initialised before.
> +        unsafe { bindings::spin_lock(ptr) }

Should this really be the same function as `SpinLock`? (ie is there not
a special version that expects IRQ to already be disabled? eg this could
add additional debug calls)

If there is such a function and you decide to use it, you need to adjust
the safety comment, since you need to justify that IRQs are disabled.
For that you need additional safety requirements on `lock` namely that
when `lock` is called there exists a object of type `Context<'a>` and
that object is alive at least until `unlock` is called.

---
Cheers,
Benno

> +    }
> +
> +    unsafe fn unlock(ptr: *mut Self::State, _guard_state: &Self::GuardState) {
> +        // SAFETY: The safety requirements of this function ensure that `ptr` is valid and that the
> +        // caller is the owner of the spinlock.
> +        unsafe { bindings::spin_unlock(ptr) }
> +    }
> +}
> --
> 2.45.2
>