Claude review: mm: setup device page migration in HMM pagewalk

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

Patch Review

The core implementation — fills in the stub functions from patch 3 with real migration entry installation, implements the split callback, and rewires `migrate_vma_setup()` to use `hmm_range_fault()` internally.

**Issue 1 — `migrate_vma_setup()` return value semantics changed:**

Old behavior:
```c
-	migrate_vma_collect(args);
-	if (args->cpages)
-		migrate_vma_unmap(args);
-	return 0;
```

New behavior:
```c
+	ret = hmm_range_fault(&range);
+	migrate_hmm_range_setup(&range);
+	if (ret) {
+		migrate_vma_pages(args);
+		migrate_vma_finalize(args);
+	}
+	return ret;
```

The old code always returned 0 after the collect/unmap phase. The new code can return errors from `hmm_range_fault()`. This changes the API contract for callers (test_hmm, nouveau, amdgpu, xe). While callers should already handle errors from `migrate_vma_setup()` (due to earlier validation failures), any caller that assumed the walk phase never fails will now see new error paths exercised. The cleanup path (calling `migrate_vma_pages` + `migrate_vma_finalize` to undo migration entries) seems correct but is a new error-handling pattern.

**Issue 2 — `migrate_hmm_range_setup` called unconditionally on error:**
```c
+	ret = hmm_range_fault(&range);
+	migrate_hmm_range_setup(&range);
```
If `hmm_range_fault` fails partway, some `hmm_pfns` entries may be in an inconsistent state. `migrate_hmm_range_setup` iterates all entries and only processes those with `HMM_PFN_MIGRATE` set, so unprocessed entries (value 0) are safely skipped. This should be correct.

**Issue 3 — `flush_cache_page` called for device private pages:**
```c
+		flush_cache_page(walk->vma, addr, pfn);
```
This is called in the common path after both the device-private and present-pte branches converge. For device private pages, the page isn't in CPU cache, making this unnecessary. The old code only called `flush_cache_page` for present PTEs. Not a correctness bug, but unnecessary work for device-private-to-device-private migrations.

**Issue 4 — `hmm_vma_handle_pmd` called with `start` instead of `addr`:**
```c
-			r = hmm_vma_handle_pmd(walk, addr, end, hmm_pfns, pmd);
+			r = hmm_vma_handle_pmd(walk, start, end, hmm_pfns, pmd);
```
Using `start` is correct for the PMD-level handler since the PMD covers `[start, end)`. But note that `hmm_pfns` is recalculated at `again:` based on `addr`, not `start`. If an `-EAGAIN` retry causes `addr != start` AND the PMD happens to be (re-)promoted to a huge page in between, `hmm_pfns` would point to the wrong offset. This is a pre-existing race condition (extremely unlikely in practice) that isn't introduced by this patch, but the `start` change makes the mismatch slightly more visible.

**Issue 5 — Style: `} else` without braces:**
```c
+	} else
+		folio_put(folio);
```
Same coding style issue as patch 3.

**Issue 6 — `hmm_vma_walk_split` folio handling for fault_folio:**
```c
+		if (folio != fault_folio) {
+			if (unlikely(!folio_trylock(folio))) {
+				folio_put(folio);
+				ret = -EBUSY;
+				goto out;
+			}
+		}  else
+			folio_put(folio);
```
When `folio == fault_folio`: `folio_get` is followed by `folio_put` (net zero), then `split_folio(folio)` is called. This relies on `fault_folio` already being locked by the caller with sufficient refcount. Correct but non-obvious — a comment would help here.

---
Generated by Claude Code Patch Reviewer