support batched checking of the young flag for MGLRU

People have already complained that these *_clear_young_notify() related
macros are very ugly, so let's use inline helpers to make them more readable.

In addition, I cannot implement these inline helper functions in the mmu_notifier.h file, because some arch-specific files will include the
mmu_notifier.h, which introduces header compilation dependencies and causes
build errors (e.g., arch/arm64/include/asm/tlbflush.h). Moreover, since
these functions are only used in the mm, implementing these inline helpers
in the mm/internal.h header seems reasonable.

Signed-off-by Baolin Wang <[email protected]>

mm/rmap.c +0 -0

index bff8f222004e..be785dfc9336 100644
@@ -1065,6 +1065,7 @@ int folio_referenced(struct folio *folio, int
 is_locked,
  		.invalid_vma = invalid_folio_referenced_vma,
  	};

mm/rmap.c +1 -0

+	VM_WARN_ON_FOLIO(folio_is_zone_device(folio), folio);

  *vm_flags = 0;
  if (!pra.mapcount)
  return 0;

Quoted 10 lines · Baolin Wang

mm/rmap.c +0 -0

index bff8f222004e..be785dfc9336 100644
@@ -1065,6 +1065,7 @@ int folio_referenced(struct folio *folio, int
 is_locked,
  		.invalid_vma = invalid_folio_referenced_vma,
  	};
 

Quoted 4 lines

mm/rmap.c +1 -0

+	VM_WARN_ON_FOLIO(folio_is_zone_device(folio), folio);
  	*vm_flags = 0;
  	if (!pra.mapcount)

  return 0;

Should be a VM_WARN_ON_ONCE_FOLIO so we do not cause a softlockup if we try to print information about a million ZONE_DEVICE pages?

The folio_referenced() is used to test whether a folio was referenced during reclaim. Moreover, ZONE_DEVICE folios are controlled by their device driver, have a lifetime tied to that driver, and are never placed on the LRU list. That means we should never try to reclaim ZONE_DEVICE folios, so add a warning to catch this unexpected behavior in folio_referenced() to avoid confusion, as discussed in the previous thread[1].

[1] https://lore.kernel.org/all/[email protected]/

Signed-off-by Baolin Wang <[email protected]>

---
 mm/rmap.c | 1 +
 1 file changed, 1 insertion(+)

mm/rmap.c +1 -0

index bff8f222004e..be785dfc9336 100644
@@ -1065,6 +1065,7 @@ int folio_referenced(struct folio *folio, int is_locked,
 		.invalid_vma = invalid_folio_referenced_vma,
 	};
 
+	VM_WARN_ON_FOLIO(folio_is_zone_device(folio), folio);
 	*vm_flags = 0;
 	if (!pra.mapcount)
 		return 0;

Signature · 1 line

2.47.3

People have already complained that these *_clear_young_notify() related macros are very ugly, so let's use inline helpers to make them more readable.

In addition, I cannot implement these inline helper functions in the mmu_notifier.h file, because some arch-specific files will include the mmu_notifier.h, which introduces header compilation dependencies and causes build errors (e.g., arch/arm64/include/asm/tlbflush.h). Moreover, since these functions are only used in the mm, implementing these inline helpers in the mm/internal.h header seems reasonable.

Signed-off-by Baolin Wang <[email protected]>

+static inline int test_and_clear_young_ptes(struct vm_area_struct *vma,

(quoted diff) +1 -0

+					    unsigned long addr,

pte_t *ptep,

include/linux/pgtable.h +1 -0

+					    unsigned int nr)

Use the batched helper clear_young_ptes_notify() to check and clear the
young flag to improve the performance during large folio reclamation when
MGLRU is enabled.

Meanwhile, we can also support batched checking the young and dirty flag
when MGLRU walks the mm's pagetable to update the folios' generation counter. Since MGLRU also checks the PTE dirty bit, use folio_pte_batch_flags()
with FPB_MERGE_YOUNG_DIRTY set to detect batches of PTEs for a large folio.

Then we can remove the ptep_clear_young_notify() since it has no users now.

Signed-off-by Baolin Wang <[email protected]>

Implement the Arm64 architecture-specific test_and_clear_young_ptes() to enable
batched checking of young flags, improving performance during large folio
reclamation when MGLRU is enabled.

W/o patchset:
real 0m0.470s
user 0m0.000s
sys 0m0.470s

W/ patchset:
real 0m0.180s
user 0m0.001s
sys 0m0.179s

Signed-off-by Baolin Wang <[email protected]>

Quoted 6 lines · Baolin Wang

+static inline int test_and_clear_young_ptes(struct vm_area_struct *vma,

(quoted diff) +1 -0

+					    unsigned long addr,

pte_t *ptep,

include/linux/pgtable.h +1 -0

+					    unsigned int nr)

include/linux/pgtable.h static inline int clear_flush_young_ptes(struct vm_area_struct *vma, · line 1136

Quoted 10 lines

include/linux/pgtable.h +10 -0

+{
+	int young = 0;
+
+	for (;;) {
+		young |= ptep_test_and_clear_young(vma, addr, ptep);
+		if (--nr == 0)
+			break;
+		ptep++;
+		addr += PAGE_SIZE;
+	}

This may be a nitpick, but could the --nr thing be stuck into the loop conditional?

Something that looks like an infinite loop just
seems wrong for something so bounded.

People have already complained that these *_clear_young_notify() related macros are very ugly, so let's use inline helpers to make them more readable.

In addition, I cannot implement these inline helper functions in the

mmu_notifier.h file, because some arch-specific files will include the mmu_notifier.h, which introduces header compilation dependencies and causes build errors (e.g., arch/arm64/include/asm/tlbflush.h). Moreover, since these functions are only used in the mm, implementing these inline helpers in the mm/internal.h header seems reasonable.

Signed-off-by Baolin Wang <[email protected]>

---
 include/linux/mmu_notifier.h | 54 ------------------------------------
 mm/internal.h                | 53 +++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 54 deletions(-)

include/linux/mmu_notifier.h +0 -54

index 07a2bbaf86e9..93894b90c8c1 100644
@@ -515,55 +515,6 @@ static inline void mmu_notifier_range_init_owner(
 	range->owner = owner;
 }
 
-#define clear_flush_young_ptes_notify(__vma, __address, __ptep, __nr)	\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	unsigned int ___nr = __nr;					\
-	__young = clear_flush_young_ptes(___vma, ___address, __ptep, ___nr);	\
-	__young |= mmu_notifier_clear_flush_young(___vma->vm_mm,	\
-						  ___address,		\
-						  ___address +		\
-						  ___nr * PAGE_SIZE);	\
-	__young;							\
-})
-
-#define pmdp_clear_flush_young_notify(__vma, __address, __pmdp)		\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	__young = pmdp_clear_flush_young(___vma, ___address, __pmdp);	\
-	__young |= mmu_notifier_clear_flush_young(___vma->vm_mm,	\
-						  ___address,		\
-						  ___address +		\
-							PMD_SIZE);	\
-	__young;							\
-})
-
-#define ptep_clear_young_notify(__vma, __address, __ptep)		\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	__young = ptep_test_and_clear_young(___vma, ___address, __ptep);\
-	__young |= mmu_notifier_clear_young(___vma->vm_mm, ___address,	\
-					    ___address + PAGE_SIZE);	\
-	__young;							\
-})
-
-#define pmdp_clear_young_notify(__vma, __address, __pmdp)		\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	__young = pmdp_test_and_clear_young(___vma, ___address, __pmdp);\
-	__young |= mmu_notifier_clear_young(___vma->vm_mm, ___address,	\
-					    ___address + PMD_SIZE);	\
-	__young;							\
-})
-
 #else /* CONFIG_MMU_NOTIFIER */
 
 struct mmu_notifier_range {

@@ -651,11 +602,6 @@ static inline void mmu_notifier_subscriptions_destroy(struct mm_struct *mm)
 
 #define mmu_notifier_range_update_to_read_only(r) false
 
-#define clear_flush_young_ptes_notify clear_flush_young_ptes
-#define pmdp_clear_flush_young_notify pmdp_clear_flush_young
-#define ptep_clear_young_notify ptep_test_and_clear_young
-#define pmdp_clear_young_notify pmdp_test_and_clear_young
-
 static inline void mmu_notifier_synchronize(void)
 {
 }

mm/internal.h +1 -0

index e0ef192b0be3..1ba175b8d4f1 100644
@@ -11,6 +11,7 @@
 #include <linux/khugepaged.h>
 #include <linux/mm.h>
 #include <linux/mm_inline.h>
+#include <linux/mmu_notifier.h>
 #include <linux/pagemap.h>
 #include <linux/pagewalk.h>
 #include <linux/rmap.h>

mm/internal.h +28 -0

+{
+	int young;
+
+	young = clear_flush_young_ptes(vma, addr, ptep, nr);
+	young |= mmu_notifier_clear_flush_young(vma->vm_mm, addr,
+						addr + nr * PAGE_SIZE);
+	return young;
+}
+
+static inline int pmdp_clear_flush_young_notify(struct vm_area_struct *vma,
+						unsigned long addr, pmd_t *pmdp)
+{
+	int young;
+
+	young = pmdp_clear_flush_young(vma, addr, pmdp);
+	young |= mmu_notifier_clear_flush_young(vma->vm_mm, addr, addr + PMD_SIZE);
+	return young;
+}
+
+static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep)
+{
+	int young;
+
+	young = ptep_test_and_clear_young(vma, addr, ptep);
+	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE);
+	return young;
+}

Quoted 18 lines · Rik van Riel

Quoted 12 lines · Baolin Wang

mm/rmap.c +0 -0

index bff8f222004e..be785dfc9336 100644
@@ -1065,6 +1065,7 @@ int folio_referenced(struct folio *folio, int
 is_locked,
           .invalid_vma = invalid_folio_referenced_vma,
       };
   +    VM_WARN_ON_FOLIO(folio_is_zone_device(folio), folio);
       *vm_flags = 0;
       if (!pra.mapcount)

Quoted 1 line

          return 0;

Should be a VM_WARN_ON_ONCE_FOLIO so we do not cause a softlockup if we try to print information about a million ZONE_DEVICE pages?

Yes, sounds reasonable. Will do in next version. Thanks for reviewing.

Currently, MGLRU will call ptep_clear_young_notify() to check and clear the young flag for each PTE sequentially, which is inefficient for large folios reclamation.

Moreover, on Arm64 architecture, which supports contiguous PTEs, the Arm64- specific ptep_test_and_clear_young() already implements an optimization to clear the young flags for PTEs within a contiguous range. However, this is not sufficient. Similar to the Arm64 specific clear_flush_young_ptes(), we can extend this to perform batched operations for the entire large folio (which might exceed the contiguous range: CONT_PTE_SIZE).

Thus, we can introduce a new batched helper: test_and_clear_young_ptes() and its wrapper clear_young_ptes_notify(), to perform batched checking of the young flags for large folios, which can help improve performance during large folio reclamation when MGLRU is enabled. And it will be overridden by the architecture that implements a more efficient batch operation in the following patches.

Signed-off-by Baolin Wang <[email protected]>

---
 include/linux/pgtable.h | 36 ++++++++++++++++++++++++++++++++++++
 mm/internal.h           | 23 ++++++++++++++++++-----
 2 files changed, 54 insertions(+), 5 deletions(-)

include/linux/pgtable.h +4 -0

index 776993d4567b..0bcd3be524d3 100644
@@ -1103,6 +1103,42 @@ static inline int clear_flush_young_ptes(struct vm_area_struct *vma,
 }
 #endif
 
+#ifndef test_and_clear_young_ptes
+/**
+ * test_and_clear_young_ptes - Mark PTEs that map consecutive pages of the same
+ *			       folio as old

include/linux/pgtable.h +3 -0

+}
+#endif
+
 /*
  * On some architectures hardware does not set page access bit when accessing
  * memory page, it is responsibility of software setting this bit. It brings

mm/internal.h +11 -4

index 1ba175b8d4f1..1b59be99dc3f 100644
@@ -1813,16 +1813,23 @@ static inline int pmdp_clear_flush_young_notify(struct vm_area_struct *vma,
 	return young;
 }
 
-static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
-					  unsigned long addr, pte_t *ptep)
+static inline int clear_young_ptes_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep,
+					  unsigned int nr)
 {
 	int young;
 
-	young = ptep_test_and_clear_young(vma, addr, ptep);
-	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE);
+	young = test_and_clear_young_ptes(vma, addr, ptep, nr);
+	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + nr * PAGE_SIZE);
 	return young;
 }
 
+static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep)
+{
+	return clear_young_ptes_notify(vma, addr, ptep, 1);
+}
+
 static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
 					  unsigned long addr, pmd_t *pmdp)
 {

@@ -1837,9 +1844,15 @@ static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
 
 #define clear_flush_young_ptes_notify	clear_flush_young_ptes

Use the batched helper clear_young_ptes_notify() to check and clear the young flag to improve the performance during large folio reclamation when MGLRU is enabled.

Meanwhile, we can also support batched checking the young and dirty flag when MGLRU walks the mm's pagetable to update the folios' generation counter. Since MGLRU also checks the PTE dirty bit, use folio_pte_batch_flags() with FPB_MERGE_YOUNG_DIRTY set to detect batches of PTEs for a large folio.

Then we can remove the ptep_clear_young_notify() since it has no users now.

Signed-off-by Baolin Wang <[email protected]>

---

mm/internal.h +0 -6

-static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
-					  unsigned long addr, pte_t *ptep)
-{
-	return clear_young_ptes_notify(vma, addr, ptep, 1);
-}
-
 static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
 					  unsigned long addr, pmd_t *pmdp)
 {

mm/internal.h +0 -0

 #endif /* CONFIG_MMU_NOTIFIER */
 
 #endif	/* __MM_INTERNAL_H */

mm/rmap.c +0 -0

index be785dfc9336..1c147251ae28 100644

mm/rmap.c +6 -14

+
+			nr = folio_pte_batch(folio, pvmw.pte, pteval, max_nr);
+			ptes += nr;
+		}
+
 		if (lru_gen_enabled() && pvmw.pte) {
-			if (lru_gen_look_around(&pvmw))
+			if (lru_gen_look_around(&pvmw, nr))
 				referenced++;
 		} else if (pvmw.pte) {
-			if (folio_test_large(folio)) {
-				unsigned long end_addr = pmd_addr_end(address, vma->vm_end);
-				unsigned int max_nr = (end_addr - address) >> PAGE_SHIFT;
-				pte_t pteval = ptep_get(pvmw.pte);
-
-				nr = folio_pte_batch(folio, pvmw.pte,
-						     pteval, max_nr);
-			}
-
-			ptes += nr;
 			if (clear_flush_young_ptes_notify(vma, address, pvmw.pte, nr))
 				referenced++;
-			/* Skip the batched PTEs */
-			pvmw.pte += nr - 1;
-			pvmw.address += (nr - 1) * PAGE_SIZE;
 		} else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
 			if (pmdp_clear_flush_young_notify(vma, address,
 						pvmw.pmd))

mm/rmap.c +0 -0

 	}
 
 	if (referenced)

mm/vmscan.c +1 -0

index 728868c61750..d83962468b2e 100644
@@ -3494,6 +3494,7 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
 	struct pglist_data *pgdat = lruvec_pgdat(walk->lruvec);
 	DEFINE_MAX_SEQ(walk->lruvec);
 	int gen = lru_gen_from_seq(max_seq);
+	unsigned int nr;
 	pmd_t pmdval;
 
 	pte = pte_offset_map_rw_nolock(args->mm, pmd, start & PMD_MASK, &pmdval, &ptl);

mm/vmscan.c +9 -1

+		nr = 1;
 		total++;
 		walk->mm_stats[MM_LEAF_TOTAL]++;
 

@@ -3528,7 +3531,14 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
 		if (!folio)
 			continue;
 
-		if (!ptep_clear_young_notify(args->vma, addr, pte + i))
+		if (folio_test_large(folio)) {
+			unsigned int max_nr = (end - addr) >> PAGE_SHIFT;
+
+			nr = folio_pte_batch_flags(folio, NULL, ptep, &ptent,
+						   max_nr, FPB_MERGE_YOUNG_DIRTY);
+		}
+
+		if (!clear_young_ptes_notify(args->vma, addr, ptep, nr))
 			continue;
 
 		if (last != folio) {

Quoted 2 lines · Baolin Wang

People have already complained that these *_clear_young_notify() related macros are very ugly, so let's use inline helpers to make them more readable.

Could it have been me :)

Quoted 1 line

In addition, I cannot implement these inline helper functions in the

"We cannot implement ..."

Quoted 5 lines

mmu_notifier.h file, because some arch-specific files will include the mmu_notifier.h, which introduces header compilation dependencies and causes build errors (e.g., arch/arm64/include/asm/tlbflush.h). Moreover, since these functions are only used in the mm, implementing these inline helpers in the mm/internal.h header seems reasonable.

Agreed. if it compiles, all good.

Quoted 95 lines

Signed-off-by Baolin Wang <[email protected]>

---
  include/linux/mmu_notifier.h | 54 ------------------------------------
  mm/internal.h                | 53 +++++++++++++++++++++++++++++++++++
  2 files changed, 53 insertions(+), 54 deletions(-)

include/linux/mmu_notifier.h +0 -54

index 07a2bbaf86e9..93894b90c8c1 100644
@@ -515,55 +515,6 @@ static inline void mmu_notifier_range_init_owner(
  	range->owner = owner;
  }
 
-#define clear_flush_young_ptes_notify(__vma, __address, __ptep, __nr)	\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	unsigned int ___nr = __nr;					\
-	__young = clear_flush_young_ptes(___vma, ___address, __ptep, ___nr);	\
-	__young |= mmu_notifier_clear_flush_young(___vma->vm_mm,	\
-						  ___address,		\
-						  ___address +		\
-						  ___nr * PAGE_SIZE);	\
-	__young;							\
-})
-
-#define pmdp_clear_flush_young_notify(__vma, __address, __pmdp)		\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	__young = pmdp_clear_flush_young(___vma, ___address, __pmdp);	\
-	__young |= mmu_notifier_clear_flush_young(___vma->vm_mm,	\
-						  ___address,		\
-						  ___address +		\
-							PMD_SIZE);	\
-	__young;							\
-})
-
-#define ptep_clear_young_notify(__vma, __address, __ptep)		\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	__young = ptep_test_and_clear_young(___vma, ___address, __ptep);\
-	__young |= mmu_notifier_clear_young(___vma->vm_mm, ___address,	\
-					    ___address + PAGE_SIZE);	\
-	__young;							\
-})
-
-#define pmdp_clear_young_notify(__vma, __address, __pmdp)		\
-({									\
-	int __young;							\
-	struct vm_area_struct *___vma = __vma;				\
-	unsigned long ___address = __address;				\
-	__young = pmdp_test_and_clear_young(___vma, ___address, __pmdp);\
-	__young |= mmu_notifier_clear_young(___vma->vm_mm, ___address,	\
-					    ___address + PMD_SIZE);	\
-	__young;							\
-})
-
  #else /* CONFIG_MMU_NOTIFIER */
 
  struct mmu_notifier_range {

@@ -651,11 +602,6 @@ static inline void mmu_notifier_subscriptions_destroy(struct mm_struct *mm)
 
  #define mmu_notifier_range_update_to_read_only(r) false
 
-#define clear_flush_young_ptes_notify clear_flush_young_ptes
-#define pmdp_clear_flush_young_notify pmdp_clear_flush_young
-#define ptep_clear_young_notify ptep_test_and_clear_young
-#define pmdp_clear_young_notify pmdp_test_and_clear_young
-
  static inline void mmu_notifier_synchronize(void)
  {
  }

mm/internal.h +1 -0

index e0ef192b0be3..1ba175b8d4f1 100644
@@ -11,6 +11,7 @@
  #include <linux/khugepaged.h>
  #include <linux/mm.h>
  #include <linux/mm_inline.h>
+#include <linux/mmu_notifier.h>
  #include <linux/pagemap.h>
  #include <linux/pagewalk.h>
  #include <linux/rmap.h>

@@ -1789,4 +1790,56 @@ static inline int io_remap_pfn_range_complete(struct vm_area_struct *vma,
  	return remap_pfn_range_complete(vma, addr, pfn, size, prot);
  }
 

mm/internal.h static inline int io_remap_pfn_range_complete(struct vm_area_struct *vma, · line 1796

Quoted 4 lines

mm/internal.h +4 -0

+#ifdef CONFIG_MMU_NOTIFIER
+static inline int clear_flush_young_ptes_notify(struct vm_area_struct *vma,
+						unsigned long addr, pte_t *ptep,
+						unsigned int nr)

Two tabs indent on the second line makes this fit into two lines.

Same for the others.

With that

Acked-by David Hildenbrand (Arm) <[email protected]>

Quoted 33 lines

mm/internal.h +33 -0

+{
+	int young;
+
+	young = clear_flush_young_ptes(vma, addr, ptep, nr);
+	young |= mmu_notifier_clear_flush_young(vma->vm_mm, addr,
+						addr + nr * PAGE_SIZE);
+	return young;
+}
+
+static inline int pmdp_clear_flush_young_notify(struct vm_area_struct *vma,
+						unsigned long addr, pmd_t *pmdp)
+{
+	int young;
+
+	young = pmdp_clear_flush_young(vma, addr, pmdp);
+	young |= mmu_notifier_clear_flush_young(vma->vm_mm, addr, addr + PMD_SIZE);
+	return young;
+}
+
+static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep)
+{
+	int young;
+
+	young = ptep_test_and_clear_young(vma, addr, ptep);
+	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE);
+	return young;
+}
+
+static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pmd_t *pmdp)
+{
+	int young;

mm/internal.h static inline int io_remap_pfn_range_complete(struct vm_area_struct *vma, · line 1834

Quoted 5 lines

mm/internal.h +5 -0

+
+	young = pmdp_test_and_clear_young(vma, addr, pmdp);
+	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PMD_SIZE);
+	return young;
+}

Wonder why someone thought it would be a good idea to not include the "test_and_", to make the function names inconsistent.

Quoted 11 lines · Baolin Wang

Currently, MGLRU will call ptep_clear_young_notify() to check and clear the young flag for each PTE sequentially, which is inefficient for large folios reclamation.

Moreover, on Arm64 architecture, which supports contiguous PTEs, the Arm64- specific ptep_test_and_clear_young() already implements an optimization to clear the young flags for PTEs within a contiguous range. However, this is not sufficient. Similar to the Arm64 specific clear_flush_young_ptes(), we can extend this to perform batched operations for the entire large folio (which might exceed the contiguous range: CONT_PTE_SIZE).

Quoted 6 lines

Thus, we can introduce a new batched helper: test_and_clear_young_ptes() and its wrapper clear_young_ptes_notify(), to perform batched checking of the young flags for large folios, which can help improve performance during large folio reclamation when MGLRU is enabled. And it will be overridden by the architecture that implements a more efficient batch operation in the following patches.

Maybe mention that the implementation follows the other existing functions.

Quoted 18 lines

Signed-off-by Baolin Wang <[email protected]>

---
  include/linux/pgtable.h | 36 ++++++++++++++++++++++++++++++++++++
  mm/internal.h           | 23 ++++++++++++++++++-----
  2 files changed, 54 insertions(+), 5 deletions(-)

include/linux/pgtable.h +4 -0

index 776993d4567b..0bcd3be524d3 100644
@@ -1103,6 +1103,42 @@ static inline int clear_flush_young_ptes(struct vm_area_struct *vma,
  }
  #endif
 
+#ifndef test_and_clear_young_ptes
+/**
+ * test_and_clear_young_ptes - Mark PTEs that map consecutive pages of the same
+ *			       folio as old

include/linux/pgtable.h static inline int clear_flush_young_ptes(struct vm_area_struct *vma, · line 1119

Quoted 10 lines

include/linux/pgtable.h +10 -0

+ * @vma: The virtual memory area the pages are mapped into.
+ * @addr: Address the first page is mapped at.
+ * @ptep: Page table pointer for the first entry.
+ * @nr: Number of entries to clear access bit.
+ *
+ * May be overridden by the architecture; otherwise, implemented as a simple
+ * loop over ptep_test_and_clear_young().
+ *
+ * Note that PTE bits in the PTE range besides the PFN can differ. For example,
+ * some PTEs might be write-protected.

Document the return value?

Returns: whether any PTE was young.

Or sth like that.

include/linux/pgtable.h static inline int clear_flush_young_ptes(struct vm_area_struct *vma, · line 1126

Quoted 7 lines

include/linux/pgtable.h +7 -0

+ *
+ * Context: The caller holds the page table lock.  The PTEs map consecutive
+ * pages that belong to the same folio.  The PTEs are all in the same PMD.
+ */
+static inline int test_and_clear_young_ptes(struct vm_area_struct *vma,
+					    unsigned long addr, pte_t *ptep,
+					    unsigned int nr)

Two tabs ...

include/linux/pgtable.h static inline int clear_flush_young_ptes(struct vm_area_struct *vma, · line 1138

Quoted 13 lines

include/linux/pgtable.h +12 -0

+{
+	int young = 0;
+
+	for (;;) {
+		young |= ptep_test_and_clear_young(vma, addr, ptep);
+		if (--nr == 0)
+			break;
+		ptep++;
+		addr += PAGE_SIZE;
+	}
+
+	return young;

BTW: can this function simply return (and use) a bool instead?

Likely we should do the same for the other functions, but that can be done separately.

Quoted 38 lines

include/linux/pgtable.h +0 -0

  /*
   * On some architectures hardware does not set page access bit when accessing
   * memory page, it is responsibility of software setting this bit. It brings

mm/internal.h +11 -4

index 1ba175b8d4f1..1b59be99dc3f 100644
@@ -1813,16 +1813,23 @@ static inline int pmdp_clear_flush_young_notify(struct vm_area_struct *vma,
  	return young;
  }
 
-static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
-					  unsigned long addr, pte_t *ptep)
+static inline int clear_young_ptes_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep,
+					  unsigned int nr)
  {
  	int young;
 
-	young = ptep_test_and_clear_young(vma, addr, ptep);
-	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE);
+	young = test_and_clear_young_ptes(vma, addr, ptep, nr);
+	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + nr * PAGE_SIZE);
  	return young;
  }
 
+static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep)
+{
+	return clear_young_ptes_notify(vma, addr, ptep, 1);
+}
+
  static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
  					  unsigned long addr, pmd_t *pmdp)
  {

@@ -1837,9 +1844,15 @@ static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
 
  #define clear_flush_young_ptes_notify	clear_flush_young_ptes

mm/internal.h static inline int pmdp_clear_young_notify(struct vm_area_struct *vma, · line 1854

Quoted 10 lines

mm/internal.h +6 -1

  #define pmdp_clear_flush_young_notify	pmdp_clear_flush_young
-#define ptep_clear_young_notify	ptep_test_and_clear_young
+#define clear_young_ptes_notify	test_and_clear_young_ptes
  #define pmdp_clear_young_notify	pmdp_test_and_clear_young
 
+static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
+					  unsigned long addr, pte_t *ptep)
+{
+	return test_and_clear_young_ptes(vma, addr, ptep, 1);
+}

Why not outside of the ifdef a single generic

static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
unsigned long addr, pte_t *ptep)

{
	return clear_young_ptes_notify(vma, addr, ptep, 1);
}

Quoted 13 lines · Baolin Wang

Use the batched helper clear_young_ptes_notify() to check and clear the young flag to improve the performance during large folio reclamation when MGLRU is enabled.

Meanwhile, we can also support batched checking the young and dirty flag when MGLRU walks the mm's pagetable to update the folios' generation counter. Since MGLRU also checks the PTE dirty bit, use folio_pte_batch_flags() with FPB_MERGE_YOUNG_DIRTY set to detect batches of PTEs for a large folio.

Then we can remove the ptep_clear_young_notify() since it has no users now.

Signed-off-by Baolin Wang <[email protected]>

---

[...]

Quoted 11 lines

mm/internal.h +0 -6

-static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
-					  unsigned long addr, pte_t *ptep)
-{
-	return clear_young_ptes_notify(vma, addr, ptep, 1);
-}
-
  static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
  					  unsigned long addr, pmd_t *pmdp)
  {

mm/internal.h static inline int pmdp_clear_young_notify(struct vm_area_struct *vma, · line 1855

Quoted 9 lines

mm/internal.h +0 -6

@@ -1847,12 +1841,6 @@ static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
  #define clear_young_ptes_notify	test_and_clear_young_ptes
  #define pmdp_clear_young_notify	pmdp_test_and_clear_young
 
-static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
-					  unsigned long addr, pte_t *ptep)
-{
-	return test_and_clear_young_ptes(vma, addr, ptep, 1);
-}
-

Oh, we remove the last user, nice.

Quoted 11 lines

mm/internal.h +0 -0

  #endif /* CONFIG_MMU_NOTIFIER */
 
  #endif	/* __MM_INTERNAL_H */

mm/rmap.c +0 -0

index be785dfc9336..1c147251ae28 100644
@@ -958,25 +958,21 @@ static bool folio_referenced_one(struct folio *folio,
  			return false;
  		}
 

mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 963

Quoted 3 lines

mm/rmap.c +3 -0

+		if (pvmw.pte && folio_test_large(folio)) {
+			unsigned long end_addr = pmd_addr_end(address, vma->vm_end);
+			unsigned int max_nr = (end_addr - address) >> PAGE_SHIFT;

Both could be const.

mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 964

Quoted 2 lines

mm/rmap.c +1 -0

+			pte_t pteval = ptep_get(pvmw.pte);

I wonder if there could be a way to avoid this ptep_get() by letting page_vma_mapped_walk() just provide the last value it used (in check_pte() I guess). Something for another patch.

Quoted 29 lines

mm/rmap.c +6 -14

+
+			nr = folio_pte_batch(folio, pvmw.pte, pteval, max_nr);
+			ptes += nr;
+		}
+
  		if (lru_gen_enabled() && pvmw.pte) {
-			if (lru_gen_look_around(&pvmw))
+			if (lru_gen_look_around(&pvmw, nr))
  				referenced++;
  		} else if (pvmw.pte) {
-			if (folio_test_large(folio)) {
-				unsigned long end_addr = pmd_addr_end(address, vma->vm_end);
-				unsigned int max_nr = (end_addr - address) >> PAGE_SHIFT;
-				pte_t pteval = ptep_get(pvmw.pte);
-
-				nr = folio_pte_batch(folio, pvmw.pte,
-						     pteval, max_nr);
-			}
-
-			ptes += nr;
  			if (clear_flush_young_ptes_notify(vma, address, pvmw.pte, nr))
  				referenced++;
-			/* Skip the batched PTEs */
-			pvmw.pte += nr - 1;
-			pvmw.address += (nr - 1) * PAGE_SIZE;
  		} else if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
  			if (pmdp_clear_flush_young_notify(vma, address,
  						pvmw.pmd))

mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 999

Quoted 9 lines

mm/rmap.c +6 -0

@@ -995,6 +991,12 @@ static bool folio_referenced_one(struct folio *folio,
  			page_vma_mapped_walk_done(&pvmw);
  			break;
  		}
+
+		/* Skip the batched PTEs */
+		if (nr > 1) {
+			pvmw.pte += nr - 1;
+			pvmw.address += (nr - 1) * PAGE_SIZE;
+		}

As nr >= 1, you can just unconditionaly do

pvmw.pte += nr - 1;
pvmw.address += (nr - 1) * PAGE_SIZE;

Quoted 16 lines

mm/rmap.c +0 -0

  	}
 
  	if (referenced)

mm/vmscan.c +1 -0

index 728868c61750..d83962468b2e 100644
@@ -3494,6 +3494,7 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
  	struct pglist_data *pgdat = lruvec_pgdat(walk->lruvec);
  	DEFINE_MAX_SEQ(walk->lruvec);
  	int gen = lru_gen_from_seq(max_seq);
+	unsigned int nr;
  	pmd_t pmdval;
 
  	pte = pte_offset_map_rw_nolock(args->mm, pmd, start & PMD_MASK, &pmdval, &ptl);

mm/vmscan.c static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end, · line 3520

Quoted 10 lines

mm/vmscan.c +3 -2

@@ -3512,11 +3513,13 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
 
  	lazy_mmu_mode_enable();
  restart:
-	for (i = pte_index(start), addr = start; addr != end; i++, addr += PAGE_SIZE) {
+	for (i = pte_index(start), addr = start; addr != end; i += nr, addr += nr * PAGE_SIZE) {
  		unsigned long pfn;
  		struct folio *folio;
-		pte_t ptent = ptep_get(pte + i);
+		pte_t *ptep = pte + i;
+		pte_t ptent = ptep_get(ptep);

Existing "pte vs ptent" vs. "ptep vs. pte" is already confusing. Combining them into "pte vs. ptep vs. ptent" is no good.

If you need another variable, call it "cur_pte". Or rename "pte" to "start_pte".

Quoted 19 lines

mm/vmscan.c +9 -1

+		nr = 1;
  		total++;
  		walk->mm_stats[MM_LEAF_TOTAL]++;
 

@@ -3528,7 +3531,14 @@ static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end,
  		if (!folio)
  			continue;
 
-		if (!ptep_clear_young_notify(args->vma, addr, pte + i))
+		if (folio_test_large(folio)) {
+			unsigned int max_nr = (end - addr) >> PAGE_SHIFT;
+
+			nr = folio_pte_batch_flags(folio, NULL, ptep, &ptent,
+						   max_nr, FPB_MERGE_YOUNG_DIRTY);
+		}
+
+		if (!clear_young_ptes_notify(args->vma, addr, ptep, nr))
  			continue;
 

mm/vmscan.c static void lru_gen_age_node(struct pglist_data *pgdat, struct scan_control *sc) · line 4199

Quoted 7 lines

mm/vmscan.c +1 -1

  		if (last != folio) {

@@ -4186,7 +4196,7 @@ static void lru_gen_age_node(struct pglist_data *pgdat, struct scan_control *sc)
   * the PTE table to the Bloom filter. This forms a feedback loop between the
   * eviction and the aging.
   */
-bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw)
+bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw, unsigned int batched)

What is "batched"? Did you mean "nr_ptes" ? Or just the initial value for "nr" ?

mm/vmscan.c bool lru_gen_look_around(struct page_vma_mapped_walk *pvmw) · line 4273

Quoted 3 lines

mm/vmscan.c +2 -1

-		if (!ptep_clear_young_notify(vma, addr, pte + i))
+		if (folio_test_large(folio)) {
+			unsigned int max_nr = (end - addr) >> PAGE_SHIFT;

Can be const.

mm/vmscan.c static bool walk_pte_range(pmd_t *pmd, unsigned long start, unsigned long end, · line 3539

Quoted 5 lines

mm/vmscan.c +4 -0

+
+			nr = folio_pte_batch_flags(folio, NULL, ptep, &ptent,
+						   max_nr, FPB_MERGE_YOUNG_DIRTY);
+		}

I guess we might benefit from a FPB_MERGE_YOUNG only here. But this should work.

Quoted 7 lines

Quoted 3 lines

+            pte_t pteval = ptep_get(pvmw.pte);

I wonder if there could be a way to avoid this ptep_get() by letting page_vma_mapped_walk() just provide the last value it used (in check_pte() I guess). Something for another patch.

Well, we’d need to add a new field to ‘struct page_vma_mapped_walk’ to store the last value (e.g., pvmw.pteval),

but this makes me wonder if it
is worth adding a new field just to avoid a lightweight read (which should have no obvious performance impact).

Quoted 3 lines

What is "batched"? Did you mean "nr_ptes" ? Or just the initial value for "nr" ?

There is already an 'nr' variable in this function. "nr_ptes" sounds good to me, and will use it.

Quoted 10 lines

Quoted 7 lines

Quoted 3 lines

+            pte_t pteval = ptep_get(pvmw.pte);

I wonder if there could be a way to avoid this ptep_get() by letting page_vma_mapped_walk() just provide the last value it used (in check_pte() I guess). Something for another patch.

Well, we’d need to add a new field to ‘struct page_vma_mapped_walk’ to store the last value (e.g., pvmw.pteval),

Yes.

Quoted 3 lines

but this makes me wonder if it
is worth adding a new field just to avoid a lightweight read (which should have no obvious performance impact).

You recall that ptep_get() on arm64 is not that lightweight due to con-pte? :)

But yeah, something for another day.

Quoted 5 lines

Quoted 2 lines

What is "batched"? Did you mean "nr_ptes" ? Or just the initial value for "nr" ?

There is already an 'nr' variable in this function. "nr_ptes" sounds good to me, and will use it.

You can just use "nr" here and reuse it for the existing variable?

Both have the same semantics (nr of ptes / pages), so having a single value might cause less confusion.