Claude review: drm/gem/shmem: Introduce __drm_gem_shmem_free_sgt_locked()

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This extracts the sgt teardown logic into a standalone function and exports it for the Rust bindings.

**Missing NULL-safety:** The new function unconditionally dereferences `shmem->sgt`:

```c
void __drm_gem_shmem_free_sgt_locked(struct drm_gem_shmem_object *shmem)
{
	dma_resv_assert_held(shmem->base.resv);

	dma_unmap_sgtable(shmem->base.dev->dev, shmem->sgt, DMA_BIDIRECTIONAL, 0);
	sg_free_table(shmem->sgt);
	kfree(shmem->sgt);
	shmem->sgt = NULL;
}
```

The caller in `drm_gem_shmem_release` guards with `if (shmem->sgt)`, so the internal use is fine. But as an exported function (and one called from Rust `SGTableMap::drop`), it should either document the precondition that `sgt` must be non-NULL, or add a NULL check. Given the destruction-ordering concern I describe in patch 4's review, a NULL guard here would be defensive hardening.

**No pages put:** This function frees the sgt mapping but does not call `drm_gem_shmem_put_pages_locked()`. This is intentional — the pages remain pinned and are cleaned up when the gem object is ultimately freed — but it should be documented explicitly, since `drm_gem_shmem_get_pages_sgt_locked` paired `get_pages` with the sgt creation.

**Naming:** The double-underscore prefix (`__`) conventionally signals "internal, don't call directly" in the kernel, but this is exported with `EXPORT_SYMBOL_GPL` and has a public doc comment. Consider dropping the `__` prefix since it's part of the exported API.

**Doc comment vs. function name mismatch:** The kerneldoc title says `drm_gem_shmem_release_sgt_locked` but the function is named `__drm_gem_shmem_free_sgt_locked`.

---

---
Generated by Claude Code Patch Reviewer