Re: [PATCH v12 4/5] rust: drm: gem: Introduce shmem::SGTable

Alexandre Courbot
2026-04-25T02:39:04Z

On Sat Apr 25, 2026 at 8:10 AM JST, lyude wrote:
>
> On Fri, 2026-04-24 at 00:01 +0900, Alexandre Courbot wrote:
>> There are 4 sites where we acquire and release the DMA resv lock,
>> each
>> of which require unsafe blocks and carrying the risk that we forget
>> releasing the lock in the end. For this method in particular we need
>> to
>> jump through hoops a bit and store the return value into a temporary
>> variable so we can unlock the DMA reservation.
>>
>> Let's do ourselves a favor and implement a small, private guard type:
>>
>>     struct DmaResvGuard<'a, T: DriverObject>(&'a Object<T>);
>>
>>     impl<'a, T: DriverObject> DmaResvGuard<'a, T> {
>>         fn new(object: &'a Object<T>) -> Self {
>>             // SAFETY: This lock is initialized throughout the
>> lifetime of `object`
>>             unsafe { bindings::dma_resv_lock(object.raw_dma_resv(),
>> ptr::null_mut()) };
>>
>>             Self(object)
>>         }
>>     }
>>
>>     impl<'a, T> Drop for DmaResvGuard<'a, T>
>>     where
>>         T: DriverObject,
>>     {
>>         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())
>> };
>>         }
>>     }
>>
>> There here you would just do
>>
>>     let _dma_resv = DmaResvGuard::new(self);
>
> I thought of doing this but lost track of how many times I was actually
> grabbing this lock…
> BTW - want me to just give you authorship on the patch for adding
> DmaResvGuard? Since on my branch I've pretty much only just added two
> inline annotations, it's otherwise identical to what you wrote here.

You should remain author since I didn't formally send a patch - but if
the code remains that close, and you are comfortable with it, then a
Co-authored-by would be appreciated. Feel free to also add my

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>