Re: [PATCH v19 1/4] rust: drm: gem: shmem: Add DmaResvGuard helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect use of `#[inline(always)]` instead of `#[inline]` for abstraction methods.
--

commit 35068fc880d5bd0d2870750a321a6b8155d32ae4
Author: Lyude Paul <lyude@redhat.com>

rust: drm: gem: shmem: Add DmaResvGuard helper

This commit adds a temporary DmaResvGuard helper type for managing the
dma_resv lock on GEM shmem objects. It implements a drop handler to
automatically release the lock.

> diff --git a/rust/kernel/drm/gem/shmem.rs b/rust/kernel/drm/gem/shmem.rs
> index 34af402899a0e..79ebd746e9611 100644
> --- a/rust/kernel/drm/gem/shmem.rs
> +++ b/rust/kernel/drm/gem/shmem.rs

[ ... ]

> @@ -235,3 +241,32 @@ impl<T: DriverObject, C: DeviceContext> driver::AllocImpl for Object<T, C> {
>          dumb_map_offset: None,
>      };
>  }
> +
> +/// Private helper-type for holding the `dma_resv` object for a GEM shmem object.
> +///
> +/// When this is dropped, the `dma_resv` lock is dropped as well.
> +///
> +// TODO: This should be replace with a WwMutex equivalent once we have such bindings in the kernel.
> +struct DmaResvGuard<'a, T: DriverObject, C: DeviceContext = Registered>(
> +    &'a Object<T, C>,
> +    NotThreadSafe,
> +);
> +
> +impl<'a, T: DriverObject, C: DeviceContext> DmaResvGuard<'a, T, C> {
> +    #[inline(always)]

[Severity: Low]
This isn't a bug, but should this use #[inline] instead of #[inline(always)]?

According to the Rust subsystem guidelines, #[inline(always)] should be
reserved for functions using build_assert!() that depend on function
parameters. For small abstraction functions that simply forward to binding
calls like this one, #[inline] is the correct annotation.

> +    #[expect(unused)]
> +    fn new(obj: &'a Object<T, C>) -> Self {
> +        // SAFETY: This lock is initialized throughout the lifetime of `object`.
> +        unsafe { bindings::dma_resv_lock(obj.raw_dma_resv(), ptr::null_mut()) };
> +
> +        Self(obj, NotThreadSafe)
> +    }
> +}
> +
> +impl<'a, T: DriverObject, C: DeviceContext> Drop for DmaResvGuard<'a, T, C> {
> +    #[inline(always)]

[Severity: Low]
This isn't a bug, but could this drop method also use #[inline] instead of
#[inline(always)] for the same reason mentioned above?

> +    fn drop(&mut self) {
> +        // SAFETY: We are releasing the lock grabbed during the creation of this object.
> +        unsafe { bindings::dma_resv_unlock(self.0.raw_dma_resv()) };
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608183057.2001376-1-lyude@redhat.com?part=1