[PATCH v3 0/4] A few cleanup and fixup patches for migration

[PATCH v3 0/4] A few cleanup and fixup patches for migration

[Miaohe Lin]

Hi everyone,
This series contains a few patches to remove unneeded lock page and
PageMovable check, reduce the rcu lock duration. Also we fix potential
pte_unmap on an not mapped pte. More details can be found in the
respective changelogs. Thanks!

---
v3:
[PATCH 1/4] mm/migration: reduce the rcu lock duration
  add the analysis that the original race condition isn't possible now
  alsoreduce the rcu lock duration in kernel_migrate_pages
[PATCH 2/4] mm/migration: remove unneeded lock page and PageMovable check
  let free_pages_prepare() clear PG_isolated for us
[PATCH 3/4] mm/migration: return errno when isolate_huge_page failed
  rename isolate_huge_page to isolate_hugetlb
[PATCH 4/4] mm/migration: fix potential pte_unmap on an not mapped pte
  collect Reviewed-by tag
v2:
  collect Reviewed-by tag
  make isolate_huge_page consistent with isolate_lru_page
  add hugetlbfs variant of hugetlb_migration_entry_wait
v1:
  rebase [1] on mainline.

[1] https://lore.kernel.org/lkml/20220304093409.25829-2-linmiaohe@huawei.com/T/
---
Miaohe Lin (4):
  mm: reduce the rcu lock duration
  mm/migration: remove unneeded lock page and PageMovable check
  mm/migration: return errno when isolate_huge_page failed
  mm/migration: fix potential pte_unmap on an not mapped pte

 include/linux/hugetlb.h |  6 +++---
 include/linux/swapops.h | 12 ++++++++----
 mm/gup.c                |  2 +-
 mm/hugetlb.c            | 15 +++++++--------
 mm/memory-failure.c     |  2 +-
 mm/mempolicy.c          |  5 ++---
 mm/migrate.c            | 40 +++++++++++++++++++++++++---------------
 7 files changed, 47 insertions(+), 35 deletions(-)

-- 
2.23.0

[PATCH v3 1/4] mm: reduce the rcu lock duration

[Miaohe Lin]

Commit 3268c63eded4 ("mm: fix move/migrate_pages() race on task struct")
extends the period of the rcu_read_lock until after the permissions checks
are done to prevent the task pointed to from changing from under us. But
the task_struct refcount is also taken at that time, the reference to task
is guaranteed to be stable. So it's unnecessary to extend the period of
the rcu_read_lock. Release the rcu lock after task refcount is successfully
grabbed to reduce the rcu holding time.

Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Christoph Lameter <cl@linux.com>
---
 mm/mempolicy.c | 3 +--
 mm/migrate.c   | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 0b4ba3ee810e..2dad094177bf 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1609,6 +1609,7 @@ static int kernel_migrate_pages(pid_t pid, unsigned long maxnode,
 		goto out;
 	}
 	get_task_struct(task);
+	rcu_read_unlock();
 
 	err = -EINVAL;
 
@@ -1617,11 +1618,9 @@ static int kernel_migrate_pages(pid_t pid, unsigned long maxnode,
 	 * Use the regular "ptrace_may_access()" checks.
 	 */
 	if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
-		rcu_read_unlock();
 		err = -EPERM;
 		goto out_put;
 	}
-	rcu_read_unlock();
 
 	task_nodes = cpuset_mems_allowed(task);
 	/* Is the user allowed to access the target nodes? */
diff --git a/mm/migrate.c b/mm/migrate.c
index e51588e95f57..e88ebb88fa6f 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1902,17 +1902,16 @@ static struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes)
 		return ERR_PTR(-ESRCH);
 	}
 	get_task_struct(task);
+	rcu_read_unlock();
 
 	/*
 	 * Check if this process has the right to modify the specified
 	 * process. Use the regular "ptrace_may_access()" checks.
 	 */
 	if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
-		rcu_read_unlock();
 		mm = ERR_PTR(-EPERM);
 		goto out;
 	}
-	rcu_read_unlock();
 
 	mm = ERR_PTR(security_task_movememory(task));
 	if (IS_ERR(mm))
-- 
2.23.0

[PATCH v3 2/4] mm/migration: remove unneeded lock page and PageMovable check

[Miaohe Lin]

When non-lru movable page was freed from under us, __ClearPageMovable must
have been done.  So we can remove unneeded lock page and PageMovable check
here. Also free_pages_prepare() will clear PG_isolated for us, so we can
further remove ClearPageIsolated as suggested by David.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 mm/migrate.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index e88ebb88fa6f..337336115e43 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1090,15 +1090,10 @@ static int unmap_and_move(new_page_t get_new_page,
 		return -ENOSYS;
 
 	if (page_count(page) == 1) {
-		/* page was freed from under us. So we are done. */
+		/* Page was freed from under us. So we are done. */
 		ClearPageActive(page);
 		ClearPageUnevictable(page);
-		if (unlikely(__PageMovable(page))) {
-			lock_page(page);
-			if (!PageMovable(page))
-				ClearPageIsolated(page);
-			unlock_page(page);
-		}
+		/* free_pages_prepare() will clear PG_isolated. */
 		goto out;
 	}
 
-- 
2.23.0

[PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed

[Miaohe Lin]

We might fail to isolate huge page due to e.g. the page is under migration
which cleared HPageMigratable. We should return errno in this case rather
than always return 1 which could confuse the user, i.e. the caller might
think all of the memory is migrated while the hugetlb page is left behind.
We make the prototype of isolate_huge_page consistent with isolate_lru_page
as suggested by Huang Ying and rename isolate_huge_page to isolate_hugetlb
as suggested by Muchun to improve the readability.

Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration")
Suggested-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 include/linux/hugetlb.h |  6 +++---
 mm/gup.c                |  2 +-
 mm/hugetlb.c            | 11 +++++------
 mm/memory-failure.c     |  2 +-
 mm/mempolicy.c          |  2 +-
 mm/migrate.c            |  5 +++--
 6 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index e4cff27d1198..756b66ff025e 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -170,7 +170,7 @@ bool hugetlb_reserve_pages(struct inode *inode, long from, long to,
 						vm_flags_t vm_flags);
 long hugetlb_unreserve_pages(struct inode *inode, long start, long end,
 						long freed);
-bool isolate_huge_page(struct page *page, struct list_head *list);
+int isolate_hugetlb(struct page *page, struct list_head *list);
 int get_hwpoison_huge_page(struct page *page, bool *hugetlb);
 int get_huge_page_for_hwpoison(unsigned long pfn, int flags);
 void putback_active_hugepage(struct page *page);
@@ -376,9 +376,9 @@ static inline pte_t *huge_pte_offset(struct mm_struct *mm, unsigned long addr,
 	return NULL;
 }
 
-static inline bool isolate_huge_page(struct page *page, struct list_head *list)
+static inline int isolate_hugetlb(struct page *page, struct list_head *list)
 {
-	return false;
+	return -EBUSY;
 }
 
 static inline int get_hwpoison_huge_page(struct page *page, bool *hugetlb)
diff --git a/mm/gup.c b/mm/gup.c
index 551264407624..3899dcb288a6 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1898,7 +1898,7 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
 		 * Try to move out any movable page before pinning the range.
 		 */
 		if (folio_test_hugetlb(folio)) {
-			if (!isolate_huge_page(&folio->page,
+			if (isolate_hugetlb(&folio->page,
 						&movable_page_list))
 				isolation_error_count++;
 			continue;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 01f0e2e5ab48..2026fcfc8886 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2765,8 +2765,7 @@ static int alloc_and_dissolve_huge_page(struct hstate *h, struct page *old_page,
 		 * Fail with -EBUSY if not possible.
 		 */
 		spin_unlock_irq(&hugetlb_lock);
-		if (!isolate_huge_page(old_page, list))
-			ret = -EBUSY;
+		ret = isolate_hugetlb(old_page, list);
 		spin_lock_irq(&hugetlb_lock);
 		goto free_new;
 	} else if (!HPageFreed(old_page)) {
@@ -2842,7 +2841,7 @@ int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list)
 	if (hstate_is_gigantic(h))
 		return -ENOMEM;
 
-	if (page_count(head) && isolate_huge_page(head, list))
+	if (page_count(head) && !isolate_hugetlb(head, list))
 		ret = 0;
 	else if (!page_count(head))
 		ret = alloc_and_dissolve_huge_page(h, head, list);
@@ -6945,15 +6944,15 @@ follow_huge_pgd(struct mm_struct *mm, unsigned long address, pgd_t *pgd, int fla
 	return pte_page(*(pte_t *)pgd) + ((address & ~PGDIR_MASK) >> PAGE_SHIFT);
 }
 
-bool isolate_huge_page(struct page *page, struct list_head *list)
+int isolate_hugetlb(struct page *page, struct list_head *list)
 {
-	bool ret = true;
+	int ret = 0;
 
 	spin_lock_irq(&hugetlb_lock);
 	if (!PageHeadHuge(page) ||
 	    !HPageMigratable(page) ||
 	    !get_page_unless_zero(page)) {
-		ret = false;
+		ret = -EBUSY;
 		goto unlock;
 	}
 	ClearHPageMigratable(page);
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index b85661cbdc4a..5deb1b1cb2fd 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2166,7 +2166,7 @@ static bool isolate_page(struct page *page, struct list_head *pagelist)
 	bool lru = PageLRU(page);
 
 	if (PageHuge(page)) {
-		isolated = isolate_huge_page(page, pagelist);
+		isolated = !isolate_hugetlb(page, pagelist);
 	} else {
 		if (lru)
 			isolated = !isolate_lru_page(page);
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 2dad094177bf..f96d55131689 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -602,7 +602,7 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
 	/* With MPOL_MF_MOVE, we migrate only unshared hugepage. */
 	if (flags & (MPOL_MF_MOVE_ALL) ||
 	    (flags & MPOL_MF_MOVE && page_mapcount(page) == 1)) {
-		if (!isolate_huge_page(page, qp->pagelist) &&
+		if (isolate_hugetlb(page, qp->pagelist) &&
 			(flags & MPOL_MF_STRICT))
 			/*
 			 * Failed to isolate page but allow migrating pages
diff --git a/mm/migrate.c b/mm/migrate.c
index 337336115e43..97c31b87d1a2 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1627,8 +1627,9 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
 
 	if (PageHuge(page)) {
 		if (PageHead(page)) {
-			isolate_huge_page(page, pagelist);
-			err = 1;
+			err = isolate_hugetlb(page, pagelist);
+			if (!err)
+				err = 1;
 		}
 	} else {
 		struct page *head;
-- 
2.23.0

[PATCH v3 4/4] mm/migration: fix potential pte_unmap on an not mapped pte

[Miaohe Lin]

__migration_entry_wait and migration_entry_wait_on_locked assume pte is
always mapped from caller. But this is not the case when it's called from
migration_entry_wait_huge and follow_huge_pmd. Add a hugetlbfs variant that
calls hugetlb_migration_entry_wait(ptep == NULL) to fix this issue.

Fixes: 30dad30922cc ("mm: migration: add migrate_entry_wait_huge()")
Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
---
 include/linux/swapops.h | 12 ++++++++----
 mm/hugetlb.c            |  4 ++--
 mm/migrate.c            | 23 +++++++++++++++++++----
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index f24775b41880..bb7afd03a324 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -244,8 +244,10 @@ extern void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,
 					spinlock_t *ptl);
 extern void migration_entry_wait(struct mm_struct *mm, pmd_t *pmd,
 					unsigned long address);
-extern void migration_entry_wait_huge(struct vm_area_struct *vma,
-		struct mm_struct *mm, pte_t *pte);
+#ifdef CONFIG_HUGETLB_PAGE
+extern void __migration_entry_wait_huge(pte_t *ptep, spinlock_t *ptl);
+extern void migration_entry_wait_huge(struct vm_area_struct *vma, pte_t *pte);
+#endif
 #else
 static inline swp_entry_t make_readable_migration_entry(pgoff_t offset)
 {
@@ -271,8 +273,10 @@ static inline void __migration_entry_wait(struct mm_struct *mm, pte_t *ptep,
 					spinlock_t *ptl) { }
 static inline void migration_entry_wait(struct mm_struct *mm, pmd_t *pmd,
 					 unsigned long address) { }
-static inline void migration_entry_wait_huge(struct vm_area_struct *vma,
-		struct mm_struct *mm, pte_t *pte) { }
+#ifdef CONFIG_HUGETLB_PAGE
+static inline void __migration_entry_wait_huge(pte_t *ptep, spinlock_t *ptl) { }
+static inline void migration_entry_wait_huge(struct vm_area_struct *vma, pte_t *pte) { }
+#endif
 static inline int is_writable_migration_entry(swp_entry_t entry)
 {
 	return 0;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2026fcfc8886..5015270cca12 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5694,7 +5694,7 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		 */
 		entry = huge_ptep_get(ptep);
 		if (unlikely(is_hugetlb_entry_migration(entry))) {
-			migration_entry_wait_huge(vma, mm, ptep);
+			migration_entry_wait_huge(vma, ptep);
 			return 0;
 		} else if (unlikely(is_hugetlb_entry_hwpoisoned(entry)))
 			return VM_FAULT_HWPOISON_LARGE |
@@ -6912,7 +6912,7 @@ follow_huge_pmd(struct mm_struct *mm, unsigned long address,
 	} else {
 		if (is_hugetlb_entry_migration(pte)) {
 			spin_unlock(ptl);
-			__migration_entry_wait(mm, (pte_t *)pmd, ptl);
+			__migration_entry_wait_huge((pte_t *)pmd, ptl);
 			goto retry;
 		}
 		/*
diff --git a/mm/migrate.c b/mm/migrate.c
index 97c31b87d1a2..581dd2b4dcbb 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -315,13 +315,28 @@ void migration_entry_wait(struct mm_struct *mm, pmd_t *pmd,
 	__migration_entry_wait(mm, ptep, ptl);
 }
 
-void migration_entry_wait_huge(struct vm_area_struct *vma,
-		struct mm_struct *mm, pte_t *pte)
+#ifdef CONFIG_HUGETLB_PAGE
+void __migration_entry_wait_huge(pte_t *ptep, spinlock_t *ptl)
 {
-	spinlock_t *ptl = huge_pte_lockptr(hstate_vma(vma), mm, pte);
-	__migration_entry_wait(mm, pte, ptl);
+	pte_t pte;
+
+	spin_lock(ptl);
+	pte = huge_ptep_get(ptep);
+
+	if (unlikely(!is_hugetlb_entry_migration(pte)))
+		spin_unlock(ptl);
+	else
+		migration_entry_wait_on_locked(pte_to_swp_entry(pte), NULL, ptl);
 }
 
+void migration_entry_wait_huge(struct vm_area_struct *vma, pte_t *pte)
+{
+	spinlock_t *ptl = huge_pte_lockptr(hstate_vma(vma), vma->vm_mm, pte);
+
+	__migration_entry_wait_huge(pte, ptl);
+}
+#endif
+
 #ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
 void pmd_migration_entry_wait(struct mm_struct *mm, pmd_t *pmd)
 {
-- 
2.23.0

Re: [PATCH v3 1/4] mm: reduce the rcu lock duration

[Oscar Salvador]

On Wed, May 25, 2022 at 04:18:19PM +0800, Miaohe Lin wrote:
> Commit 3268c63eded4 ("mm: fix move/migrate_pages() race on task struct")
> extends the period of the rcu_read_lock until after the permissions checks
> are done to prevent the task pointed to from changing from under us. But
> the task_struct refcount is also taken at that time, the reference to task
> is guaranteed to be stable. So it's unnecessary to extend the period of
> the rcu_read_lock. Release the rcu lock after task refcount is successfully
> grabbed to reduce the rcu holding time.
> 
> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Christoph Lameter <cl@linux.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/mempolicy.c | 3 +--
>  mm/migrate.c   | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0b4ba3ee810e..2dad094177bf 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1609,6 +1609,7 @@ static int kernel_migrate_pages(pid_t pid, unsigned long maxnode,
>  		goto out;
>  	}
>  	get_task_struct(task);
> +	rcu_read_unlock();
>  
>  	err = -EINVAL;
>  
> @@ -1617,11 +1618,9 @@ static int kernel_migrate_pages(pid_t pid, unsigned long maxnode,
>  	 * Use the regular "ptrace_may_access()" checks.
>  	 */
>  	if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
> -		rcu_read_unlock();
>  		err = -EPERM;
>  		goto out_put;
>  	}
> -	rcu_read_unlock();
>  
>  	task_nodes = cpuset_mems_allowed(task);
>  	/* Is the user allowed to access the target nodes? */
> diff --git a/mm/migrate.c b/mm/migrate.c
> index e51588e95f57..e88ebb88fa6f 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1902,17 +1902,16 @@ static struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes)
>  		return ERR_PTR(-ESRCH);
>  	}
>  	get_task_struct(task);
> +	rcu_read_unlock();
>  
>  	/*
>  	 * Check if this process has the right to modify the specified
>  	 * process. Use the regular "ptrace_may_access()" checks.
>  	 */
>  	if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) {
> -		rcu_read_unlock();
>  		mm = ERR_PTR(-EPERM);
>  		goto out;
>  	}
> -	rcu_read_unlock();
>  
>  	mm = ERR_PTR(security_task_movememory(task));
>  	if (IS_ERR(mm))
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v3 2/4] mm/migration: remove unneeded lock page and PageMovable check

[Oscar Salvador]

On Wed, May 25, 2022 at 04:18:20PM +0800, Miaohe Lin wrote:
> When non-lru movable page was freed from under us, __ClearPageMovable must
> have been done.  So we can remove unneeded lock page and PageMovable check
> here. Also free_pages_prepare() will clear PG_isolated for us, so we can
> further remove ClearPageIsolated as suggested by David.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> ---
>  mm/migrate.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index e88ebb88fa6f..337336115e43 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1090,15 +1090,10 @@ static int unmap_and_move(new_page_t get_new_page,
>  		return -ENOSYS;
>  
>  	if (page_count(page) == 1) {
> -		/* page was freed from under us. So we are done. */
> +		/* Page was freed from under us. So we are done. */
>  		ClearPageActive(page);
>  		ClearPageUnevictable(page);
> -		if (unlikely(__PageMovable(page))) {
> -			lock_page(page);
> -			if (!PageMovable(page))
> -				ClearPageIsolated(page);
> -			unlock_page(page);
> -		}
> +		/* free_pages_prepare() will clear PG_isolated. */
>  		goto out;
>  	}
>  
> -- 
> 2.23.0
> 
> 

-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed

[Oscar Salvador]

On Wed, May 25, 2022 at 04:18:21PM +0800, Miaohe Lin wrote:
> We might fail to isolate huge page due to e.g. the page is under migration
> which cleared HPageMigratable. We should return errno in this case rather
> than always return 1 which could confuse the user, i.e. the caller might
> think all of the memory is migrated while the hugetlb page is left behind.
> We make the prototype of isolate_huge_page consistent with isolate_lru_page
> as suggested by Huang Ying and rename isolate_huge_page to isolate_hugetlb
> as suggested by Muchun to improve the readability.
> 
> Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration")
> Suggested-by: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Looks good to me, one thing below though:

> ---
>  include/linux/hugetlb.h |  6 +++---
>  mm/gup.c                |  2 +-
>  mm/hugetlb.c            | 11 +++++------
>  mm/memory-failure.c     |  2 +-
>  mm/mempolicy.c          |  2 +-
>  mm/migrate.c            |  5 +++--
>  6 files changed, 14 insertions(+), 14 deletions(-)
> 
...
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -1627,8 +1627,9 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
>  
>  	if (PageHuge(page)) {
>  		if (PageHead(page)) {
> -			isolate_huge_page(page, pagelist);
> -			err = 1;
> +			err = isolate_hugetlb(page, pagelist);
> +			if (!err)
> +				err = 1;
>  		}

We used to always return 1 which means page has been queued for migration, as we
did not check isolate_huge_page() return value.
Now, we either return 1 or 0 depending on isolate_hugetlb(). 
I guess that was fine because in the end, if isolate_huge_page() failed,
the page just does not get added to 'pagelist', right? So, it is just
confusing for the user because he might not get an error so he thinks
the page will be migrated, and yet will not?


-- 
Oscar Salvador
SUSE Labs

Re: [PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed

[kernel test robot]

Hi Miaohe,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v5.18 next-20220525]
[cannot apply to hch-configfs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Miaohe-Lin/A-few-cleanup-and-fixup-patches-for-migration/20220525-162042
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20220526/202205260528.uxNyPadV-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d52a6e75b0c402c7f3b42a2b1b2873f151220947)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/bd48c43874403db1bccb576eae4a0e475942a64b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Miaohe-Lin/A-few-cleanup-and-fixup-patches-for-migration/20220525-162042
        git checkout bd48c43874403db1bccb576eae4a0e475942a64b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> mm/memory_hotplug.c:1646:4: error: call to undeclared function 'isolate_huge_page'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                           isolate_huge_page(head, &source);
                           ^
   1 error generated.


vim +/isolate_huge_page +1646 mm/memory_hotplug.c

0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1624  
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1625  static int
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1626  do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1627  {
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1628  	unsigned long pfn;
6c357848b44b401 Matthew Wilcox (Oracle  2020-08-14  1629) 	struct page *page, *head;
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1630  	int ret = 0;
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1631  	LIST_HEAD(source);
786dee864804f8e Liam Mark               2021-06-30  1632  	static DEFINE_RATELIMIT_STATE(migrate_rs, DEFAULT_RATELIMIT_INTERVAL,
786dee864804f8e Liam Mark               2021-06-30  1633  				      DEFAULT_RATELIMIT_BURST);
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1634  
a85009c377929d1 Michal Hocko            2018-12-28  1635  	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1636) 		struct folio *folio;
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1637) 
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1638  		if (!pfn_valid(pfn))
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1639  			continue;
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1640  		page = pfn_to_page(pfn);
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1641) 		folio = page_folio(page);
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1642) 		head = &folio->page;
c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1643  
c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1644  		if (PageHuge(page)) {
d8c6546b1aea843 Matthew Wilcox (Oracle  2019-09-23  1645) 			pfn = page_to_pfn(head) + compound_nr(head) - 1;
daf3538ad5a4800 Oscar Salvador          2019-03-05 @1646  			isolate_huge_page(head, &source);
c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1647  			continue;
94723aafb9e7641 Michal Hocko            2018-04-10  1648  		} else if (PageTransHuge(page))
6c357848b44b401 Matthew Wilcox (Oracle  2020-08-14  1649) 			pfn = page_to_pfn(head) + thp_nr_pages(page) - 1;
c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1650  
b15c87263a69272 Michal Hocko            2018-12-28  1651  		/*
b15c87263a69272 Michal Hocko            2018-12-28  1652  		 * HWPoison pages have elevated reference counts so the migration would
b15c87263a69272 Michal Hocko            2018-12-28  1653  		 * fail on them. It also doesn't make any sense to migrate them in the
b15c87263a69272 Michal Hocko            2018-12-28  1654  		 * first place. Still try to unmap such a page in case it is still mapped
b15c87263a69272 Michal Hocko            2018-12-28  1655  		 * (e.g. current hwpoison implementation doesn't unmap KSM pages but keep
b15c87263a69272 Michal Hocko            2018-12-28  1656  		 * the unmap as the catch all safety net).
b15c87263a69272 Michal Hocko            2018-12-28  1657  		 */
b15c87263a69272 Michal Hocko            2018-12-28  1658  		if (PageHWPoison(page)) {
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1659) 			if (WARN_ON(folio_test_lru(folio)))
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1660) 				folio_isolate_lru(folio);
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1661) 			if (folio_mapped(folio))
869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1662) 				try_to_unmap(folio, TTU_IGNORE_MLOCK);
b15c87263a69272 Michal Hocko            2018-12-28  1663  			continue;
b15c87263a69272 Michal Hocko            2018-12-28  1664  		}
b15c87263a69272 Michal Hocko            2018-12-28  1665  
700c2a46e882653 Konstantin Khlebnikov   2011-05-24  1666  		if (!get_page_unless_zero(page))
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1667  			continue;
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1668  		/*
0efadf48bca01f1 Yisheng Xie             2017-02-24  1669  		 * We can skip free pages. And we can deal with pages on
0efadf48bca01f1 Yisheng Xie             2017-02-24  1670  		 * LRU and non-lru movable pages.
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1671  		 */
0efadf48bca01f1 Yisheng Xie             2017-02-24  1672  		if (PageLRU(page))
62695a84eb8f2e7 Nicholas Piggin         2008-10-18  1673  			ret = isolate_lru_page(page);
0efadf48bca01f1 Yisheng Xie             2017-02-24  1674  		else
0efadf48bca01f1 Yisheng Xie             2017-02-24  1675  			ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE);
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1676  		if (!ret) { /* Success */
62695a84eb8f2e7 Nicholas Piggin         2008-10-18  1677  			list_add_tail(&page->lru, &source);
0efadf48bca01f1 Yisheng Xie             2017-02-24  1678  			if (!__PageMovable(page))
599d0c954f91d06 Mel Gorman              2016-07-28  1679  				inc_node_page_state(page, NR_ISOLATED_ANON +
9de4f22a60f7319 Huang Ying              2020-04-06  1680  						    page_is_file_lru(page));
6d9c285a632b39a KOSAKI Motohiro         2009-12-14  1681  
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1682  		} else {
786dee864804f8e Liam Mark               2021-06-30  1683  			if (__ratelimit(&migrate_rs)) {
2932c8b05056d4b Michal Hocko            2018-12-28  1684  				pr_warn("failed to isolate pfn %lx\n", pfn);
0efadf48bca01f1 Yisheng Xie             2017-02-24  1685  				dump_page(page, "isolation failed");
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1686  			}
786dee864804f8e Liam Mark               2021-06-30  1687  		}
1723058eab19de7 Oscar Salvador          2019-02-01  1688  		put_page(page);
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1689  	}
f3ab2636c5c1dd9 Bob Liu                 2010-10-26  1690  	if (!list_empty(&source)) {
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1691  		nodemask_t nmask = node_states[N_MEMORY];
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1692  		struct migration_target_control mtc = {
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1693  			.nmask = &nmask,
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1694  			.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1695  		};
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1696  
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1697  		/*
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1698  		 * We have checked that migration range is on a single zone so
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1699  		 * we can use the nid of the first page to all the others.
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1700  		 */
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1701  		mtc.nid = page_to_nid(list_first_entry(&source, struct page, lru));
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1702  
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1703  		/*
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1704  		 * try to allocate from a different node but reuse this node
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1705  		 * if there are no other online nodes to be used (e.g. we are
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1706  		 * offlining a part of the only existing node)
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1707  		 */
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1708  		node_clear(mtc.nid, nmask);
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1709  		if (nodes_empty(nmask))
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1710  			node_set(mtc.nid, nmask);
203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1711  		ret = migrate_pages(&source, alloc_migration_target, NULL,
5ac95884a784e82 Yang Shi                2021-09-02  1712  			(unsigned long)&mtc, MIGRATE_SYNC, MR_MEMORY_HOTPLUG, NULL);
2932c8b05056d4b Michal Hocko            2018-12-28  1713  		if (ret) {
2932c8b05056d4b Michal Hocko            2018-12-28  1714  			list_for_each_entry(page, &source, lru) {
786dee864804f8e Liam Mark               2021-06-30  1715  				if (__ratelimit(&migrate_rs)) {
786dee864804f8e Liam Mark               2021-06-30  1716  					pr_warn("migrating pfn %lx failed ret:%d\n",
2932c8b05056d4b Michal Hocko            2018-12-28  1717  						page_to_pfn(page), ret);
2932c8b05056d4b Michal Hocko            2018-12-28  1718  					dump_page(page, "migration failure");
2932c8b05056d4b Michal Hocko            2018-12-28  1719  				}
786dee864804f8e Liam Mark               2021-06-30  1720  			}
c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1721  			putback_movable_pages(&source);
f3ab2636c5c1dd9 Bob Liu                 2010-10-26  1722  		}
2932c8b05056d4b Michal Hocko            2018-12-28  1723  	}
1723058eab19de7 Oscar Salvador          2019-02-01  1724  
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1725  	return ret;
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1726  }
0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1727  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed

[Miaohe Lin]

On 2022/5/26 5:30, kernel test robot wrote:
> Hi Miaohe,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on akpm-mm/mm-everything]
> [also build test ERROR on linus/master v5.18 next-20220525]
> [cannot apply to hch-configfs/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Miaohe-Lin/A-few-cleanup-and-fixup-patches-for-migration/20220525-162042
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20220526/202205260528.uxNyPadV-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d52a6e75b0c402c7f3b42a2b1b2873f151220947)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/bd48c43874403db1bccb576eae4a0e475942a64b
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Miaohe-Lin/A-few-cleanup-and-fixup-patches-for-migration/20220525-162042
>         git checkout bd48c43874403db1bccb576eae4a0e475942a64b
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>

Thanks robot! I forgot to enable the CONFIG_MEMORY_HOTREMOVE. Sorry about it. Will fix this in next version.

Thanks!

> 
> All errors (new ones prefixed by >>):
> 
>>> mm/memory_hotplug.c:1646:4: error: call to undeclared function 'isolate_huge_page'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>                            isolate_huge_page(head, &source);
>                            ^
>    1 error generated.
> 
> 
> vim +/isolate_huge_page +1646 mm/memory_hotplug.c
> 
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1624  
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1625  static int
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1626  do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1627  {
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1628  	unsigned long pfn;
> 6c357848b44b401 Matthew Wilcox (Oracle  2020-08-14  1629) 	struct page *page, *head;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1630  	int ret = 0;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1631  	LIST_HEAD(source);
> 786dee864804f8e Liam Mark               2021-06-30  1632  	static DEFINE_RATELIMIT_STATE(migrate_rs, DEFAULT_RATELIMIT_INTERVAL,
> 786dee864804f8e Liam Mark               2021-06-30  1633  				      DEFAULT_RATELIMIT_BURST);
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1634  
> a85009c377929d1 Michal Hocko            2018-12-28  1635  	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1636) 		struct folio *folio;
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1637) 
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1638  		if (!pfn_valid(pfn))
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1639  			continue;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1640  		page = pfn_to_page(pfn);
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1641) 		folio = page_folio(page);
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1642) 		head = &folio->page;
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1643  
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1644  		if (PageHuge(page)) {
> d8c6546b1aea843 Matthew Wilcox (Oracle  2019-09-23  1645) 			pfn = page_to_pfn(head) + compound_nr(head) - 1;
> daf3538ad5a4800 Oscar Salvador          2019-03-05 @1646  			isolate_huge_page(head, &source);
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1647  			continue;
> 94723aafb9e7641 Michal Hocko            2018-04-10  1648  		} else if (PageTransHuge(page))
> 6c357848b44b401 Matthew Wilcox (Oracle  2020-08-14  1649) 			pfn = page_to_pfn(head) + thp_nr_pages(page) - 1;
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1650  
> b15c87263a69272 Michal Hocko            2018-12-28  1651  		/*
> b15c87263a69272 Michal Hocko            2018-12-28  1652  		 * HWPoison pages have elevated reference counts so the migration would
> b15c87263a69272 Michal Hocko            2018-12-28  1653  		 * fail on them. It also doesn't make any sense to migrate them in the
> b15c87263a69272 Michal Hocko            2018-12-28  1654  		 * first place. Still try to unmap such a page in case it is still mapped
> b15c87263a69272 Michal Hocko            2018-12-28  1655  		 * (e.g. current hwpoison implementation doesn't unmap KSM pages but keep
> b15c87263a69272 Michal Hocko            2018-12-28  1656  		 * the unmap as the catch all safety net).
> b15c87263a69272 Michal Hocko            2018-12-28  1657  		 */
> b15c87263a69272 Michal Hocko            2018-12-28  1658  		if (PageHWPoison(page)) {
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1659) 			if (WARN_ON(folio_test_lru(folio)))
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1660) 				folio_isolate_lru(folio);
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1661) 			if (folio_mapped(folio))
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1662) 				try_to_unmap(folio, TTU_IGNORE_MLOCK);
> b15c87263a69272 Michal Hocko            2018-12-28  1663  			continue;
> b15c87263a69272 Michal Hocko            2018-12-28  1664  		}
> b15c87263a69272 Michal Hocko            2018-12-28  1665  
> 700c2a46e882653 Konstantin Khlebnikov   2011-05-24  1666  		if (!get_page_unless_zero(page))
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1667  			continue;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1668  		/*
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1669  		 * We can skip free pages. And we can deal with pages on
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1670  		 * LRU and non-lru movable pages.
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1671  		 */
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1672  		if (PageLRU(page))
> 62695a84eb8f2e7 Nicholas Piggin         2008-10-18  1673  			ret = isolate_lru_page(page);
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1674  		else
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1675  			ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE);
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1676  		if (!ret) { /* Success */
> 62695a84eb8f2e7 Nicholas Piggin         2008-10-18  1677  			list_add_tail(&page->lru, &source);
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1678  			if (!__PageMovable(page))
> 599d0c954f91d06 Mel Gorman              2016-07-28  1679  				inc_node_page_state(page, NR_ISOLATED_ANON +
> 9de4f22a60f7319 Huang Ying              2020-04-06  1680  						    page_is_file_lru(page));
> 6d9c285a632b39a KOSAKI Motohiro         2009-12-14  1681  
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1682  		} else {
> 786dee864804f8e Liam Mark               2021-06-30  1683  			if (__ratelimit(&migrate_rs)) {
> 2932c8b05056d4b Michal Hocko            2018-12-28  1684  				pr_warn("failed to isolate pfn %lx\n", pfn);
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1685  				dump_page(page, "isolation failed");
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1686  			}
> 786dee864804f8e Liam Mark               2021-06-30  1687  		}
> 1723058eab19de7 Oscar Salvador          2019-02-01  1688  		put_page(page);
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1689  	}
> f3ab2636c5c1dd9 Bob Liu                 2010-10-26  1690  	if (!list_empty(&source)) {
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1691  		nodemask_t nmask = node_states[N_MEMORY];
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1692  		struct migration_target_control mtc = {
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1693  			.nmask = &nmask,
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1694  			.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1695  		};
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1696  
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1697  		/*
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1698  		 * We have checked that migration range is on a single zone so
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1699  		 * we can use the nid of the first page to all the others.
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1700  		 */
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1701  		mtc.nid = page_to_nid(list_first_entry(&source, struct page, lru));
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1702  
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1703  		/*
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1704  		 * try to allocate from a different node but reuse this node
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1705  		 * if there are no other online nodes to be used (e.g. we are
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1706  		 * offlining a part of the only existing node)
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1707  		 */
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1708  		node_clear(mtc.nid, nmask);
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1709  		if (nodes_empty(nmask))
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1710  			node_set(mtc.nid, nmask);
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1711  		ret = migrate_pages(&source, alloc_migration_target, NULL,
> 5ac95884a784e82 Yang Shi                2021-09-02  1712  			(unsigned long)&mtc, MIGRATE_SYNC, MR_MEMORY_HOTPLUG, NULL);
> 2932c8b05056d4b Michal Hocko            2018-12-28  1713  		if (ret) {
> 2932c8b05056d4b Michal Hocko            2018-12-28  1714  			list_for_each_entry(page, &source, lru) {
> 786dee864804f8e Liam Mark               2021-06-30  1715  				if (__ratelimit(&migrate_rs)) {
> 786dee864804f8e Liam Mark               2021-06-30  1716  					pr_warn("migrating pfn %lx failed ret:%d\n",
> 2932c8b05056d4b Michal Hocko            2018-12-28  1717  						page_to_pfn(page), ret);
> 2932c8b05056d4b Michal Hocko            2018-12-28  1718  					dump_page(page, "migration failure");
> 2932c8b05056d4b Michal Hocko            2018-12-28  1719  				}
> 786dee864804f8e Liam Mark               2021-06-30  1720  			}
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1721  			putback_movable_pages(&source);
> f3ab2636c5c1dd9 Bob Liu                 2010-10-26  1722  		}
> 2932c8b05056d4b Michal Hocko            2018-12-28  1723  	}
> 1723058eab19de7 Oscar Salvador          2019-02-01  1724  
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1725  	return ret;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1726  }
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1727  
> 

Re: [PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed

[Miaohe Lin]

[-- Attachment #1: Type: text/plain, Size: 11628 bytes --]

On 2022/5/26 5:30, kernel test robot wrote:
> Hi Miaohe,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on akpm-mm/mm-everything]
> [also build test ERROR on linus/master v5.18 next-20220525]
> [cannot apply to hch-configfs/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Miaohe-Lin/A-few-cleanup-and-fixup-patches-for-migration/20220525-162042
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> config: x86_64-randconfig-a005 (https://download.01.org/0day-ci/archive/20220526/202205260528.uxNyPadV-lkp(a)intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d52a6e75b0c402c7f3b42a2b1b2873f151220947)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/bd48c43874403db1bccb576eae4a0e475942a64b
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Miaohe-Lin/A-few-cleanup-and-fixup-patches-for-migration/20220525-162042
>         git checkout bd48c43874403db1bccb576eae4a0e475942a64b
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>

Thanks robot! I forgot to enable the CONFIG_MEMORY_HOTREMOVE. Sorry about it. Will fix this in next version.

Thanks!

> 
> All errors (new ones prefixed by >>):
> 
>>> mm/memory_hotplug.c:1646:4: error: call to undeclared function 'isolate_huge_page'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>                            isolate_huge_page(head, &source);
>                            ^
>    1 error generated.
> 
> 
> vim +/isolate_huge_page +1646 mm/memory_hotplug.c
> 
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1624  
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1625  static int
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1626  do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1627  {
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1628  	unsigned long pfn;
> 6c357848b44b401 Matthew Wilcox (Oracle  2020-08-14  1629) 	struct page *page, *head;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1630  	int ret = 0;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1631  	LIST_HEAD(source);
> 786dee864804f8e Liam Mark               2021-06-30  1632  	static DEFINE_RATELIMIT_STATE(migrate_rs, DEFAULT_RATELIMIT_INTERVAL,
> 786dee864804f8e Liam Mark               2021-06-30  1633  				      DEFAULT_RATELIMIT_BURST);
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1634  
> a85009c377929d1 Michal Hocko            2018-12-28  1635  	for (pfn = start_pfn; pfn < end_pfn; pfn++) {
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1636) 		struct folio *folio;
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1637) 
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1638  		if (!pfn_valid(pfn))
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1639  			continue;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1640  		page = pfn_to_page(pfn);
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1641) 		folio = page_folio(page);
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1642) 		head = &folio->page;
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1643  
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1644  		if (PageHuge(page)) {
> d8c6546b1aea843 Matthew Wilcox (Oracle  2019-09-23  1645) 			pfn = page_to_pfn(head) + compound_nr(head) - 1;
> daf3538ad5a4800 Oscar Salvador          2019-03-05 @1646  			isolate_huge_page(head, &source);
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1647  			continue;
> 94723aafb9e7641 Michal Hocko            2018-04-10  1648  		} else if (PageTransHuge(page))
> 6c357848b44b401 Matthew Wilcox (Oracle  2020-08-14  1649) 			pfn = page_to_pfn(head) + thp_nr_pages(page) - 1;
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1650  
> b15c87263a69272 Michal Hocko            2018-12-28  1651  		/*
> b15c87263a69272 Michal Hocko            2018-12-28  1652  		 * HWPoison pages have elevated reference counts so the migration would
> b15c87263a69272 Michal Hocko            2018-12-28  1653  		 * fail on them. It also doesn't make any sense to migrate them in the
> b15c87263a69272 Michal Hocko            2018-12-28  1654  		 * first place. Still try to unmap such a page in case it is still mapped
> b15c87263a69272 Michal Hocko            2018-12-28  1655  		 * (e.g. current hwpoison implementation doesn't unmap KSM pages but keep
> b15c87263a69272 Michal Hocko            2018-12-28  1656  		 * the unmap as the catch all safety net).
> b15c87263a69272 Michal Hocko            2018-12-28  1657  		 */
> b15c87263a69272 Michal Hocko            2018-12-28  1658  		if (PageHWPoison(page)) {
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1659) 			if (WARN_ON(folio_test_lru(folio)))
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1660) 				folio_isolate_lru(folio);
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1661) 			if (folio_mapped(folio))
> 869f7ee6f647734 Matthew Wilcox (Oracle  2022-02-15  1662) 				try_to_unmap(folio, TTU_IGNORE_MLOCK);
> b15c87263a69272 Michal Hocko            2018-12-28  1663  			continue;
> b15c87263a69272 Michal Hocko            2018-12-28  1664  		}
> b15c87263a69272 Michal Hocko            2018-12-28  1665  
> 700c2a46e882653 Konstantin Khlebnikov   2011-05-24  1666  		if (!get_page_unless_zero(page))
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1667  			continue;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1668  		/*
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1669  		 * We can skip free pages. And we can deal with pages on
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1670  		 * LRU and non-lru movable pages.
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1671  		 */
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1672  		if (PageLRU(page))
> 62695a84eb8f2e7 Nicholas Piggin         2008-10-18  1673  			ret = isolate_lru_page(page);
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1674  		else
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1675  			ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE);
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1676  		if (!ret) { /* Success */
> 62695a84eb8f2e7 Nicholas Piggin         2008-10-18  1677  			list_add_tail(&page->lru, &source);
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1678  			if (!__PageMovable(page))
> 599d0c954f91d06 Mel Gorman              2016-07-28  1679  				inc_node_page_state(page, NR_ISOLATED_ANON +
> 9de4f22a60f7319 Huang Ying              2020-04-06  1680  						    page_is_file_lru(page));
> 6d9c285a632b39a KOSAKI Motohiro         2009-12-14  1681  
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1682  		} else {
> 786dee864804f8e Liam Mark               2021-06-30  1683  			if (__ratelimit(&migrate_rs)) {
> 2932c8b05056d4b Michal Hocko            2018-12-28  1684  				pr_warn("failed to isolate pfn %lx\n", pfn);
> 0efadf48bca01f1 Yisheng Xie             2017-02-24  1685  				dump_page(page, "isolation failed");
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1686  			}
> 786dee864804f8e Liam Mark               2021-06-30  1687  		}
> 1723058eab19de7 Oscar Salvador          2019-02-01  1688  		put_page(page);
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1689  	}
> f3ab2636c5c1dd9 Bob Liu                 2010-10-26  1690  	if (!list_empty(&source)) {
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1691  		nodemask_t nmask = node_states[N_MEMORY];
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1692  		struct migration_target_control mtc = {
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1693  			.nmask = &nmask,
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1694  			.gfp_mask = GFP_USER | __GFP_MOVABLE | __GFP_RETRY_MAYFAIL,
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1695  		};
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1696  
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1697  		/*
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1698  		 * We have checked that migration range is on a single zone so
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1699  		 * we can use the nid of the first page to all the others.
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1700  		 */
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1701  		mtc.nid = page_to_nid(list_first_entry(&source, struct page, lru));
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1702  
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1703  		/*
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1704  		 * try to allocate from a different node but reuse this node
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1705  		 * if there are no other online nodes to be used (e.g. we are
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1706  		 * offlining a part of the only existing node)
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1707  		 */
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1708  		node_clear(mtc.nid, nmask);
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1709  		if (nodes_empty(nmask))
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1710  			node_set(mtc.nid, nmask);
> 203e6e5ca4eac64 Joonsoo Kim             2020-10-17  1711  		ret = migrate_pages(&source, alloc_migration_target, NULL,
> 5ac95884a784e82 Yang Shi                2021-09-02  1712  			(unsigned long)&mtc, MIGRATE_SYNC, MR_MEMORY_HOTPLUG, NULL);
> 2932c8b05056d4b Michal Hocko            2018-12-28  1713  		if (ret) {
> 2932c8b05056d4b Michal Hocko            2018-12-28  1714  			list_for_each_entry(page, &source, lru) {
> 786dee864804f8e Liam Mark               2021-06-30  1715  				if (__ratelimit(&migrate_rs)) {
> 786dee864804f8e Liam Mark               2021-06-30  1716  					pr_warn("migrating pfn %lx failed ret:%d\n",
> 2932c8b05056d4b Michal Hocko            2018-12-28  1717  						page_to_pfn(page), ret);
> 2932c8b05056d4b Michal Hocko            2018-12-28  1718  					dump_page(page, "migration failure");
> 2932c8b05056d4b Michal Hocko            2018-12-28  1719  				}
> 786dee864804f8e Liam Mark               2021-06-30  1720  			}
> c8721bbbdd36382 Naoya Horiguchi         2013-09-11  1721  			putback_movable_pages(&source);
> f3ab2636c5c1dd9 Bob Liu                 2010-10-26  1722  		}
> 2932c8b05056d4b Michal Hocko            2018-12-28  1723  	}
> 1723058eab19de7 Oscar Salvador          2019-02-01  1724  
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1725  	return ret;
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1726  }
> 0c0e61958965354 KAMEZAWA Hiroyuki       2007-10-16  1727  
> 

Re: [PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed

[Miaohe Lin]

On 2022/5/25 16:41, Oscar Salvador wrote:
> On Wed, May 25, 2022 at 04:18:21PM +0800, Miaohe Lin wrote:
>> We might fail to isolate huge page due to e.g. the page is under migration
>> which cleared HPageMigratable. We should return errno in this case rather
>> than always return 1 which could confuse the user, i.e. the caller might
>> think all of the memory is migrated while the hugetlb page is left behind.
>> We make the prototype of isolate_huge_page consistent with isolate_lru_page
>> as suggested by Huang Ying and rename isolate_huge_page to isolate_hugetlb
>> as suggested by Muchun to improve the readability.
>>
>> Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration")
>> Suggested-by: Huang Ying <ying.huang@intel.com>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> Looks good to me, one thing below though:
> 
>> ---
>>  include/linux/hugetlb.h |  6 +++---
>>  mm/gup.c                |  2 +-
>>  mm/hugetlb.c            | 11 +++++------
>>  mm/memory-failure.c     |  2 +-
>>  mm/mempolicy.c          |  2 +-
>>  mm/migrate.c            |  5 +++--
>>  6 files changed, 14 insertions(+), 14 deletions(-)
>>
> ...
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -1627,8 +1627,9 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
>>  
>>  	if (PageHuge(page)) {
>>  		if (PageHead(page)) {
>> -			isolate_huge_page(page, pagelist);
>> -			err = 1;
>> +			err = isolate_hugetlb(page, pagelist);
>> +			if (!err)
>> +				err = 1;
>>  		}
> 
> We used to always return 1 which means page has been queued for migration, as we
> did not check isolate_huge_page() return value.
> Now, we either return 1 or 0 depending on isolate_hugetlb(). 

Return 1 or -EBUSY just as normal page case.

> I guess that was fine because in the end, if isolate_huge_page() failed,
> the page just does not get added to 'pagelist', right? So, it is just
> confusing for the user because he might not get an error so he thinks
> the page will be migrated, and yet will not?

Yes, the hugetlb page might not be migrated due to error while it's not reported in the
__user *status. So the caller might think all of the memory is migrated and thus does
not retry to migrate the hugetlb page in the next round.

Many thanks for your review and comment! :)

> 
>