From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dmem: add amdgpu support and one more test Date: Sat, 16 May 2026 12:42:31 +1000 Message-ID: In-Reply-To: <20260512215156.4083082-1-cascardo@igalia.com> References: <20260512215156.4083082-1-cascardo@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: dmem: add amdgpu support and one more test Author: Thadeu Lima de Souza Cascardo Patches: 16 Reviewed: 2026-05-16T12:42:31.479017 --- This is an IGT (intel-gpu-tools) series, not a kernel series. It introduces a driver abstraction layer (`igt_dmem_driver`) to generalize the Xe-specific `xe_cgroups` dmem cgroup test so the same tests work across multiple GPU drivers. The series adds amdgpu support and a new `test_current` subtest that verifies cgroup `dmem.current`/`dmem.max` tracking. The refactoring approach is sound: extract a vtable interface, port the existing Xe code, rename the test to be driver-agnostic, add amdgpu, then layer on improvements. The patch ordering is logical, and the series is well-decomposed. **Key concerns:** 1. **Bug in patch 6** -- the max=0 assertion is inverted and will cause the test to fail on Xe (the primary target) while passing on amdgpu for the wrong reason. 2. **VA address leak in xe_dmem** -- `xe_ctx->addr` is never reset after `free_vram`, so the `test_current` subtest (which does multiple allocate/free cycles) will keep incrementing the bind address. 3. **amdgpu GTT fallback** weakens test coverage -- `fill_vram` with `err == 0` accepted means the eviction test may not exercise the intended code path on amdgpu at all. --- --- Generated by Claude Code Patch Reviewer