Re: [PATCH 1/3] mm: migrate: Fix the return value of migrate_pages()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxx>
Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
---
  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?




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux