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/panthor: set __GFP_SKIP_KASAN Date: Sat, 16 May 2026 13:08:47 +1000 Message-ID: In-Reply-To: <20260512-panthor-kasan-v1-1-d8d3e275d71b@gmail.com> References: <20260512-panthor-kasan-v1-1-d8d3e275d71b@gmail.com> <20260512-panthor-kasan-v1-1-d8d3e275d71b@gmail.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 **Correctness analysis:** The key expansion check: - `GFP_HIGHUSER` =3D `GFP_USER | __GFP_HIGHMEM` - `GFP_HIGHUSER_MOVABLE` =3D `GFP_HIGHUSER | __GFP_MOVABLE | __GFP_SKIP_KAS= AN` - `GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE` =3D `GFP_HIGHUSER | __GFP_SKIP_KA= SAN` So the new expression: ```c (GFP_HIGHUSER_MOVABLE & ~__GFP_MOVABLE) | __GFP_RETRY_MAYFAIL | __GFP_NOWARN ``` is equivalent to the old: ```c GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN ``` plus `__GFP_SKIP_KASAN`. This is exactly the intended change =E2=80=94 the = only new flag added is `__GFP_SKIP_KASAN`. **Concerns:** 1. **Fragility of the indirection:** The expression `(GFP_HIGHUSER_MOVABLE = & ~__GFP_MOVABLE)` is a somewhat indirect way to spell `GFP_HIGHUSER | __GF= P_SKIP_KASAN`. If `GFP_HIGHUSER_MOVABLE` ever gains additional flags beyond= `__GFP_MOVABLE | __GFP_SKIP_KASAN`, those would silently be included here.= However, this is a common kernel pattern (e.g., `GFP_TRANSHUGE_LIGHT` does= a similar mask-and-strip), and the commit message explains the reasoning, = so this is acceptable. 2. **Alternative: using `__GFP_SKIP_KASAN` directly.** A more explicit alte= rnative would be: ```c GFP_HIGHUSER | __GFP_SKIP_KASAN | __GFP_RETRY_MAYFAIL | __GFP_NOWARN ``` This is arguably clearer about what's happening, but the author's approa= ch has the advantage of tying the flags to `GFP_HIGHUSER_MOVABLE` semantics= (these pages *would* be movable if panthor didn't pin them), which is a re= asonable design choice and matches the existing comment about why `MOVABLE`= is stripped. 3. **Alignment nit:** The `__GFP_RETRY_MAYFAIL` on the continuation line is= indented with tabs plus extra spaces to align with the opening `(`, but th= e alignment looks like it might use one extra tab. This is very minor and d= epends on the project's tab-stop convention. 4. **The `drm/msm` note** in the commit's post-sign-off section is a useful= heads-up. If msm GEM objects can also be swapped via shmem and the system = has MTE, the same bug would apply there. This is worth tracking but doesn't= affect this patch. **Overall for this patch:** Correct fix for a real bug. The commit message = is excellent =E2=80=94 it explains the root cause clearly, including the no= n-obvious `shmem_swapin_cluster` interaction path. No functional issues fou= nd. Reviewed-by: would be appropriate here. --- Generated by Claude Code Patch Reviewer