From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/pagemap: Add helper to access zone_device_data
Date: Fri, 13 Mar 2026 13:55:14 +1000
Message-ID: <review-patch2-20260312192126.2024853-3-francois.dugast@intel.com>
In-Reply-To: <20260312192126.2024853-3-francois.dugast@intel.com>
References: <20260312192126.2024853-1-francois.dugast@intel.com> <20260312192126.2024853-3-francois.dugast@intel.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

**Good:** Centralizes `zone_device_data` access through `drm_pagemap_page_z=
one_device_data()`, which correctly goes through the folio API (`page_folio=
()` =E2=86=92 `folio_zone_device_data()`). This prepares for THP where only=
 the head page's `zone_device_data` is meaningful.

**Concern =E2=80=94 `folio_zone_device_data` only checks `folio_is_device_p=
rivate`:**

Looking at the kernel's `folio_zone_device_data()`:
```c
static inline void *folio_zone_device_data(const struct folio *folio)
{
    VM_WARN_ON_FOLIO(!folio_is_device_private(folio), folio);
    return folio->page.zone_device_data;
}
```

This will trigger a `VM_WARN` if called on a device-coherent folio. The exi=
sting code in `drm_pagemap.c` accesses `zone_device_data` on both device-pr=
ivate and device-coherent pages (e.g., `drm_pagemap_migrate_unmap_pages` ch=
ecks `is_zone_device_page` which covers both). In `drm_gpusvm.c` the call s=
ite at line 1493 also checks `is_device_coherent_page`. So the new helper w=
ill produce `VM_WARN` on coherent pages in debug builds. This seems like a =
pre-existing issue with the `folio_zone_device_data` API rather than this p=
atch specifically, but it's worth noting =E2=80=94 the callers in `drm_page=
map_migrate_unmap_pages` at line 325-326 use `is_zone_device_page` which in=
cludes coherent pages.

**Good:** The `#else` stub returning NULL for `!CONFIG_ZONE_DEVICE` is corr=
ectly placed (the v9 changelog notes this was a build fix).

**Minor:** In `drm_pagemap_folio_free`:
```c
struct page *page =3D folio_page(folio, 0);
drm_pagemap_zdd_put(drm_pagemap_page_zone_device_data(page));
```
This could simply use `folio_zone_device_data(folio)` directly without the =
extra `folio_page` + `page_folio` round-trip inside the helper. Not a bug, =
just a minor indirection.

---
Generated by Claude Code Patch Reviewer