From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: rust: drm: gem/shmem: Add DmaResvGuard helper
Date: Wed, 27 May 2026 14:05:19 +1000
Message-ID: <review-patch5-20260526212857.1158294-6-lyude@redhat.com>
In-Reply-To: <20260526212857.1158294-6-lyude@redhat.com>
References: <20260526212857.1158294-1-lyude@redhat.com> <20260526212857.1158294-6-lyude@redhat.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Status: Minor concerns**

The guard pattern is correct in structure:

```rust
+struct DmaResvGuard<'a, T: DriverObject>(&'a Object<T>);
+
+impl<'a, T: DriverObject> DmaResvGuard<'a, T> {
+    fn new(obj: &'a Object<T>) -> Self {
+        unsafe { bindings::dma_resv_lock(obj.raw_dma_resv(), ptr::null_mut=
()) };
+        Self(obj)
+    }
+}
```

**Concern 1**: `dma_resv_lock()` returns `int`, but the return value is sil=
ently discarded. With `ctx =3D NULL`, `ww_mutex_lock` reduces to a simple m=
utex lock that in practice always returns 0, but ignoring the return value =
is not ideal. Since this is called from `Drop` contexts (via `SGTableMap::d=
rop` in patch 6) where returning errors isn't possible, at minimum a `WARN_=
ON` or `BUG_ON` on failure would be appropriate =E2=80=94 or a comment expl=
aining why the return is ignored.

**Concern 2**: The `#[expect(unused)]` attribute is removed in patch 6, whi=
ch is correct for the series as a whole, but it means patch 5 in isolation =
adds dead code. This is a minor bisectability concern =E2=80=94 consider sq=
uashing patches 5 and 6, or keeping `#[expect(unused)]` in patch 5 and remo=
ving it in patch 6.

The TODO comment about replacing with WwMutex is appropriate.

---
Generated by Claude Code Patch Reviewer