Claude review: drm/shmem-helper: Fix Map huge page mapping in fault handler

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Correctness of the fix:** The approach is sound. By moving the PMD mapping to `.huge_fault`, the mm subsystem handles the dispatch between PTE-level and PMD-level faults, eliminating the race. This is the correct way to do it — the old code was essentially doing huge page mapping opportunistically from the regular `.fault` path, which is inherently racy.

**Unused variable warning when `CONFIG_ARCH_SUPPORTS_PMD_PFNMAP` is not set:**

```c
static vm_fault_t drm_gem_shmem_any_fault(struct vm_fault *vmf,
					   unsigned int order)
{
	...
	unsigned long paddr;
	bool aligned;
```

The variables `paddr` and `aligned` are declared at function scope but only used inside the `#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP` block in `case PMD_ORDER:`. When that config is disabled, these variables are unused and will produce compiler warnings (`-Wunused-variable`). They should be moved inside the `case PMD_ORDER:` block (with braces around the case body), or `__maybe_unused` could be applied, though moving them is cleaner.

That said, in practice `drm_gem_shmem_any_fault` is only called with `order != 0` from `drm_gem_shmem_huge_fault`, which is only compiled when `CONFIG_ARCH_SUPPORTS_HUGE_PFNMAP` is set, which implies `CONFIG_ARCH_SUPPORTS_PMD_PFNMAP` is also set (per `mm/Kconfig`). So the code is functionally correct, but the warning is still a build issue for configurations without these options enabled (where `.fault` still calls `drm_gem_shmem_any_fault(vmf, 0)` and the variables are dead code).

**Dropped `pmd_none()` check:**

The old code had:
```c
if (aligned &&
    pmd_none(*vmf->pmd) &&
    folio_test_pmd_mappable(page_folio(page))) {
```

The new code drops the `pmd_none(*vmf->pmd)` check:
```c
if (aligned &&
    folio_test_pmd_mappable(page_folio(pages[page_offset]))) {
	pfn &= PMD_MASK >> PAGE_SHIFT;
	ret = vmf_insert_pfn_pmd(vmf, pfn, false);
}
```

This is fine. In the old code, the `pmd_none` check was needed because the code was being called from the regular `.fault` path where the PMD might already be populated with PTEs. In the new `.huge_fault` path, the mm subsystem ensures we're only called when a PMD-level mapping is appropriate, so `vmf_insert_pfn_pmd()` can handle any existing PMD state internally (returning an appropriate error/fallback if needed).

**Minor style nit — commit subject:** "Fix Map huge page mapping" has an odd capitalisation of "Map". Consider: "Fix huge page mapping race in fault handler".

**Missing `default:` case in switch:**

```c
switch (order) {
case 0:
	ret = vmf_insert_pfn(vma, vmf->address, pfn);
	break;
#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
case PMD_ORDER:
	...
	break;
#endif
}
```

There's no `default:` case. If an unexpected order is passed, `ret` remains `VM_FAULT_FALLBACK` (from initialization), which is the right behavior, but a `default: break;` would be clearer and would silence any `-Wswitch` warnings if new enum values were ever relevant. The VFIO version (`vfio_pci_vmf_insert_pfn`) explicitly has a `default: return VM_FAULT_FALLBACK;`. This is very minor since `drm_gem_shmem_huge_fault` already filters for `PMD_ORDER` only.

**Overall:** Good fix for a real bug. The pattern matches existing in-tree usage (VFIO). The unused variable issue should be fixed before merging, but otherwise the patch looks correct.

---
Generated by Claude Code Patch Reviewer