Claude review: mm/hmm: do the plumbing for HMM to participate in migration

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

Patch Review

This is the largest structural change — it reworks `hmm_vma_walk` to carry migration state, adds pmd/pte lock tracking, and introduces `hmm_vma_capture_migrate_range()` for capturing the VMA and MMU notifier range during the walk.

**Issue 1 — Potential duplicate definitions when CONFIG_DEVICE_MIGRATION is set:**

Patch adds a fallback `enum migrate_vma_info` and `hmm_select_migrate()` inside `#ifdef CONFIG_MIGRATION`:
```c
+enum migrate_vma_info {
+	MIGRATE_VMA_SELECT_NONE = 0,
+	MIGRATE_VMA_SELECT_COMPOUND = MIGRATE_VMA_SELECT_NONE,
+};
+
+static inline enum migrate_vma_info hmm_select_migrate(struct hmm_range *range)
+{
+	return MIGRATE_VMA_SELECT_NONE;
+}
+
 #endif /* CONFIG_MIGRATION */
```

And then in the `#ifdef CONFIG_DEVICE_MIGRATION` block (which implies `CONFIG_MIGRATION`):
```c
-enum migrate_vma_direction {
+enum migrate_vma_info {
 	MIGRATE_VMA_SELECT_SYSTEM = 1 << 0,
 ...
+static inline enum migrate_vma_info hmm_select_migrate(struct hmm_range *range)
+{
+	return 0;
+}
```

When `CONFIG_DEVICE_MIGRATION=y` (which requires `CONFIG_MIGRATION=y`), both blocks are compiled, giving duplicate definitions of the enum and the inline function. This should be a compile error. The fallback definition needs a `#ifndef CONFIG_DEVICE_MIGRATION` guard. I may be misreading the base tree structure (the patches target drm-tip, not mainline), but if both blocks are unconditionally compiled, this won't build with `CONFIG_DEVICE_MIGRATION=y`.

**Issue 2 — `hmm_vma_walk` struct growth:**
```c
 struct hmm_vma_walk {
-	struct hmm_range	*range;
-	unsigned long		last;
+	struct mmu_notifier_range	mmu_range;
+	struct vm_area_struct		*vma;
+	struct hmm_range		*range;
+	unsigned long			start;
+	unsigned long			end;
+	unsigned long			last;
+	bool				ptelocked;
+	bool				pmdlocked;
+	spinlock_t			*ptl;
 };
```
This struct is stack-allocated in `hmm_range_fault()`. The `mmu_notifier_range` alone is a significant addition. The overhead is always paid even when not migrating. This is fine functionally but worth noting for non-migration callers.

**Issue 3 — Lock state sharing via single `ptl` field:**

The `ptl` pointer is used for both PMD and PTE locks, which works because they're never held simultaneously. But it makes the code harder to reason about — a future change that accidentally holds both would corrupt state silently. The `HMM_ASSERT_*` macros help but only catch issues at runtime.

**Issue 4 — `hmm_vma_capture_migrate_range` MMU notifier range only covers the first callback:**
```c
+	if (!hmm_vma_walk->mmu_range.owner) {
+		mmu_notifier_range_init_owner(&hmm_vma_walk->mmu_range, MMU_NOTIFY_MIGRATE, 0,
+					      walk->vma->vm_mm, start, end,
+					      range->dev_private_owner);
+		mmu_notifier_invalidate_range_start(&hmm_vma_walk->mmu_range);
+	}
```
The MMU notifier range is initialized once with the first `start`/`end` from `hmm_vma_walk_test`. Subsequent calls update `hmm_vma_walk->end` but NOT the notifier range. Since the series enforces single-VMA via the `-ERANGE` check, this should be fine for the per-VMA test callback, but the `start`/`end` from `hmm_vma_walk_test` may be the per-VMA range which could differ from individual PMD-level sub-ranges.

**Issue 5 — `else` after `}` without braces (kernel style):**
Multiple instances of:
```c
+		} else
+			pte_unmap(ptep);
```
Kernel coding style requires braces on the `else` if the `if` block has braces.

**Issue 6 — C++ style comments for stubs:**
```c
+	// TODO: implement migration entry insertion
+	return 0;
```
These are removed in patch 4, so it's fine for bisect, but `/* */` is preferred in kernel code (though `//` is increasingly accepted).

---
Generated by Claude Code Patch Reviewer