On 10/21/26 5:04 PM, [email protected] wrote:
> From: Mika Penttilä <[email protected]>
>
> For migration to happen after hmm_range_fault(), the vma has to
> retrieved. Luckily, pagewalk already resolves that for us, so
> just have to save it for further use. For migration, mmap_lock
> has to be hold for the whole operation, so vma stays stable.
> In case lock-drop behavior is requested, return an error while
> migrating. Also, rename some struct hmm_vma_walk fields to solve
> namespace collisions, and we are later introducing more
> xxlocked fields so be unambiguous.
>
> Take care also of firing the mmu_notifier when migrating.
>
> Also, prepare for differencies of how mm_struct * is retrieved
> in fault and migrate_vma paths.
>
> Cc: David Hildenbrand <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Leon Romanovsky <[email protected]>
> Cc: Alistair Popple <[email protected]>
> Cc: Balbir Singh <[email protected]>
> Cc: Zi Yan <[email protected]>
> Cc: Matthew Brost <[email protected]>
> Suggested-by: Alistair Popple <[email protected]>
> Signed-off-by: Mika Penttilä <[email protected]>
> ---
> include/linux/migrate.h | 50 +++++++++++++++++-
> lib/test_hmm.c | 2 +-
> mm/hmm.c | 110 ++++++++++++++++++++++++++++++++++------
> 3 files changed, 145 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index 280ec9a866c7..dd7dd7ec6995 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -5,6 +5,7 @@
> #include <linux/mm.h>
> #include <linux/hmm.h>
> #include <linux/mempolicy.h>
> +#include <linux/mmu_notifier.h>
> #include <linux/migrate_mode.h>
> #include <linux/hugetlb.h>
>
> @@ -108,6 +109,29 @@ static inline void
> softleaf_entry_wait_on_locked(softleaf_t entry, spinlock_t *p
> spin_unlock(ptl);
> }
>
> +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;
> +}
> +
> +static inline void hmm_fill_migrate_vma(struct hmm_range *range,
> + struct vm_area_struct *vma,
> + unsigned long start,
> + unsigned long end)
> +{
> +}
> +
> +#ifdef CONFIG_MMU_NOTIFIER
> +static inline struct mm_struct *hmm_range_fault_mm(struct hmm_range *range)
> +{
> + return range->notifier->mm;
> +}
> +#endif
> #endif /* CONFIG_MIGRATION */
>
> #ifdef CONFIG_NUMA_BALANCING
> @@ -151,7 +175,8 @@ static inline unsigned long migrate_pfn(unsigned long pfn)
> return (pfn << MIGRATE_PFN_SHIFT) | MIGRATE_PFN_VALID;
> }
>
> -enum migrate_vma_direction {
> +enum migrate_vma_info {
> + MIGRATE_VMA_SELECT_NONE = 0,
> MIGRATE_VMA_SELECT_SYSTEM = 1 << 0,
> MIGRATE_VMA_SELECT_DEVICE_PRIVATE = 1 << 1,
> MIGRATE_VMA_SELECT_DEVICE_COHERENT = 1 << 2,
> @@ -193,6 +218,29 @@ struct migrate_vma {
> struct page *fault_page;
> };
>
> +// TODO: enable migration
> +static inline enum migrate_vma_info hmm_select_migrate(struct hmm_range
> *range)
> +{
> + return 0;
> +}
> +
> +#ifdef CONFIG_MMU_NOTIFIER
> +static inline struct mm_struct *hmm_range_fault_mm(struct hmm_range *range)
> +{
> + return range->notifier ? range->notifier->mm :
> range->migrate->vma->vm_mm;
> +}
> +#endif
> +
> +static inline void hmm_fill_migrate_vma(struct hmm_range *range,
> + struct vm_area_struct *vma,
> + unsigned long start,
> + unsigned long end)
> +{
> + range->migrate->vma = vma;
> + range->migrate->start = start;
> + range->migrate->end = end;
> +}
> +
> int migrate_vma_setup(struct migrate_vma *args);
> void migrate_vma_pages(struct migrate_vma *migrate);
> void migrate_vma_finalize(struct migrate_vma *migrate);
> diff --git a/lib/test_hmm.c b/lib/test_hmm.c
> index 6911daa9f854..cd88e8177d0a 100644
> --- a/lib/test_hmm.c
> +++ b/lib/test_hmm.c
> @@ -145,7 +145,7 @@ static bool dmirror_is_private_zone(struct dmirror_device
> *mdevice)
> HMM_DMIRROR_MEMORY_DEVICE_PRIVATE);
> }
>
> -static enum migrate_vma_direction
> +static enum migrate_vma_info
> dmirror_select_device(struct dmirror *dmirror)
> {
> return (dmirror->mdevice->zone_device_type ==
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 2f1e98c6b644..4e9542aedc25 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -21,6 +21,7 @@
> #include <linux/pagemap.h>
> #include <linux/leafops.h>
> #include <linux/hugetlb.h>
> +#include <linux/migrate.h>
> #include <linux/memremap.h>
> #include <linux/sched/mm.h>
> #include <linux/jump_label.h>
> @@ -28,15 +29,23 @@
> #include <linux/pci-p2pdma.h>
> #include <linux/mmu_notifier.h>
> #include <linux/memory_hotplug.h>
> +#include <asm/tlbflush.h>
>
> #include "internal.h"
>
> struct hmm_vma_walk {
> - struct hmm_range *range;
> - bool *locked;
> - unsigned long last;
> - unsigned long end;
> - unsigned int required_fault;
> + struct mmu_notifier_range mmu_range;
> + /* Set once mmu_range is armed with
> mmu_notifier_invalidate_range_start() */
> + bool mmu_armed;
> + struct vm_area_struct *vma;
> + struct hmm_range *range;
> + unsigned long start;
> + unsigned long end;
> + unsigned long last;
> + /* fault and lock drop related fields */
> + bool *mmlocked;
> + unsigned long fault_end;
> + unsigned int required_fault;
> };
>
> /*
> @@ -95,7 +104,7 @@ static int hmm_record_fault(unsigned long addr, unsigned
> long end,
>
> WARN_ON_ONCE(!required_fault);
> hmm_vma_walk->last = addr;
> - hmm_vma_walk->end = end;
> + hmm_vma_walk->fault_end = end;
> hmm_vma_walk->required_fault = required_fault;
> return HMM_FAULT_PENDING;
> }
> @@ -400,6 +409,58 @@ static int hmm_vma_handle_absent_pmd(struct mm_walk
> *walk, unsigned long start,
> }
> #endif /* CONFIG_ARCH_HAS_PMD_SOFTLEAVES */
>
> +static int hmm_vma_capture_migrate_range(unsigned long start,
> + unsigned long end,
> + struct mm_walk *walk)
> +{
> + struct hmm_vma_walk *hmm_vma_walk = walk->private;
> + struct hmm_range *range = hmm_vma_walk->range;
> +
> + if (!hmm_select_migrate(range))
> + return 0;
> +
> + if (hmm_vma_walk->vma && (hmm_vma_walk->vma != walk->vma))
> + return -ERANGE;
> +
> + hmm_vma_walk->vma = walk->vma;
I don't quite follow the check for -ERANGE, it checks for hmm_vma_walk->vma !=
walk->vma, but then set's it later to be the same?
> +
> + if (end - start > range->end - range->start)
> + return -ERANGE;
> +
Have we established that end > start and same for range->end > range->start?
> + if (!hmm_vma_walk->mmu_armed) {
> + hmm_vma_walk->start = start;
> + hmm_vma_walk->end = end;
Does the walk range matter here?
> + 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);
> + hmm_vma_walk->mmu_armed = true;
> + }
> +
> + return 0;
> +}
> +
> +static void hmm_vma_post_range_fault(struct hmm_vma_walk *hmm_vma_walk)
> +{
> +
> + struct hmm_range *range = hmm_vma_walk->range;
> +
> + if (hmm_select_migrate(range) &&
> + hmm_vma_walk->mmu_armed) {
> + /*
> + * The migrate_vma path has the following initialized,
> + * so take care of fault path below.
> + */
> + if (range->notifier) {
> + hmm_fill_migrate_vma(range,
> + hmm_vma_walk->vma,
> + hmm_vma_walk->start,
> + hmm_vma_walk->end);
> + }
> + mmu_notifier_invalidate_range_end(&hmm_vma_walk->mmu_range);
> + }
> +}
> +
> static int hmm_vma_walk_pmd(pmd_t *pmdp,
> unsigned long start,
> unsigned long end,
> @@ -594,6 +655,11 @@ static int hmm_vma_walk_test(unsigned long start,
> unsigned long end,
> struct hmm_vma_walk *hmm_vma_walk = walk->private;
> struct hmm_range *range = hmm_vma_walk->range;
> struct vm_area_struct *vma = walk->vma;
> + int r;
> +
> + r = hmm_vma_capture_migrate_range(start, end, walk);
> + if (r)
> + return r;
>
> if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)) &&
> vma->vm_flags & VM_READ)
> @@ -646,12 +712,12 @@ static int hmm_do_fault(struct mm_struct *mm,
> struct hmm_vma_walk *hmm_vma_walk)
> {
> unsigned long addr = hmm_vma_walk->last;
> - unsigned long end = hmm_vma_walk->end;
> + unsigned long end = hmm_vma_walk->fault_end;
> unsigned int required_fault = hmm_vma_walk->required_fault;
> unsigned int fault_flags = FAULT_FLAG_REMOTE;
> struct vm_area_struct *vma;
>
> - if (hmm_vma_walk->locked)
> + if (hmm_vma_walk->mmlocked)
> fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
>
> vma = vma_lookup(mm, addr);
> @@ -670,8 +736,8 @@ static int hmm_do_fault(struct mm_struct *mm,
> ret = handle_mm_fault(vma, addr, fault_flags, NULL);
>
> if (ret & (VM_FAULT_COMPLETED | VM_FAULT_RETRY)) {
> - if (hmm_vma_walk->locked) /* needed by sparse */
> - *hmm_vma_walk->locked = false;
> + if (hmm_vma_walk->mmlocked) /* needed by sparse */
> + *hmm_vma_walk->mmlocked = false;
> else
> WARN_ON_ONCE(1); /* broken fault handler */
> return HMM_FAULT_UNLOCKED;
> @@ -694,19 +760,31 @@ static int hmm_range_fault_locked(struct hmm_range
> *range, bool *locked)
> {
> struct hmm_vma_walk hmm_vma_walk = {
> .range = range,
> - .locked = locked,
> + .mmlocked = locked,
> .last = range->start,
> };
> - struct mm_struct *mm = range->notifier->mm;
> + /*
> + * Could be serving a device fault or come from migrate
> + * entry point. For the former we have not resolved the vma
> + * yet, and the latter we don't have a notifier (but have a vma).
> + *
> + */
> + struct mm_struct *mm = hmm_range_fault_mm(range);
> int ret;
>
> + /* Migration is incompatible with mmap lock drop */
> + if (locked && hmm_select_migrate(range))
> + return -EINVAL;
> +
> mmap_assert_locked(mm);
>
> do {
> /* If range is no longer valid force retry. */
> - if (mmu_interval_check_retry(range->notifier,
> - range->notifier_seq))
> - return -EBUSY;
> + if (range->notifier && mmu_interval_check_retry(range->notifier,
> +
> range->notifier_seq)) {
> + ret = -EBUSY;
> + break;
> + }
> ret = walk_page_range(mm, hmm_vma_walk.last, range->end,
> &hmm_walk_ops, &hmm_vma_walk);
> /*
> @@ -732,6 +810,8 @@ static int hmm_range_fault_locked(struct hmm_range
> *range, bool *locked)
> * output, and all >= are still at their input values.
> */
> } while (ret == -EBUSY);
> +
> + hmm_vma_post_range_fault(&hmm_vma_walk);
Have you checked to see if we could end up with notifier start/end inbalance?
For example if hmm_select_migrate returns false?
> return ret;
> }
>
