Claude review: drm/qxl: Convert qxl release idr to xarray

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

Patch Review

**Unnecessary locking around `xa_load()` in `qxl_release_from_id_locked`:**

```c
xa_lock(&qdev->release_xa);
release = xa_load(&qdev->release_xa, id);
xa_unlock(&qdev->release_xa);
```

`xa_load()` is RCU-safe internally and does not require external locking for read access. The `xa_lock`/`xa_unlock` pair here is harmless but unnecessary overhead. A bare `xa_load()` call would be sufficient and more idiomatic. The v3 changelog states "Keep xa_lock in xa_lock due to concurrent access", but this reflects a misunderstanding — `xa_load()` already handles concurrent access correctly via RCU. `xa_lock` is only needed for modification operations (`xa_store`, `xa_erase`, `xa_alloc`), and the modifications in this patch (`xa_alloc`, `xa_erase`) already handle their own locking internally. The simplification should be:

```c
release = xa_load(&qdev->release_xa, id);
```

**Seqno ordering subtlety (observation, not a blocker):**

In the original code, IDR allocation and seqno increment were both inside `release_idr_lock`:

```c
spin_lock(&qdev->release_idr_lock);
handle = idr_alloc(&qdev->release_idr, release, 1, 0, GFP_NOWAIT);
release->base.seqno = ++qdev->release_seqno;
spin_unlock(&qdev->release_idr_lock);
```

In the new code these are separate atomic operations:

```c
r = xa_alloc(&qdev->release_xa, &handle, release, xa_limit_31b, GFP_KERNEL);
release->base.seqno = (u32)atomic_inc_return(&qdev->release_seqno);
```

Two concurrent allocations could now observe ID ordering that doesn't match seqno ordering (thread A gets ID 5/seqno 7, thread B gets ID 6/seqno 6). In practice this is fine — the DMA fence seqno just needs uniqueness within a context, not strict correlation with the xarray index — but it's a behavioral change worth noting.

**Missing `xa_destroy()` in teardown (pre-existing):**

`qxl_device_fini()` never called `idr_destroy()` on `release_idr`, and the patch doesn't add `xa_destroy()` for `release_xa` either. While by teardown time the xarray should be empty (after the release event wait + GC flush), adding `xa_destroy(&qdev->release_xa)` in `qxl_device_fini()` would be good hygiene. This is a pre-existing issue rather than a regression, but this patch would be a natural place to fix it.

**Positive aspects:**

- `xa_limit_31b` correctly keeps IDs within `INT_MAX`, matching `release->id` being `int` and the function returning `int`.
- Using `GFP_KERNEL` instead of the `idr_preload(GFP_KERNEL)` + `idr_alloc(..., GFP_NOWAIT)` dance is a good simplification enabled by `xa_alloc` supporting sleeping allocation directly.
- `XA_FLAGS_ALLOC1` correctly mirrors the old `idr_init_base(&qdev->release_idr, 1)` behavior.
- The `(u32)` cast on `atomic_inc_return` correctly handles potential wrapping above `INT_MAX`.
- Format string change `%d` → `%u` for `handle` is correct.

---
Generated by Claude Code Patch Reviewer