Claude review: drm/pagemap_util: Ensure proper cache lock management on free

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

Patch Review

**Bug analysis:** The original code has three paths through the function after acquiring `cache->lock`:

1. `dpagemap == NULL` → unlocks ✓
2. `dpagemap != NULL && drm_pagemap_shrinker_cancel() == true` → unlocks ✓
3. `dpagemap != NULL && drm_pagemap_shrinker_cancel() == false` → **never unlocks** ✗

In case 3, execution falls through to `mutex_destroy()` and `kfree(cache)` with the spinlock still held. This is a real bug, not just a static analysis false positive — calling `mutex_destroy` under a spinlock is invalid, and `kfree` frees the lock memory while it's held.

**Review of the fix:**

The new code consolidates to a single unlock point:

```c
spin_lock(&cache->lock);
dpagemap = cache->dpagemap;
cache->dpagemap = NULL;
if (dpagemap && !drm_pagemap_shrinker_cancel(dpagemap))
    dpagemap = NULL;
spin_unlock(&cache->lock);

if (dpagemap)
    drm_pagemap_destroy(dpagemap, false);
```

This is clean and correct:
- `cache->dpagemap` is unconditionally cleared under the lock (v3 improvement per reviewer feedback).
- If `drm_pagemap_shrinker_cancel()` returns false (the shrinker is already processing this dpagemap), the local `dpagemap` is NULLed so we don't double-destroy — the shrinker scan path (`drm_pagemap_shrinker_scan` at line 382-388) will handle destruction.
- If cancel succeeds, we own the dpagemap and destroy it outside the lock.
- The lock is always released exactly once.

**Race condition check:** Setting `cache->dpagemap = NULL` before checking the shrinker is safe. The shrinker scan (`drm_pagemap_shrinker_scan`) takes `cache->lock` before clearing `cache->dpagemap`, so these two paths are properly serialized. `drm_pagemap_shrinker_cancel` accesses `dpagemap->cache` (the back-pointer from pagemap to cache), which is unaffected by clearing `cache->dpagemap`.

**Nit:** The commit message says "static analysis issue" and "probably unnecessary given the cache is being freed," which undersells the severity — this is a genuine spinlock leak bug that could cause lockups or undefined behavior. But that doesn't affect the code itself.

**Reviewed-by worthy: Yes.** The patch is correct and well-structured.

---
Generated by Claude Code Patch Reviewer