[PATCH mm-unstable 0/5]

[PATCH mm-unstable 0/5]

[Vishal Moola (Oracle)]

This patch series converts migrate_page_add() and queue_pages_required()
to migrate_folio_add() and queue_page_required(). It also converts the
callers of the functions to use folios as well.

Vishal Moola (Oracle) (5):
  mm/mempolicy: Convert queue_pages_pmd() to queue_folios_pmd()
  mm/mempolicy: Convert queue_pages_pte_range() to
    queue_folios_pte_range()
  mm/mempolicy: Convert queue_pages_hugetlb() to queue_folios_hugetlb()
  mm/mempolicy: Convert queue_pages_required() to queue_folio_required()
  mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

 mm/mempolicy.c | 108 ++++++++++++++++++++++++-------------------------
 1 file changed, 52 insertions(+), 56 deletions(-)

-- 
2.38.1

[PATCH mm-unstable 1/5] mm/mempolicy: Convert queue_pages_pmd() to queue_folios_pmd()

[Vishal Moola (Oracle)]

The function now operates on a folio instead of
the page associated with a pmd.

This change is in preparation for the conversion of
queue_pages_required() to queue_folio_required() and migrate_page_add()
to migrate_folio_add().

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/mempolicy.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index fd99d303e34f..00fffa93adae 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -442,21 +442,21 @@ static inline bool queue_pages_required(struct page *page,
 }
 
 /*
- * queue_pages_pmd() has three possible return values:
- * 0 - pages are placed on the right node or queued successfully, or
+ * queue_folios_pmd() has three possible return values:
+ * 0 - folios are placed on the right node or queued successfully, or
  *     special page is met, i.e. huge zero page.
- * 1 - there is unmovable page, and MPOL_MF_MOVE* & MPOL_MF_STRICT were
+ * 1 - there is unmovable folio, and MPOL_MF_MOVE* & MPOL_MF_STRICT were
  *     specified.
  * -EIO - is migration entry or only MPOL_MF_STRICT was specified and an
- *        existing page was already on a node that does not follow the
+ *        existing folio was already on a node that does not follow the
  *        policy.
  */
-static int queue_pages_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
+static int queue_folios_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
 				unsigned long end, struct mm_walk *walk)
 	__releases(ptl)
 {
 	int ret = 0;
-	struct page *page;
+	struct folio *folio;
 	struct queue_pages *qp = walk->private;
 	unsigned long flags;
 
@@ -464,19 +464,19 @@ static int queue_pages_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
 		ret = -EIO;
 		goto unlock;
 	}
-	page = pmd_page(*pmd);
-	if (is_huge_zero_page(page)) {
+	folio = pfn_folio(pmd_pfn(*pmd));
+	if (is_huge_zero_page(&folio->page)) {
 		walk->action = ACTION_CONTINUE;
 		goto unlock;
 	}
-	if (!queue_pages_required(page, qp))
+	if (!queue_pages_required(&folio->page, qp))
 		goto unlock;
 
 	flags = qp->flags;
-	/* go to thp migration */
+	/* go to folio migration */
 	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
 		if (!vma_migratable(walk->vma) ||
-		    migrate_page_add(page, qp->pagelist, flags)) {
+		    migrate_page_add(&folio->page, qp->pagelist, flags)) {
 			ret = 1;
 			goto unlock;
 		}
@@ -512,7 +512,7 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr,
 
 	ptl = pmd_trans_huge_lock(pmd, vma);
 	if (ptl)
-		return queue_pages_pmd(pmd, ptl, addr, end, walk);
+		return queue_folios_pmd(pmd, ptl, addr, end, walk);
 
 	if (pmd_trans_unstable(pmd))
 		return 0;
-- 
2.38.1

[PATCH mm-unstable 2/5] mm/mempolicy: Convert queue_pages_pte_range() to queue_folios_pte_range()

[Vishal Moola (Oracle)]

This function now operates on folios associated with ptes instead of
pages.

This change is in preparation for the conversion of
queue_pages_required() to queue_folio_required() and migrate_page_add()
to migrate_folio_add().

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/mempolicy.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 00fffa93adae..ae9d16124f45 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -491,19 +491,19 @@ static int queue_folios_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
  * Scan through pages checking if pages follow certain conditions,
  * and move them to the pagelist if they do.
  *
- * queue_pages_pte_range() has three possible return values:
- * 0 - pages are placed on the right node or queued successfully, or
+ * queue_folios_pte_range() has three possible return values:
+ * 0 - folios are placed on the right node or queued successfully, or
  *     special page is met, i.e. zero page.
- * 1 - there is unmovable page, and MPOL_MF_MOVE* & MPOL_MF_STRICT were
+ * 1 - there is unmovable folio, and MPOL_MF_MOVE* & MPOL_MF_STRICT were
  *     specified.
- * -EIO - only MPOL_MF_STRICT was specified and an existing page was already
+ * -EIO - only MPOL_MF_STRICT was specified and an existing folio was already
  *        on a node that does not follow the policy.
  */
-static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr,
+static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
 			unsigned long end, struct mm_walk *walk)
 {
 	struct vm_area_struct *vma = walk->vma;
-	struct page *page;
+	struct folio *folio;
 	struct queue_pages *qp = walk->private;
 	unsigned long flags = qp->flags;
 	bool has_unmovable = false;
@@ -521,16 +521,16 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr,
 	for (; addr != end; pte++, addr += PAGE_SIZE) {
 		if (!pte_present(*pte))
 			continue;
-		page = vm_normal_page(vma, addr, *pte);
-		if (!page || is_zone_device_page(page))
+		folio = vm_normal_folio(vma, addr, *pte);
+		if (!folio || folio_is_zone_device(folio))
 			continue;
 		/*
-		 * vm_normal_page() filters out zero pages, but there might
-		 * still be PageReserved pages to skip, perhaps in a VDSO.
+		 * vm_normal_folio() filters out zero pages, but there might
+		 * still be reserved folios to skip, perhaps in a VDSO.
 		 */
-		if (PageReserved(page))
+		if (folio_test_reserved(folio))
 			continue;
-		if (!queue_pages_required(page, qp))
+		if (!queue_pages_required(&folio->page, qp))
 			continue;
 		if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
 			/* MPOL_MF_STRICT must be specified if we get here */
@@ -544,7 +544,7 @@ static int queue_pages_pte_range(pmd_t *pmd, unsigned long addr,
 			 * temporary off LRU pages in the range.  Still
 			 * need migrate other LRU pages.
 			 */
-			if (migrate_page_add(page, qp->pagelist, flags))
+			if (migrate_page_add(&folio->page, qp->pagelist, flags))
 				has_unmovable = true;
 		} else
 			break;
@@ -703,7 +703,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
 
 static const struct mm_walk_ops queue_pages_walk_ops = {
 	.hugetlb_entry		= queue_pages_hugetlb,
-	.pmd_entry		= queue_pages_pte_range,
+	.pmd_entry		= queue_folios_pte_range,
 	.test_walk		= queue_pages_test_walk,
 };
 
-- 
2.38.1

[PATCH mm-unstable 3/5] mm/mempolicy: Convert queue_pages_hugetlb() to queue_folios_hugetlb()

[Vishal Moola (Oracle)]

This change is in preparation for the conversion of
queue_pages_required() to queue_folio_required() and migrate_page_add()
to migrate_folio_add().

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/mempolicy.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index ae9d16124f45..0b82f8159541 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -558,7 +558,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
 	return addr != end ? -EIO : 0;
 }
 
-static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
+static int queue_folios_hugetlb(pte_t *pte, unsigned long hmask,
 			       unsigned long addr, unsigned long end,
 			       struct mm_walk *walk)
 {
@@ -566,7 +566,7 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
 #ifdef CONFIG_HUGETLB_PAGE
 	struct queue_pages *qp = walk->private;
 	unsigned long flags = (qp->flags & MPOL_MF_VALID);
-	struct page *page;
+	struct folio *folio;
 	spinlock_t *ptl;
 	pte_t entry;
 
@@ -574,13 +574,13 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
 	entry = huge_ptep_get(pte);
 	if (!pte_present(entry))
 		goto unlock;
-	page = pte_page(entry);
-	if (!queue_pages_required(page, qp))
+	folio = pfn_folio(pte_pfn(entry));
+	if (!queue_pages_required(&folio->page, qp))
 		goto unlock;
 
 	if (flags == MPOL_MF_STRICT) {
 		/*
-		 * STRICT alone means only detecting misplaced page and no
+		 * STRICT alone means only detecting misplaced folio and no
 		 * need to further check other vma.
 		 */
 		ret = -EIO;
@@ -591,7 +591,7 @@ static int queue_pages_hugetlb(pte_t *pte, unsigned long hmask,
 		/*
 		 * Must be STRICT with MOVE*, otherwise .test_walk() have
 		 * stopped walking current vma.
-		 * Detecting misplaced page but allow migrating pages which
+		 * Detecting misplaced folio but allow migrating folios which
 		 * have been queued.
 		 */
 		ret = 1;
@@ -600,11 +600,11 @@ 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_hugetlb(page_folio(page), qp->pagelist) &&
+	    (flags & MPOL_MF_MOVE && folio_mapcount(folio) == 1)) {
+		if (isolate_hugetlb(folio, qp->pagelist) &&
 			(flags & MPOL_MF_STRICT))
 			/*
-			 * Failed to isolate page but allow migrating pages
+			 * Failed to isolate folio but allow migrating folios
 			 * which have been queued.
 			 */
 			ret = 1;
@@ -702,7 +702,7 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
 }
 
 static const struct mm_walk_ops queue_pages_walk_ops = {
-	.hugetlb_entry		= queue_pages_hugetlb,
+	.hugetlb_entry		= queue_folios_hugetlb,
 	.pmd_entry		= queue_folios_pte_range,
 	.test_walk		= queue_pages_test_walk,
 };
-- 
2.38.1

[PATCH mm-unstable 4/5] mm/mempolicy: Convert queue_pages_required() to queue_folio_required()

[Vishal Moola (Oracle)]

Replace queue_pages_required() with queue_folio_required().
queue_folio_required() does the same as queue_pages_required(), except
takes in a folio instead of a page.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/mempolicy.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 0b82f8159541..0a3690ecab7d 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -427,15 +427,15 @@ struct queue_pages {
 };
 
 /*
- * Check if the page's nid is in qp->nmask.
+ * Check if the folio's nid is in qp->nmask.
  *
  * If MPOL_MF_INVERT is set in qp->flags, check if the nid is
  * in the invert of qp->nmask.
  */
-static inline bool queue_pages_required(struct page *page,
+static inline bool queue_folio_required(struct folio *folio,
 					struct queue_pages *qp)
 {
-	int nid = page_to_nid(page);
+	int nid = folio_nid(folio);
 	unsigned long flags = qp->flags;
 
 	return node_isset(nid, *qp->nmask) == !(flags & MPOL_MF_INVERT);
@@ -469,7 +469,7 @@ static int queue_folios_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
 		walk->action = ACTION_CONTINUE;
 		goto unlock;
 	}
-	if (!queue_pages_required(&folio->page, qp))
+	if (!queue_folio_required(folio, qp))
 		goto unlock;
 
 	flags = qp->flags;
@@ -530,7 +530,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
 		 */
 		if (folio_test_reserved(folio))
 			continue;
-		if (!queue_pages_required(&folio->page, qp))
+		if (!queue_folio_required(folio, qp))
 			continue;
 		if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
 			/* MPOL_MF_STRICT must be specified if we get here */
@@ -575,7 +575,7 @@ static int queue_folios_hugetlb(pte_t *pte, unsigned long hmask,
 	if (!pte_present(entry))
 		goto unlock;
 	folio = pfn_folio(pte_pfn(entry));
-	if (!queue_pages_required(&folio->page, qp))
+	if (!queue_folio_required(folio, qp))
 		goto unlock;
 
 	if (flags == MPOL_MF_STRICT) {
-- 
2.38.1

[PATCH mm-unstable 5/5] mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

[Vishal Moola (Oracle)]

Replace migrate_page_add() with migrate_folio_add().
migrate_folio_add() does the same a migrate_page_add() but takes in a
folio instead of a page. This removes a couple of calls to
compound_head().

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/mempolicy.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 0a3690ecab7d..253ce368cf16 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -414,7 +414,7 @@ static const struct mempolicy_operations mpol_ops[MPOL_MAX] = {
 	},
 };
 
-static int migrate_page_add(struct page *page, struct list_head *pagelist,
+static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
 				unsigned long flags);
 
 struct queue_pages {
@@ -476,7 +476,7 @@ static int queue_folios_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
 	/* go to folio migration */
 	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
 		if (!vma_migratable(walk->vma) ||
-		    migrate_page_add(&folio->page, qp->pagelist, flags)) {
+		    migrate_folio_add(folio, qp->pagelist, flags)) {
 			ret = 1;
 			goto unlock;
 		}
@@ -544,7 +544,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
 			 * temporary off LRU pages in the range.  Still
 			 * need migrate other LRU pages.
 			 */
-			if (migrate_page_add(&folio->page, qp->pagelist, flags))
+			if (migrate_folio_add(folio, qp->pagelist, flags))
 				has_unmovable = true;
 		} else
 			break;
@@ -1022,27 +1022,23 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 }
 
 #ifdef CONFIG_MIGRATION
-/*
- * page migration, thp tail pages can be passed.
- */
-static int migrate_page_add(struct page *page, struct list_head *pagelist,
+static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
 				unsigned long flags)
 {
-	struct page *head = compound_head(page);
 	/*
-	 * Avoid migrating a page that is shared with others.
+	 * Avoid migrating a folio that is shared with others.
 	 */
-	if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
-		if (!isolate_lru_page(head)) {
-			list_add_tail(&head->lru, pagelist);
-			mod_node_page_state(page_pgdat(head),
-				NR_ISOLATED_ANON + page_is_file_lru(head),
-				thp_nr_pages(head));
+	if ((flags & MPOL_MF_MOVE_ALL) || folio_mapcount(folio) == 1) {
+		if (!folio_isolate_lru(folio)) {
+			list_add_tail(&folio->lru, foliolist);
+			node_stat_mod_folio(folio,
+				NR_ISOLATED_ANON + folio_is_file_lru(folio),
+				folio_nr_pages(folio));
 		} else if (flags & MPOL_MF_STRICT) {
 			/*
-			 * Non-movable page may reach here.  And, there may be
-			 * temporary off LRU pages or non-LRU movable pages.
-			 * Treat them as unmovable pages since they can't be
+			 * Non-movable folio may reach here.  And, there may be
+			 * temporary off LRU folios or non-LRU movable folios.
+			 * Treat them as unmovable folios since they can't be
 			 * isolated, so they can't be moved at the moment.  It
 			 * should return -EIO for this case too.
 			 */
@@ -1234,7 +1230,7 @@ static struct page *new_page(struct page *page, unsigned long start)
 }
 #else
 
-static int migrate_page_add(struct page *page, struct list_head *pagelist,
+static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
 				unsigned long flags)
 {
 	return -EIO;
-- 
2.38.1

Re: [PATCH mm-unstable 5/5] mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

[Yin, Fengwei]

On 1/19/2023 7:22 AM, Vishal Moola (Oracle) wrote:
> Replace migrate_page_add() with migrate_folio_add().
> migrate_folio_add() does the same a migrate_page_add() but takes in a
> folio instead of a page. This removes a couple of calls to
> compound_head().
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
>  mm/mempolicy.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0a3690ecab7d..253ce368cf16 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -414,7 +414,7 @@ static const struct mempolicy_operations mpol_ops[MPOL_MAX] = {
>  	},
>  };
>  
> -static int migrate_page_add(struct page *page, struct list_head *pagelist,
> +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
>  				unsigned long flags);
>  
>  struct queue_pages {
> @@ -476,7 +476,7 @@ static int queue_folios_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
>  	/* go to folio migration */
>  	if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
>  		if (!vma_migratable(walk->vma) ||
> -		    migrate_page_add(&folio->page, qp->pagelist, flags)) {
> +		    migrate_folio_add(folio, qp->pagelist, flags)) {
>  			ret = 1;
>  			goto unlock;
>  		}
> @@ -544,7 +544,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>  			 * temporary off LRU pages in the range.  Still
>  			 * need migrate other LRU pages.
>  			 */
> -			if (migrate_page_add(&folio->page, qp->pagelist, flags))
> +			if (migrate_folio_add(folio, qp->pagelist, flags))
>  				has_unmovable = true;
>  		} else
>  			break;
> @@ -1022,27 +1022,23 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
>  }
>  
>  #ifdef CONFIG_MIGRATION
> -/*
> - * page migration, thp tail pages can be passed.
> - */
> -static int migrate_page_add(struct page *page, struct list_head *pagelist,
> +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
>  				unsigned long flags)
>  {
> -	struct page *head = compound_head(page);
>  	/*
> -	 * Avoid migrating a page that is shared with others.
> +	 * Avoid migrating a folio that is shared with others.
>  	 */
> -	if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
> -		if (!isolate_lru_page(head)) {
> -			list_add_tail(&head->lru, pagelist);
> -			mod_node_page_state(page_pgdat(head),
> -				NR_ISOLATED_ANON + page_is_file_lru(head),
> -				thp_nr_pages(head));
> +	if ((flags & MPOL_MF_MOVE_ALL) || folio_mapcount(folio) == 1) {
One question to the page_mapcount -> folio_mapcount here.

For a large folio with 0 entire mapcount, if the first sub-page and any
other sub-page are mapped, page_mapcount(head) == 1 is true while
folio_mapcount(folio) == 1 is not.

Regards
Yin, Fengwei

> +		if (!folio_isolate_lru(folio)) {
> +			list_add_tail(&folio->lru, foliolist);
> +			node_stat_mod_folio(folio,
> +				NR_ISOLATED_ANON + folio_is_file_lru(folio),
> +				folio_nr_pages(folio));
>  		} else if (flags & MPOL_MF_STRICT) {
>  			/*
> -			 * Non-movable page may reach here.  And, there may be
> -			 * temporary off LRU pages or non-LRU movable pages.
> -			 * Treat them as unmovable pages since they can't be
> +			 * Non-movable folio may reach here.  And, there may be
> +			 * temporary off LRU folios or non-LRU movable folios.
> +			 * Treat them as unmovable folios since they can't be
>  			 * isolated, so they can't be moved at the moment.  It
>  			 * should return -EIO for this case too.
>  			 */
> @@ -1234,7 +1230,7 @@ static struct page *new_page(struct page *page, unsigned long start)
>  }
>  #else
>  
> -static int migrate_page_add(struct page *page, struct list_head *pagelist,
> +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
>  				unsigned long flags)
>  {
>  	return -EIO;

Re: [PATCH mm-unstable 5/5] mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

[Vishal Moola]

On Wed, Jan 18, 2023 at 5:24 PM Yin, Fengwei <fengwei.yin@intel.com> wrote:
>
>
>
> On 1/19/2023 7:22 AM, Vishal Moola (Oracle) wrote:
> > Replace migrate_page_add() with migrate_folio_add().
> > migrate_folio_add() does the same a migrate_page_add() but takes in a
> > folio instead of a page. This removes a couple of calls to
> > compound_head().
> >
> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > ---
> >  mm/mempolicy.c | 34 +++++++++++++++-------------------
> >  1 file changed, 15 insertions(+), 19 deletions(-)
> >
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 0a3690ecab7d..253ce368cf16 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -414,7 +414,7 @@ static const struct mempolicy_operations mpol_ops[MPOL_MAX] = {
> >       },
> >  };
> >
> > -static int migrate_page_add(struct page *page, struct list_head *pagelist,
> > +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
> >                               unsigned long flags);
> >
> >  struct queue_pages {
> > @@ -476,7 +476,7 @@ static int queue_folios_pmd(pmd_t *pmd, spinlock_t *ptl, unsigned long addr,
> >       /* go to folio migration */
> >       if (flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) {
> >               if (!vma_migratable(walk->vma) ||
> > -                 migrate_page_add(&folio->page, qp->pagelist, flags)) {
> > +                 migrate_folio_add(folio, qp->pagelist, flags)) {
> >                       ret = 1;
> >                       goto unlock;
> >               }
> > @@ -544,7 +544,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
> >                        * temporary off LRU pages in the range.  Still
> >                        * need migrate other LRU pages.
> >                        */
> > -                     if (migrate_page_add(&folio->page, qp->pagelist, flags))
> > +                     if (migrate_folio_add(folio, qp->pagelist, flags))
> >                               has_unmovable = true;
> >               } else
> >                       break;
> > @@ -1022,27 +1022,23 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
> >  }
> >
> >  #ifdef CONFIG_MIGRATION
> > -/*
> > - * page migration, thp tail pages can be passed.
> > - */
> > -static int migrate_page_add(struct page *page, struct list_head *pagelist,
> > +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
> >                               unsigned long flags)
> >  {
> > -     struct page *head = compound_head(page);
> >       /*
> > -      * Avoid migrating a page that is shared with others.
> > +      * Avoid migrating a folio that is shared with others.
> >        */
> > -     if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
> > -             if (!isolate_lru_page(head)) {
> > -                     list_add_tail(&head->lru, pagelist);
> > -                     mod_node_page_state(page_pgdat(head),
> > -                             NR_ISOLATED_ANON + page_is_file_lru(head),
> > -                             thp_nr_pages(head));
> > +     if ((flags & MPOL_MF_MOVE_ALL) || folio_mapcount(folio) == 1) {
> One question to the page_mapcount -> folio_mapcount here.
>
> For a large folio with 0 entire mapcount, if the first sub-page and any
> other sub-page are mapped, page_mapcount(head) == 1 is true while
> folio_mapcount(folio) == 1 is not.

Hmm, you're right. Using page_mapcount(&folio->page) would definitely
maintain the same behavior, but I'm not sure that's what we actually want.

My understanding of the purpose of this check is to avoid migrating
pages shared with other processes. Meaning if a folio (or any pages
within) are mapped to different processes we would want to skip that
folio.

Although looking at it now, I don't think using folio_mapcount()
accomplishes this either in the case that multiple pages in a large
folio are mapped to the same process.

Does anyone have any better ideas for this?

Re: [PATCH mm-unstable 5/5] mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

[Matthew Wilcox]

On Thu, Jan 19, 2023 at 09:24:16AM +0800, Yin, Fengwei wrote:
> On 1/19/2023 7:22 AM, Vishal Moola (Oracle) wrote:
> > @@ -1022,27 +1022,23 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
> >  }
> >  
> >  #ifdef CONFIG_MIGRATION
> > -/*
> > - * page migration, thp tail pages can be passed.
> > - */
> > -static int migrate_page_add(struct page *page, struct list_head *pagelist,
> > +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
> >  				unsigned long flags)
> >  {
> > -	struct page *head = compound_head(page);
> >  	/*
> > -	 * Avoid migrating a page that is shared with others.
> > +	 * Avoid migrating a folio that is shared with others.
> >  	 */
> > -	if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
> > -		if (!isolate_lru_page(head)) {
> > -			list_add_tail(&head->lru, pagelist);
> > -			mod_node_page_state(page_pgdat(head),
> > -				NR_ISOLATED_ANON + page_is_file_lru(head),
> > -				thp_nr_pages(head));
> > +	if ((flags & MPOL_MF_MOVE_ALL) || folio_mapcount(folio) == 1) {
> One question to the page_mapcount -> folio_mapcount here.
> 
> For a large folio with 0 entire mapcount, if the first sub-page and any
> other sub-page are mapped, page_mapcount(head) == 1 is true while
> folio_mapcount(folio) == 1 is not.

We had a good discussion about this in today's THP Cabal meeting [1].  I
didn't quite check everything that I said was true, so let me summarise
& correct it now ...

 - This is a heuristic.  We're trying to see whether this folio is
   mapped by multiple processes (because if it is, it's probably not
   worth migrating).  If the heuristic is wrong, it probably doesn't
   matter _too_ much?
 - A proper heuristic for this would be
		folio_total_mapcount(folio) == folio_nr_pages(folio)
   but this would be expensive to calculate as it requires examining
   512 cachelines for a 2MB page.
 - For a large folio which is smaller than PMD size, we're guaranteed
   that folio_mapcount() is 0 today.
 - In the meeting I said that page_mapcount() of the head of a THP
   page was zero; that's not true; I had forgotten that we added in
   entire_mapcount to the individual page mapcount.

so I now think this should be:

	page_mapcount(folio_page(folio, 0))

with an explanation that checking every page is too heavy-weight.
Maybe it should be its own function:

static inline int folio_estimated_mapcount(folio)
{
	return page_mapcount(folio_page(folio, 0));
}

with a nice comment explaining what's going on.

[1] https://www.youtube.com/watch?v=A3PoGQQQD3Q is the recording of
today's meeting.

Re: [PATCH mm-unstable 5/5] mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

[Yin, Fengwei]

On 1/21/2023 3:41 AM, Vishal Moola wrote:
> My understanding of the purpose of this check is to avoid migrating
> pages shared with other processes. Meaning if a folio (or any pages
> within) are mapped to different processes we would want to skip that
> folio.

This is my understanding also. But check whether a large file folio is
mapped to different process is not very direct. Thanks.

Regards
Yin, Fengwei

Re: [PATCH mm-unstable 5/5] mm/mempolicy: Convert migrate_page_add() to migrate_folio_add()

[Yin, Fengwei]

On 1/21/2023 3:47 AM, Matthew Wilcox wrote:
> On Thu, Jan 19, 2023 at 09:24:16AM +0800, Yin, Fengwei wrote:
>> On 1/19/2023 7:22 AM, Vishal Moola (Oracle) wrote:
>>> @@ -1022,27 +1022,23 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
>>>  }
>>>  
>>>  #ifdef CONFIG_MIGRATION
>>> -/*
>>> - * page migration, thp tail pages can be passed.
>>> - */
>>> -static int migrate_page_add(struct page *page, struct list_head *pagelist,
>>> +static int migrate_folio_add(struct folio *folio, struct list_head *foliolist,
>>>  				unsigned long flags)
>>>  {
>>> -	struct page *head = compound_head(page);
>>>  	/*
>>> -	 * Avoid migrating a page that is shared with others.
>>> +	 * Avoid migrating a folio that is shared with others.
>>>  	 */
>>> -	if ((flags & MPOL_MF_MOVE_ALL) || page_mapcount(head) == 1) {
>>> -		if (!isolate_lru_page(head)) {
>>> -			list_add_tail(&head->lru, pagelist);
>>> -			mod_node_page_state(page_pgdat(head),
>>> -				NR_ISOLATED_ANON + page_is_file_lru(head),
>>> -				thp_nr_pages(head));
>>> +	if ((flags & MPOL_MF_MOVE_ALL) || folio_mapcount(folio) == 1) {
>> One question to the page_mapcount -> folio_mapcount here.
>>
>> For a large folio with 0 entire mapcount, if the first sub-page and any
>> other sub-page are mapped, page_mapcount(head) == 1 is true while
>> folio_mapcount(folio) == 1 is not.
> 
> We had a good discussion about this in today's THP Cabal meeting [1].  I
> didn't quite check everything that I said was true, so let me summarise
> & correct it now ...
> 
>  - This is a heuristic.  We're trying to see whether this folio is
>    mapped by multiple processes (because if it is, it's probably not
>    worth migrating).  If the heuristic is wrong, it probably doesn't
>    matter _too_ much?
Agree.

>  - A proper heuristic for this would be
> 		folio_total_mapcount(folio) == folio_nr_pages(folio)
I am not sure. File folio can be partially mapped. Maybe following check?
  for each sub-pages:
     (folio_entire_mapcount(folio) + sub-pages->_mapcount) <= 1

But it's also expensive to check all sub-pages. Maybe a bit in folio
if filio mapped to only one process is really important?

>    but this would be expensive to calculate as it requires examining
>    512 cachelines for a 2MB page.
>  - For a large folio which is smaller than PMD size, we're guaranteed
>    that folio_mapcount() is 0 today.
My understanding is: for large folio, if any sub-page is mapped,
folio_mapcount() can not be 0.

>  - In the meeting I said that page_mapcount() of the head of a THP
>    page was zero; that's not true; I had forgotten that we added in
>    entire_mapcount to the individual page mapcount.
> 
> so I now think this should be:
> 
> 	page_mapcount(folio_page(folio, 0))
For file large folio, it's possible folio_page(folio, 0) mapped only
once, other sub-pages mapped multiple times.

But I think this maybe the best choice here.

> 
> with an explanation that checking every page is too heavy-weight.
> Maybe it should be its own function:
> 
> static inline int folio_estimated_mapcount(folio)
> {
> 	return page_mapcount(folio_page(folio, 0));
> }
> 
> with a nice comment explaining what's going on.
> 
> [1] https://www.youtube.com/watch?v=A3PoGQQQD3Q is the recording of
> today's meeting.
This is nice. Thanks a lot for sharing.


Regards
Yin, Fengwei