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/pagemap: Unlock and put folios when possible Date: Fri, 13 Mar 2026 13:55:14 +1000 Message-ID: In-Reply-To: <20260312192126.2024853-2-francois.dugast@intel.com> References: <20260312192126.2024853-1-francois.dugast@intel.com> <20260312192126.2024853-2-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:** Clean conversion from page-level to folio-level unlock/put. The l= oop change from `++i` to stepping by `NR_PAGES(order)` correctly skips tail= pages of large folios. **Concern =E2=80=94 uninitialized `order` on zero migrate_pfn path:** ```c for (i =3D 0; i < npages;) { struct page *page; struct folio *folio; unsigned int order =3D 0; if (!migrate_pfn[i]) goto next; ... next: i +=3D NR_PAGES(order); } ``` When `migrate_pfn[i]` is zero, `order` is 0 and `NR_PAGES(0)` is 1, so the = loop advances by 1. This is correct. However, it means for null entries int= erleaved within a large folio's PFN slots, each null entry is iterated indi= vidually. This is fine for correctness =E2=80=94 the null entries simply in= dicate pages that weren't migrated =E2=80=94 but it's worth noting the sema= ntics are consistent. **Minor:** The `migrate_pfn[i] =3D 0` clear on line 273 only clears the hea= d page's entry, not tail entries. This appears correct since `drm_pagemap_m= igration_unlock_put_pages` is cleaning up after errors and only the head PF= N would have been set during setup. Looks good. --- Generated by Claude Code Patch Reviewer