Messages in this thread | | | Date | Wed, 24 Nov 2021 18:30:57 +0800 | Subject | Re: [PATCH 1/3] mm: migrate: Fix the return value of migrate_pages() | From | Baolin Wang <> |
| |
On 2021/11/24 2:46, Mike Kravetz wrote: > On 11/7/21 01:57, Baolin Wang wrote: >> As Zi Yan pointed out, the syscall move_pages() can return a non-migrated >> number larger than the number of pages the users tried to migrate, when a >> THP page is failed to migrate. This is confusing for users. >> >> Since other migration scenarios do not care about the actual non-migrated >> number of pages except the memory compaction migration which will fix in >> following patch. Thus we can change the return value to return the number >> of {normal page, THP, hugetlb} instead to avoid this issue, and the number >> of THP splits will be considered as the number of non-migrated THP, no matter >> how many subpages of the THP are migrated successfully. Meanwhile we should >> still keep the migration counters using the number of normal pages. >> >> Co-developed-by: Zi Yan <ziy@nvidia.com> >> Signed-off-by: Zi Yan <ziy@nvidia.com> >> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com> >> --- >> mm/migrate.c | 63 +++++++++++++++++++++++++++++++++++++++++++++--------------- >> 1 file changed, 47 insertions(+), 16 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index a11e948..9aafdab 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1428,7 +1428,7 @@ static inline int try_split_thp(struct page *page, struct page **page2, >> * @mode: The migration mode that specifies the constraints for >> * page migration, if any. >> * @reason: The reason for page migration. >> - * @ret_succeeded: Set to the number of pages migrated successfully if >> + * @ret_succeeded: Set to the number of normal pages migrated successfully if >> * the caller passes a non-NULL pointer. >> * >> * The function returns after 10 attempts or if no pages are movable any more >> @@ -1436,7 +1436,9 @@ static inline int try_split_thp(struct page *page, struct page **page2, >> * It is caller's responsibility to call putback_movable_pages() to return pages >> * to the LRU or free list only if ret != 0. >> * >> - * Returns the number of pages that were not migrated, or an error code. >> + * Returns the number of {normal page, THP} that were not migrated, or an error code. >> + * The number of THP splits will be considered as the number of non-migrated THP, >> + * no matter how many subpages of the THP are migrated successfully. >> */ >> int migrate_pages(struct list_head *from, new_page_t get_new_page, >> free_page_t put_new_page, unsigned long private, >> @@ -1445,6 +1447,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> int retry = 1; >> int thp_retry = 1; >> int nr_failed = 0; >> + int nr_failed_pages = 0; >> int nr_succeeded = 0; >> int nr_thp_succeeded = 0; >> int nr_thp_failed = 0; >> @@ -1456,13 +1459,16 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> int swapwrite = current->flags & PF_SWAPWRITE; >> int rc, nr_subpages; >> LIST_HEAD(ret_pages); >> + LIST_HEAD(thp_split_pages); >> bool nosplit = (reason == MR_NUMA_MISPLACED); >> + bool no_subpage_counting = false; >> >> trace_mm_migrate_pages_start(mode, reason); >> >> if (!swapwrite) >> current->flags |= PF_SWAPWRITE; >> >> +thp_subpage_migration: >> for (pass = 0; pass < 10 && (retry || thp_retry); pass++) { >> retry = 0; >> thp_retry = 0; >> @@ -1511,18 +1517,20 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> case -ENOSYS: >> /* THP migration is unsupported */ >> if (is_thp) { >> - if (!try_split_thp(page, &page2, from)) { >> + nr_thp_failed++; >> + if (!try_split_thp(page, &page2, &thp_split_pages)) { > > Does thp_split_pages need to be initialized before this call?
The declaration "LIST_HEAD(thp_split_pages);" already did list initialization, right?
| |