From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/amdgpu: add SVM attr prefetch/force-trigger functionality Date: Tue, 28 Apr 2026 14:43:04 +1000 Message-ID: In-Reply-To: <20260427100522.7014-5-Junhua.Shen@amd.com> References: <20260427100522.7014-1-Junhua.Shen@amd.com> <20260427100522.7014-5-Junhua.Shen@amd.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Overall:** Small and focused. The one-shot prefetch semantics are well-mo= tivated. **Issue: `preferred_loc` changes no longer trigger `LOCATION_CHANGE`:** ```c - if (prev_attrs->preferred_loc !=3D new_attrs->preferred_loc || - prev_attrs->prefetch_loc !=3D new_attrs->prefetch_loc) + if (prev_attrs->prefetch_loc !=3D new_attrs->prefetch_loc) trigger |=3D AMDGPU_SVM_ATTR_TRIGGER_LOCATION_CHANGE; ``` This means changing `preferred_loc` alone will never trigger migration. In = patch 5, `amdgpu_svm_range_apply_attr_change()` only acts on `AMDGPU_SVM_AT= TR_TRIGGER_LOCATION_CHANGE` for migration. So if userspace sets a new `pref= erred_loc` without also issuing a prefetch, the new location preference won= 't take effect until the next fault or restore event. Is this intentional? = The commit message doesn't explain this tradeoff. **Good:** The `force_trigger` change from `bool` to `uint32_t` bitmask is t= he right approach =E2=80=94 it allows composing multiple trigger reasons. --- Generated by Claude Code Patch Reviewer