Messages in this thread | | | Date | Thu, 24 Aug 2023 11:14:59 +0800 | Subject | Re: [PATCH v2 2/4] mm: migrate: move the numamigrate_isolate_page() into do_numa_page() | From | Baolin Wang <> |
| |
On 8/22/2023 5:02 PM, Bharata B Rao wrote: > On 22-Aug-23 6:23 AM, Baolin Wang wrote: >> Move the numamigrate_isolate_page() into do_numa_page() to simplify the >> migrate_misplaced_page(), which now only focuses on page migration, and >> it also serves as a preparation for supporting batch migration for >> migrate_misplaced_page(). >> >> While we are at it, change the numamigrate_isolate_page() to boolean >> type to make the return value more clear. >> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> --- >> include/linux/migrate.h | 6 ++++++ >> mm/huge_memory.c | 7 +++++++ >> mm/memory.c | 7 +++++++ >> mm/migrate.c | 22 +++++++--------------- >> 4 files changed, 27 insertions(+), 15 deletions(-) >> >> diff --git a/include/linux/migrate.h b/include/linux/migrate.h >> index 711dd9412561..ddcd62ec2c12 100644 >> --- a/include/linux/migrate.h >> +++ b/include/linux/migrate.h >> @@ -144,12 +144,18 @@ const struct movable_operations *page_movable_ops(struct page *page) >> #ifdef CONFIG_NUMA_BALANCING >> int migrate_misplaced_page(struct page *page, struct vm_area_struct *vma, >> int node); >> +bool numamigrate_isolate_page(pg_data_t *pgdat, struct page *page); >> #else >> static inline int migrate_misplaced_page(struct page *page, >> struct vm_area_struct *vma, int node) >> { >> return -EAGAIN; /* can't migrate now */ >> } >> + >> +static inline bool numamigrate_isolate_page(pg_data_t *pgdat, struct page *page) >> +{ >> + return false; >> +} >> #endif /* CONFIG_NUMA_BALANCING */ >> >> #ifdef CONFIG_MIGRATION >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index 4a9b34a89854..07149ead11e4 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -1496,6 +1496,7 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) >> int target_nid, last_cpupid = (-1 & LAST_CPUPID_MASK); >> bool migrated = false, writable = false; >> int flags = 0; >> + pg_data_t *pgdat; >> >> vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd); >> if (unlikely(!pmd_same(oldpmd, *vmf->pmd))) { >> @@ -1545,6 +1546,12 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) >> goto migrate_fail; >> } >> >> + pgdat = NODE_DATA(target_nid); >> + if (!numamigrate_isolate_page(pgdat, page)) { >> + put_page(page); >> + goto migrate_fail; >> + } >> + >> migrated = migrate_misplaced_page(page, vma, target_nid); >> if (migrated) { >> flags |= TNF_MIGRATED; >> diff --git a/mm/memory.c b/mm/memory.c >> index fc6f6b7a70e1..4e451b041488 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -4769,6 +4769,7 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf) >> int target_nid; >> pte_t pte, old_pte; >> int flags = 0; >> + pg_data_t *pgdat; >> >> /* >> * The "pte" at this point cannot be used safely without >> @@ -4844,6 +4845,12 @@ static vm_fault_t do_numa_page(struct vm_fault *vmf) >> goto migrate_fail; >> } >> >> + pgdat = NODE_DATA(target_nid); >> + if (!numamigrate_isolate_page(pgdat, page)) { >> + put_page(page); >> + goto migrate_fail; >> + } >> + >> /* Migrate to the requested node */ >> if (migrate_misplaced_page(page, vma, target_nid)) { >> page_nid = target_nid; >> diff --git a/mm/migrate.c b/mm/migrate.c >> index 9cc98fb1d6ec..0b2b69a2a7ab 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -2478,7 +2478,7 @@ static struct folio *alloc_misplaced_dst_folio(struct folio *src, >> return __folio_alloc_node(gfp, order, nid); >> } >> >> -static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page) >> +bool numamigrate_isolate_page(pg_data_t *pgdat, struct page *page) >> { >> int nr_pages = thp_nr_pages(page); >> int order = compound_order(page); >> @@ -2496,11 +2496,11 @@ static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page) >> break; >> } > > There is an other s/return 0/return false/ changed required here for this chunk: > > if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING)) > return 0;
Good catch.
> >> wakeup_kswapd(pgdat->node_zones + z, 0, order, ZONE_MOVABLE); >> - return 0; >> + return false; >> } > > Looks like this whole section under "Avoiding migrating to a node that is nearly full" > check could be moved to numa_page_can_migrate() as that can be considered as one more > check (or action to) see if the page can be migrated or not. After that numamigrate_isolate_page() > will truly be about isolating the page.
Good idea. Will do. Thanks.
| |