Claude review: drm/virtio: add timeout to virtqueue wait to avoid hung task

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

Patch Review

**virtio_gpu_queue_ctrl_sgs timeout path (lines 142–150 of the diff):**

```c
+		if (!wait_event_timeout(vgdev->ctrlq.ack_queue,
+					vq->num_free >= elemcnt,
+					5 * HZ)) {
+			/* The device did not respond */
+			if (fence && vbuf->objs)
+				virtio_gpu_array_unlock_resv(vbuf->objs);
+			free_vbuf(vgdev, vbuf);
+			drm_dev_exit(idx);
+			return -ENODEV;
+		}
```

- **Cleanup is correct** — it mirrors the `drm_dev_enter()` failure path at lines 383–388 of the source. The fence has not been emitted yet at this point (that happens after the `goto again` loop succeeds), so not touching `fence` here is fine.
- **Missing error logging** — this is a significant event (device unresponsive for 5 seconds). At minimum a `drm_err(vgdev->ddev, "timed out waiting for ctrl virtqueue space\n")` should be emitted so the admin/developer can diagnose the failure.
- **`drm_dev_exit(idx)` is correct** — the function called `drm_dev_enter` earlier at line 383 and the timeout path needs to balance it.

**virtio_gpu_queue_cursor timeout path (lines 161–167 of the diff):**

```c
+		if (!wait_event_timeout(vgdev->cursorq.ack_queue,
+					vq->num_free >= outcnt,
+					5 * HZ)) {
+			/* The device did not respond */
+			free_vbuf(vgdev, vbuf);
+			drm_dev_exit(idx);
+			return;
+		}
```

- **Cleanup mirrors the `drm_dev_enter` failure path** at line 555–557 of the source (`free_vbuf` + return). This is correct.
- **`drm_dev_exit(idx)` is correct** — balances the `drm_dev_enter` at line 555.
- **Same missing logging concern** — should log a message on timeout.
- **Silent cursor drop** — the cursor update is silently lost on timeout. The sole caller `virtio_gpu_cursor_ping()` at line 1305 doesn't check for errors (and can't, since the function returns void). This is acceptable for a cursor ping but still deserves a log message.

**General issues across both sites:**

- **Race with legitimate completion:** The 5-second window is a fixed wall-clock timeout. Under memory pressure or host CPU contention, a legitimate completion that arrives at 5.1 seconds would trigger a false `-ENODEV`. Consider whether the timeout should be longer (e.g., 30s) or whether `wait_event_interruptible_timeout` would be more appropriate so the wait can be interrupted by signals — though that introduces its own complexity.
- **No `WARN` or telemetry:** Other drivers that add device-timeout paths often include a `drm_err` or `WARN_ONCE` to make the failure visible. Silently returning `-ENODEV` makes debugging harder.
- **Commit message claims consistency with existing timeout but doesn't cite it.** The claim "consistent with the existing timeout pattern in the driver" should reference the specific code location for reviewer verification.

**Summary:** The patch addresses a real problem but needs (1) logging on the timeout paths, (2) justification for the 5-second value (and consideration of whether it's too short for legitimate workloads), and (3) the commit message should cite which "existing timeout pattern" it's referring to.

---
Generated by Claude Code Patch Reviewer