From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: rust: drm: gem: shmem: Add sg_table() function Date: Wed, 27 May 2026 14:05:17 +1000 Message-ID: In-Reply-To: <20260526212857.1158294-1-lyude@redhat.com> References: <20260526212857.1158294-1-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 Overall Series Review Subject: rust: drm: gem: shmem: Add sg_table() function Author: Lyude Paul Patches: 7 Reviewed: 2026-05-27T14:05:17.818782 --- This is v14 of the series adding `sg_table()` support to the Rust DRM GEM s= hmem bindings. The series is well-structured: patches 1-3 are small prepara= tory cleanups, patch 4 adds a C helper, patch 5 adds a Rust locking helper,= and patch 6 is the main feature. The architecture of using `Devres` + `Laz= yInit` to manage SGTable lifetime and ensure revocation on driver-unbind is= sound and clearly evolved through many review iterations. However, there are a few issues: 1. **Patch 4**: The kerneldoc comment names the function incorrectly (`rele= ase` vs `free`). 2. **Patch 5**: `DmaResvGuard::new` silently discards the return value of `= dma_resv_lock()`. 3. **Patch 6**: The test code has a type mismatch =E2=80=94 `create_drm_dev= ` takes `&'static CStr` but is called with `Some(c"...")`. 4. **Patch 6**: `wrong_drm` is unused and will produce a warning. None of these are fundamental design issues, and the core logic (SGTableMap= invariants, Devres revocation ordering, self-referential pointer safety th= rough pinning) is carefully handled. --- --- Generated by Claude Code Patch Reviewer