Skip to contentSkip to conversation

Optimize anonymous swapbacked large folio unmapping

PATCH Superseded Dev Jain 4 participants 21 messages Sep 24 – Oct 2

Subsystems (7)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Dev Jain

author
#0 lore raw

v3 cover Optimize anonymous swapbacked large folio unmapping

Speed up unmapping of anonymous swapbacked large folios by clearing the ptes, and setting swap ptes, in one go.

The following benchmark (stolen from Barry) is used to measure the time taken to swapout 256M worth of memory backed by 64K large folios:

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <errno.h>

#define SIZE_MB 256
#define SIZE_BYTES (SIZE_MB * 1024 * 1024)

 int main() {
     void *addr = mmap(NULL, SIZE_BYTES, PROT_READ | PROT_WRITE,
                       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
     if (addr == MAP_FAILED) {
         perror("mmap failed");
         return 1;
     }

     memset(addr, 0, SIZE_BYTES);

     struct timespec start, end;
     clock_gettime(CLOCK_MONOTONIC, &start);

     if (madvise(addr, SIZE_BYTES, MADV_PAGEOUT) != 0) {
         perror("madvise(MADV_PAGEOUT) failed");
         munmap(addr, SIZE_BYTES);
         return 1;
     }

     clock_gettime(CLOCK_MONOTONIC, &end);

     long duration_ns = (end.tv_sec - start.tv_sec) * 1e9 +
                        (end.tv_nsec - start.tv_nsec);
     printf("madvise(MADV_PAGEOUT) took %ld ns (%.3f ms)\n",
            duration_ns, duration_ns / 1e6);

     munmap(addr, SIZE_BYTES);
     return 0;

}

Performance as measured on a Linux VM on Apple M3 (arm64):

Vanilla - Mean: 37401913 ns, std dev: 12%
Patched - Mean: 17420282 ns, std dev: 11%

resulting in more than 2x speedup.

No regression observed on 4K folios.

Performance as measured on bare metal x86:

Vanilla - mean: 54986286 ns, std dev: 1.5%
Patched - mean: 51930795 ns, std dev: 3%

I tried magnifying the difference on x86 by using 1M large folios, but can't spot an obvious improvement (looks like my system is too fast to benefit from batched atomic operations!), hinting that the benefit lies mainly in the reduction of ptep_get() calls and the reduction of TLB flushes during contpte-unfolding, on arm64.

No regression is observed on 4K folios on x86 too.

---
Applies on mm-unstable.

mm-selftests pass.

This breakout patchset is the final completion of
https://lore.kernel.org/all/[email protected]/

The extra addition is the generic set_softleaf_ptes() helper.

v2->v3:
- page_anon_exclusive_batch returns unsigned long, shift to rmap.h
- Drop a para in description of patch 5, use softleaf_is_swap to gate pte_swp_exclusive
- finish_folio_unmap -> finish_folio_unmap_batch
- Add patch 9 to do batching for sparc

v1->v2:
- Add WARNs for folio span in folio_*_swap_pages
- Trivial changes to page_anon_exclusive_batch
- Add two sanity checks in patch 4

Dev Jain (9):
mm/swapfile: add batched version of folio_dup_swap mm/swapfile: add batched version of folio_put_swap
mm: move anon-exclusive batch helper to rmap.h mm/rmap: Add batched version of folio_try_share_anon_rmap_pte mm/internal: rename swap offset helpers to softleaf offset mm/internal: add set_softleaf_ptes
mm/memory: use set_softleaf_ptes for uffd-wp markers mm/rmap: batch unmap anonymous swap-backed large folios mm, sparc: batch arch_unmap_one()

 arch/sparc/include/asm/pgtable_64.h |  10 ++-
 arch/sparc/kernel/adi_64.c          |  21 ++++++
 include/linux/pgtable.h             |   6 +-
 include/linux/rmap.h                |  73 +++++++++++++-----
 mm/internal.h                       |  64 ++++++++++++----
 mm/memory.c                         |  20 ++---
 mm/mprotect.c                       |  18 +----
 mm/rmap.c                           | 110 ++++++++++++++++++++--------
 mm/shmem.c                          |   8 +-
 mm/swap.h                           |  35 ++++++++-
 mm/swapfile.c                       |  39 +++++-----
 11 files changed, 275 insertions(+), 129 deletions(-)

base-commit: 8d61431ed2607386b427752505379536eb634ce8

Signature · 1 line
2.43.0
author
#1 lore raw

PATCH v3 1/9 mm/swapfile: add batched version of folio_dup_swap

Add folio_dup_swap_pages to handle a batch of consecutive pages. Note that folio_dup_swap already can handle a subset of this: nr_pages == 1 and nr_pages == folio_nr_pages(folio). Generalize this to any nr_pages.

Currently we have a not-so-nice logic of passing in subpage == NULL if we mean to exercise the logic on the entire folio, and subpage != NULL if we want to exercise the logic on only that subpage. Remove this
indirection: the caller invokes folio_dup_swap_pages() if it wants to operate on a range of pages in the folio (i.e nr_pages may be anything between 1 and folio_nr_pages()), and invokes folio_dup_swap() if it wants to operate on the entire folio.

---
 mm/rmap.c     |  2 +-
 mm/shmem.c    |  2 +-
 mm/swap.h     | 18 ++++++++++++++++--
 mm/swapfile.c | 18 ++++++++----------
 4 files changed, 26 insertions(+), 14 deletions(-)
mm/rmap.c +1 -1
index 6661bc11ce658..a70dbc8d544e1 100644
@@ -2150,7 +2150,7 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
 	swp_entry_t entry = folio_page_swap_entry(folio, page);
 	struct mm_struct *mm = vma->vm_mm;
 
-	if (folio_dup_swap(folio, page) < 0)
+	if (folio_dup_swap_pages(folio, page, 1) < 0)
 		return false;
 
 	/*
mm/shmem.c +1 -1
index b572c60f2af85..d5bd9506ce500 100644
@@ -1914,7 +1914,7 @@ int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
 			spin_unlock(&shmem_swaplist_lock);
 		}
 
-		folio_dup_swap(folio, NULL);
+		folio_dup_swap(folio);
 		shmem_delete_from_page_cache(folio, swp_to_radix_entry(folio->swap));
 
 		BUG_ON(folio_mapped(folio));
mm/swap.h +16 -2
index b3b54c28929a1..47290510de264 100644
@@ -244,7 +244,8 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
  * folio_put_swap(): does the opposite thing of folio_dup_swap().
  */
 int folio_alloc_swap(struct folio *folio);
-int folio_dup_swap(struct folio *folio, struct page *page);
+int folio_dup_swap_pages(struct folio *folio, struct page *page,
+		unsigned long nr_pages);
 void folio_put_swap(struct folio *folio, struct page *page);
 
 /* For internal use */
@@ -368,7 +369,8 @@ static inline int folio_alloc_swap(struct folio *folio)
 	return -EINVAL;
 }
 
-static inline int folio_dup_swap(struct folio *folio, struct page *page)
+static inline int folio_dup_swap_pages(struct folio *folio, struct page *page,
+		unsigned long nr_pages)
 {
 	return -EINVAL;
 }
@@ -464,6 +466,18 @@ static inline void __swap_cache_replace_folio(struct swap_cluster_info *ci,
 }
 #endif /* CONFIG_SWAP */
 
+/**
+ * folio_dup_swap() - Increase swap count of all swap entries of a folio.
+ * @folio: folio with swap entries bound.
+ *
+ * See folio_dup_swap_pages() for more information.
+ */
+static inline int folio_dup_swap(struct folio *folio)
+{
+	return folio_dup_swap_pages(folio, folio_page(folio, 0),
+				    folio_nr_pages(folio));
+}
+
 extern const struct swap_ops swap_bdev_ops;
 
 int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
mm/swapfile.c +8 -10
index c1c5fbb3c909d..75167b8580cc0 100644
@@ -1799,9 +1799,10 @@ int folio_alloc_swap(struct folio *folio)
 }
 
 /**
- * folio_dup_swap() - Increase swap count of swap entries of a folio.
+ * folio_dup_swap_pages() - Increase swap count of swap entries of a folio.
  * @folio: folio with swap entries bounded.
- * @page: if not NULL, only increase the swap count of this page.
+ * @page: the first page in the folio to increase the swap count for.
+ * @nr_pages: the number of pages in the folio to increase the swap count for.
  *
  * Typically called when the folio is unmapped and have its swap entry to
  * take its place: Swap entries allocated to a folio has count == 0 and pinned
@@ -1815,18 +1816,15 @@ int folio_alloc_swap(struct folio *folio)
  * swap_put_entries_direct on its swap entry before this helper returns, or
  * the swap count may underflow.
  */
-int folio_dup_swap(struct folio *folio, struct page *page)
+int folio_dup_swap_pages(struct folio *folio, struct page *page,
+		unsigned long nr_pages)
 {
-	swp_entry_t entry = folio->swap;
-	unsigned long nr_pages = folio_nr_pages(folio);
+	swp_entry_t entry = folio_page_swap_entry(folio, page);
+	unsigned long idx = folio_page_idx(folio, page);
 
 	VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
 	VM_WARN_ON_FOLIO(!folio_test_swapcache(folio), folio);
-
-	if (page) {
-		entry = folio_page_swap_entry(folio, page);
-		nr_pages = 1;
-	}
+	VM_WARN_ON_FOLIO(idx + nr_pages > folio_nr_pages(folio), folio);
 
 	return swap_dup_entries_cluster(swap_entry_to_info(entry),
 					swp_offset(entry), nr_pages);
Signature · 1 line
2.43.0
Reviewed-by on v3 1/9
#2 lore raw

Re: [PATCH v3 1/9] mm/swapfile: add batched version of folio_dup_swap

Quoted 134 lines · Dev Jain

Add folio_dup_swap_pages to handle a batch of consecutive pages. Note that folio_dup_swap already can handle a subset of this: nr_pages == 1 and nr_pages == folio_nr_pages(folio). Generalize this to any nr_pages.

Currently we have a not-so-nice logic of passing in subpage == NULL if we mean to exercise the logic on the entire folio, and subpage != NULL if we want to exercise the logic on only that subpage. Remove this
indirection: the caller invokes folio_dup_swap_pages() if it wants to operate on a range of pages in the folio (i.e nr_pages may be anything between 1 and folio_nr_pages()), and invokes folio_dup_swap() if it wants to operate on the entire folio.

---
 mm/rmap.c     |  2 +-
 mm/shmem.c    |  2 +-
 mm/swap.h     | 18 ++++++++++++++++--
 mm/swapfile.c | 18 ++++++++----------
 4 files changed, 26 insertions(+), 14 deletions(-)
mm/rmap.c +1 -1
index 6661bc11ce658..a70dbc8d544e1 100644
@@ -2150,7 +2150,7 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
        swp_entry_t entry = folio_page_swap_entry(folio, page);
        struct mm_struct *mm = vma->vm_mm;
 
-       if (folio_dup_swap(folio, page) < 0)
+       if (folio_dup_swap_pages(folio, page, 1) < 0)
                return false;
 
        /*
mm/shmem.c +1 -1
index b572c60f2af85..d5bd9506ce500 100644
@@ -1914,7 +1914,7 @@ int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
                        spin_unlock(&shmem_swaplist_lock);
                }
 
-               folio_dup_swap(folio, NULL);
+               folio_dup_swap(folio);
                shmem_delete_from_page_cache(folio, swp_to_radix_entry(folio->swap));
 
                BUG_ON(folio_mapped(folio));
mm/swap.h +16 -2
index b3b54c28929a1..47290510de264 100644
@@ -244,7 +244,8 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
  * folio_put_swap(): does the opposite thing of folio_dup_swap().
  */
 int folio_alloc_swap(struct folio *folio);
-int folio_dup_swap(struct folio *folio, struct page *page);
+int folio_dup_swap_pages(struct folio *folio, struct page *page,
+               unsigned long nr_pages);
 void folio_put_swap(struct folio *folio, struct page *page);
 
 /* For internal use */
@@ -368,7 +369,8 @@ static inline int folio_alloc_swap(struct folio *folio)
        return -EINVAL;
 }
 
-static inline int folio_dup_swap(struct folio *folio, struct page *page)
+static inline int folio_dup_swap_pages(struct folio *folio, struct page *page,
+               unsigned long nr_pages)
 {
        return -EINVAL;
 }
@@ -464,6 +466,18 @@ static inline void __swap_cache_replace_folio(struct swap_cluster_info *ci,
 }
 #endif /* CONFIG_SWAP */
 
+/**
+ * folio_dup_swap() - Increase swap count of all swap entries of a folio.
+ * @folio: folio with swap entries bound.
+ *
+ * See folio_dup_swap_pages() for more information.
+ */
+static inline int folio_dup_swap(struct folio *folio)
+{
+       return folio_dup_swap_pages(folio, folio_page(folio, 0),
+                                   folio_nr_pages(folio));
+}
+
 extern const struct swap_ops swap_bdev_ops;
 
 int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
mm/swapfile.c +8 -11
index c1c5fbb3c909d..75167b8580cc0 100644
@@ -1799,9 +1799,10 @@ int folio_alloc_swap(struct folio *folio)
 }
 
 /**
- * folio_dup_swap() - Increase swap count of swap entries of a folio.
+ * folio_dup_swap_pages() - Increase swap count of swap entries of a folio.
  * @folio: folio with swap entries bounded.
- * @page: if not NULL, only increase the swap count of this page.
+ * @page: the first page in the folio to increase the swap count for.
+ * @nr_pages: the number of pages in the folio to increase the swap count for.
  *
  * Typically called when the folio is unmapped and have its swap entry to
  * take its place: Swap entries allocated to a folio has count == 0 and pinned
@@ -1815,18 +1816,15 @@ int folio_alloc_swap(struct folio *folio)
  * swap_put_entries_direct on its swap entry before this helper returns, or
  * the swap count may underflow.
  */
-int folio_dup_swap(struct folio *folio, struct page *page)
+int folio_dup_swap_pages(struct folio *folio, struct page *page,
+               unsigned long nr_pages)
 {
-       swp_entry_t entry = folio->swap;
-       unsigned long nr_pages = folio_nr_pages(folio);
+       swp_entry_t entry = folio_page_swap_entry(folio, page);
+       unsigned long idx = folio_page_idx(folio, page);
 
        VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
        VM_WARN_ON_FOLIO(!folio_test_swapcache(folio), folio);
-
-       if (page) {
-               entry = folio_page_swap_entry(folio, page);
-               nr_pages = 1;
-       }
+       VM_WARN_ON_FOLIO(idx + nr_pages > folio_nr_pages(folio), folio);
 
        return swap_dup_entries_cluster(swap_entry_to_info(entry),
                                        swp_offset(entry), nr_pages);
--
Quoted 1 line

2.43.0

Looks good to me.

author
#3 lore raw

PATCH v3 2/9 mm/swapfile: add batched version of folio_put_swap

In reply to Dev Jain

Add folio_put_swap_pages to handle a batch of consecutive pages. Note that folio_put_swap already can handle a subset of this: nr_pages == 1 and nr_pages == folio_nr_pages(folio). Generalize this to any nr_pages.

Currently we have a not-so-nice logic of passing in subpage == NULL if we mean to exercise the logic on the entire folio, and subpage != NULL if we want to exercise the logic on only that subpage. Remove this
indirection: the caller invokes folio_put_swap_pages() if it wants to operate on a range of pages in the folio (i.e nr_pages may be anything between 1 and folio_nr_pages()), and invokes folio_put_swap() if it wants to operate on the entire folio.

---
 mm/memory.c   |  6 +++---
 mm/rmap.c     |  4 ++--
 mm/shmem.c    |  6 +++---
 mm/swap.h     | 17 +++++++++++++++--
 mm/swapfile.c | 21 ++++++++++-----------
 5 files changed, 33 insertions(+), 21 deletions(-)
mm/memory.c +3 -3
index 338fce99e7119..a7e979bfc814b 100644
@@ -5267,7 +5267,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 	if (unlikely(folio != swapcache)) {
 		folio_add_new_anon_rmap(folio, vma, address, RMAP_EXCLUSIVE);
 		folio_add_lru_vma(folio, vma);
-		folio_put_swap(swapcache, NULL);
+		folio_put_swap(swapcache);
 	} else if (!folio_test_anon(folio)) {
 		/*
 		 * We currently only expect !anon folios that are fully
@@ -5276,12 +5276,12 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 		VM_WARN_ON_ONCE_FOLIO(folio_nr_pages(folio) != nr_pages, folio);
 		VM_WARN_ON_ONCE_FOLIO(folio_mapped(folio), folio);
 		folio_add_new_anon_rmap(folio, vma, address, rmap_flags);
-		folio_put_swap(folio, NULL);
+		folio_put_swap(folio);
 	} else {
 		VM_WARN_ON_ONCE(nr_pages != 1 && nr_pages != folio_nr_pages(folio));
 		folio_add_anon_rmap_ptes(folio, page, nr_pages, vma, address,
 					 rmap_flags);
-		folio_put_swap(folio, nr_pages == 1 ? page : NULL);
+		folio_put_swap_pages(folio, page, nr_pages);
 	}
 
 	VM_BUG_ON(!folio_test_anon(folio) ||
mm/rmap.c +2 -2
index a70dbc8d544e1..fa9fc8374fc26 100644
@@ -2159,13 +2159,13 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
 	 * so we'll not check/care.
 	 */
 	if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-		folio_put_swap(folio, page);
+		folio_put_swap_pages(folio, page, 1);
 		return false;
 	}
 
 	/* See folio_try_share_anon_rmap(): clear PTE first. */
 	if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
-		folio_put_swap(folio, page);
+		folio_put_swap_pages(folio, page, 1);
 		return false;
 	}
 
mm/shmem.c +3 -3
index d5bd9506ce500..5f46061c6320c 100644
@@ -1935,7 +1935,7 @@ int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
 		/* Swap entry might be erased by racing shmem_free_swap() */
 		if (!error) {
 			shmem_recalc_inode(inode, 0, -nr_pages);
-			folio_put_swap(folio, NULL);
+			folio_put_swap(folio);
 		}
 
 		/*
@@ -2368,7 +2368,7 @@ static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
 
 	nr_pages = folio_nr_pages(folio);
 	folio_wait_writeback(folio);
-	folio_put_swap(folio, NULL);
+	folio_put_swap(folio);
 	swap_cache_del_folio(folio);
 	/*
 	 * Don't treat swapin error folio as alloced. Otherwise inode->i_blocks
@@ -2598,7 +2598,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
 	if (sgp == SGP_WRITE)
 		folio_mark_accessed(folio);
 
-	folio_put_swap(folio, NULL);
+	folio_put_swap(folio);
 	swap_cache_del_folio(folio);
 	folio_mark_dirty(folio);
 	put_swap_device(si);
mm/swap.h +15 -2
index 47290510de264..7ae915f594034 100644
@@ -246,7 +246,8 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
 int folio_alloc_swap(struct folio *folio);
 int folio_dup_swap_pages(struct folio *folio, struct page *page,
 		unsigned long nr_pages);
-void folio_put_swap(struct folio *folio, struct page *page);
+void folio_put_swap_pages(struct folio *folio, struct page *page,
+		unsigned long nr_pages);
 
 /* For internal use */
 extern void __swap_cluster_free_entries(struct swap_info_struct *si,
@@ -375,7 +376,8 @@ static inline int folio_dup_swap_pages(struct folio *folio, struct page *page,
 	return -EINVAL;
 }
 
-static inline void folio_put_swap(struct folio *folio, struct page *page)
+static inline void folio_put_swap_pages(struct folio *folio, struct page *page,
+		unsigned long nr_pages)
 {
 }
 
@@ -478,6 +480,17 @@ static inline int folio_dup_swap(struct folio *folio)
 				    folio_nr_pages(folio));
 }
 
+/**
+ * folio_put_swap() - Decrease swap count of all swap entries of a folio.
+ * @folio: folio with swap entries bound.
+ *
+ * See folio_put_swap_pages() for more information.
+ */
+static inline void folio_put_swap(struct folio *folio)
+{
+	folio_put_swap_pages(folio, folio_page(folio, 0), folio_nr_pages(folio));
+}
+
 extern const struct swap_ops swap_bdev_ops;
 
 int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
mm/swapfile.c +10 -11
index 75167b8580cc0..8b47e7f1e1931 100644
@@ -1831,27 +1831,25 @@ int folio_dup_swap_pages(struct folio *folio, struct page *page,
 }
 
 /**
- * folio_put_swap() - Decrease swap count of swap entries of a folio.
+ * folio_put_swap_pages() - Decrease swap count of swap entries of a folio.
  * @folio: folio with swap entries bounded, must be in swap cache and locked.
- * @page: if not NULL, only decrease the swap count of this page.
+ * @page: the first page in the folio to decrease the swap count for.
+ * @nr_pages: the number of pages in the folio to decrease the swap count for.
  *
  * This won't free the swap slots even if swap count drops to zero, they are
  * still pinned by the swap cache. User may call folio_free_swap to free them.
  * Context: Caller must ensure the folio is locked and in the swap cache.
  */
-void folio_put_swap(struct folio *folio, struct page *page)
+void folio_put_swap_pages(struct folio *folio, struct page *page,
+		unsigned long nr_pages)
 {
-	swp_entry_t entry = folio->swap;
-	unsigned long nr_pages = folio_nr_pages(folio);
+	swp_entry_t entry = folio_page_swap_entry(folio, page);
 	struct swap_info_struct *si = __swap_entry_to_info(entry);
+	unsigned long idx = folio_page_idx(folio, page);
 
 	VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
 	VM_WARN_ON_FOLIO(!folio_test_swapcache(folio), folio);
-
-	if (page) {
-		entry = folio_page_swap_entry(folio, page);
-		nr_pages = 1;
-	}
+	VM_WARN_ON_FOLIO(idx + nr_pages > folio_nr_pages(folio), folio);
 
 	swap_put_entries_cluster(si, swp_offset(entry), nr_pages, false);
 }
@@ -2535,7 +2533,8 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
 
 setpte:
 	set_pte_at(vma->vm_mm, addr, pte, new_pte);
-	folio_put_swap(swapcache, folio_file_page(swapcache, swp_offset(entry)));
+	folio_put_swap_pages(swapcache,
+			     folio_file_page(swapcache, swp_offset(entry)), 1);
 out:
 	if (pte)
 		pte_unmap_unlock(pte, ptl);
Signature · 1 line
2.43.0
Reviewed-by on v3 2/9
#4 lore raw

Re: [PATCH v3 2/9] mm/swapfile: add batched version of folio_put_swap

Quoted 196 lines · Dev Jain

Add folio_put_swap_pages to handle a batch of consecutive pages. Note that folio_put_swap already can handle a subset of this: nr_pages == 1 and nr_pages == folio_nr_pages(folio). Generalize this to any nr_pages.

Currently we have a not-so-nice logic of passing in subpage == NULL if we mean to exercise the logic on the entire folio, and subpage != NULL if we want to exercise the logic on only that subpage. Remove this
indirection: the caller invokes folio_put_swap_pages() if it wants to operate on a range of pages in the folio (i.e nr_pages may be anything between 1 and folio_nr_pages()), and invokes folio_put_swap() if it wants to operate on the entire folio.

---
 mm/memory.c   |  6 +++---
 mm/rmap.c     |  4 ++--
 mm/shmem.c    |  6 +++---
 mm/swap.h     | 17 +++++++++++++++--
 mm/swapfile.c | 21 ++++++++++-----------
 5 files changed, 33 insertions(+), 21 deletions(-)
mm/memory.c +3 -3
index 338fce99e7119..a7e979bfc814b 100644
@@ -5267,7 +5267,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
        if (unlikely(folio != swapcache)) {
                folio_add_new_anon_rmap(folio, vma, address, RMAP_EXCLUSIVE);
                folio_add_lru_vma(folio, vma);
-               folio_put_swap(swapcache, NULL);
+               folio_put_swap(swapcache);
        } else if (!folio_test_anon(folio)) {
                /*
                 * We currently only expect !anon folios that are fully
@@ -5276,12 +5276,12 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
                VM_WARN_ON_ONCE_FOLIO(folio_nr_pages(folio) != nr_pages, folio);
                VM_WARN_ON_ONCE_FOLIO(folio_mapped(folio), folio);
                folio_add_new_anon_rmap(folio, vma, address, rmap_flags);
-               folio_put_swap(folio, NULL);
+               folio_put_swap(folio);
        } else {
                VM_WARN_ON_ONCE(nr_pages != 1 && nr_pages != folio_nr_pages(folio));
                folio_add_anon_rmap_ptes(folio, page, nr_pages, vma, address,
                                         rmap_flags);
-               folio_put_swap(folio, nr_pages == 1 ? page : NULL);
+               folio_put_swap_pages(folio, page, nr_pages);
        }
 
        VM_BUG_ON(!folio_test_anon(folio) ||
mm/rmap.c +2 -2
index a70dbc8d544e1..fa9fc8374fc26 100644
@@ -2159,13 +2159,13 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
         * so we'll not check/care.
         */
        if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-               folio_put_swap(folio, page);
+               folio_put_swap_pages(folio, page, 1);
                return false;
        }
 
        /* See folio_try_share_anon_rmap(): clear PTE first. */
        if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
-               folio_put_swap(folio, page);
+               folio_put_swap_pages(folio, page, 1);
                return false;
        }
 
mm/shmem.c +3 -3
index d5bd9506ce500..5f46061c6320c 100644
@@ -1935,7 +1935,7 @@ int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
                /* Swap entry might be erased by racing shmem_free_swap() */
                if (!error) {
                        shmem_recalc_inode(inode, 0, -nr_pages);
-                       folio_put_swap(folio, NULL);
+                       folio_put_swap(folio);
                }
 
                /*
@@ -2368,7 +2368,7 @@ static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
 
        nr_pages = folio_nr_pages(folio);
        folio_wait_writeback(folio);
-       folio_put_swap(folio, NULL);
+       folio_put_swap(folio);
        swap_cache_del_folio(folio);
        /*
         * Don't treat swapin error folio as alloced. Otherwise inode->i_blocks
@@ -2598,7 +2598,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
        if (sgp == SGP_WRITE)
                folio_mark_accessed(folio);
 
-       folio_put_swap(folio, NULL);
+       folio_put_swap(folio);
        swap_cache_del_folio(folio);
        folio_mark_dirty(folio);
        put_swap_device(si);
mm/swap.h +15 -2
index 47290510de264..7ae915f594034 100644
@@ -246,7 +246,8 @@ extern int swap_retry_table_alloc(swp_entry_t entry, gfp_t gfp);
 int folio_alloc_swap(struct folio *folio);
 int folio_dup_swap_pages(struct folio *folio, struct page *page,
                unsigned long nr_pages);
-void folio_put_swap(struct folio *folio, struct page *page);
+void folio_put_swap_pages(struct folio *folio, struct page *page,
+               unsigned long nr_pages);
 
 /* For internal use */
 extern void __swap_cluster_free_entries(struct swap_info_struct *si,
@@ -375,7 +376,8 @@ static inline int folio_dup_swap_pages(struct folio *folio, struct page *page,
        return -EINVAL;
 }
 
-static inline void folio_put_swap(struct folio *folio, struct page *page)
+static inline void folio_put_swap_pages(struct folio *folio, struct page *page,
+               unsigned long nr_pages)
 {
 }
 
@@ -478,6 +480,17 @@ static inline int folio_dup_swap(struct folio *folio)
                                    folio_nr_pages(folio));
 }
 
+/**
+ * folio_put_swap() - Decrease swap count of all swap entries of a folio.
+ * @folio: folio with swap entries bound.
+ *
+ * See folio_put_swap_pages() for more information.
+ */
+static inline void folio_put_swap(struct folio *folio)
+{
+       folio_put_swap_pages(folio, folio_page(folio, 0), folio_nr_pages(folio));
+}
+
 extern const struct swap_ops swap_bdev_ops;
 
 int shmem_writeout(struct swap_io_ctx *ctx, struct folio *folio,
mm/swapfile.c +10 -12
index 75167b8580cc0..8b47e7f1e1931 100644
@@ -1831,27 +1831,25 @@ int folio_dup_swap_pages(struct folio *folio, struct page *page,
 }
 
 /**
- * folio_put_swap() - Decrease swap count of swap entries of a folio.
+ * folio_put_swap_pages() - Decrease swap count of swap entries of a folio.
  * @folio: folio with swap entries bounded, must be in swap cache and locked.
- * @page: if not NULL, only decrease the swap count of this page.
+ * @page: the first page in the folio to decrease the swap count for.
+ * @nr_pages: the number of pages in the folio to decrease the swap count for.
  *
  * This won't free the swap slots even if swap count drops to zero, they are
  * still pinned by the swap cache. User may call folio_free_swap to free them.
  * Context: Caller must ensure the folio is locked and in the swap cache.
  */
-void folio_put_swap(struct folio *folio, struct page *page)
+void folio_put_swap_pages(struct folio *folio, struct page *page,
+               unsigned long nr_pages)
 {
-       swp_entry_t entry = folio->swap;
-       unsigned long nr_pages = folio_nr_pages(folio);
+       swp_entry_t entry = folio_page_swap_entry(folio, page);
        struct swap_info_struct *si = __swap_entry_to_info(entry);
+       unsigned long idx = folio_page_idx(folio, page);
 
        VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
        VM_WARN_ON_FOLIO(!folio_test_swapcache(folio), folio);
-
-       if (page) {
-               entry = folio_page_swap_entry(folio, page);
-               nr_pages = 1;
-       }
+       VM_WARN_ON_FOLIO(idx + nr_pages > folio_nr_pages(folio), folio);
 
        swap_put_entries_cluster(si, swp_offset(entry), nr_pages, false);
 }
@@ -2535,7 +2533,8 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
 
 setpte:
        set_pte_at(vma->vm_mm, addr, pte, new_pte);
-       folio_put_swap(swapcache, folio_file_page(swapcache, swp_offset(entry)));
+       folio_put_swap_pages(swapcache,
+                            folio_file_page(swapcache, swp_offset(entry)), 1);
 out:
        if (pte)
                pte_unmap_unlock(pte, ptl);
--
Quoted 1 line

2.43.0

author
#5 lore raw

PATCH v3 3/9 mm: move anon-exclusive batch helper to rmap.h

In reply to Dev Jain

In preparation for optimizing large folio unmapping, we need to reuse the page_anon_exclusive_batch helper in rmap.c and rmap.h and obey the existing use in mprotect.c .

Therefore, move it from mprotect.c to rmap.h.

While at it, change return type, start_idx and max_len to unsigned long type for future proofing against THP support at >= PUD level. Also shorten expected_anon_exclusive -> anon_exclusive.

---
 include/linux/rmap.h | 17 +++++++++++++++++
 mm/mprotect.c        | 18 +-----------------
 2 files changed, 18 insertions(+), 17 deletions(-)
include/linux/rmap.h +17 -0
index 74cca0e3c7264..62ef511a6175a 100644
@@ -106,6 +106,23 @@ enum ttu_flags {
 
 #ifdef CONFIG_MMU
 
+/*
+ * Get max length of consecutive PTEs pointing to PageAnonExclusive() pages or
+ * !PageAnonExclusive() pages, starting from start_idx. Caller must enforce
+ * that the PTEs point to consecutive pages of the same anon large folio.
+ */
+static __always_inline unsigned long page_anon_exclusive_batch(unsigned long start_idx,
+		unsigned long max_len, struct page *first_page, bool anon_exclusive)
+{
+	unsigned long idx;
+
+	for (idx = start_idx + 1; idx < start_idx + max_len; ++idx) {
+		if (anon_exclusive != PageAnonExclusive(first_page + idx))
+			break;
+	}
+	return idx - start_idx;
+}
+
 void anon_vma_init(void);	/* create anon_vma_cachep */
 
 #ifdef CONFIG_MM_ID
mm/mprotect.c +1 -17
index a1b6d29bf0390..cb996dbd0e525 100644
@@ -30,6 +30,7 @@
 #include <linux/mm_inline.h>
 #include <linux/pgtable.h>
 #include <linux/userfaultfd_k.h>
+#include <linux/rmap.h>
 #include <uapi/linux/mman.h>
 #include <asm/cacheflush.h>
 #include <asm/mmu_context.h>
@@ -138,23 +139,6 @@ static __always_inline void prot_commit_flush_ptes(struct vm_area_struct *vma,
 		tlb_flush_pte_range(tlb, addr, nr_ptes * PAGE_SIZE);
 }
 
-/*
- * Get max length of consecutive ptes pointing to PageAnonExclusive() pages or
- * !PageAnonExclusive() pages, starting from start_idx. Caller must enforce
- * that the ptes point to consecutive pages of the same anon large folio.
- */
-static __always_inline int page_anon_exclusive_batch(int start_idx, int max_len,
-		struct page *first_page, bool expected_anon_exclusive)
-{
-	int idx;
-
-	for (idx = start_idx + 1; idx < start_idx + max_len; ++idx) {
-		if (expected_anon_exclusive != PageAnonExclusive(first_page + idx))
-			break;
-	}
-	return idx - start_idx;
-}
-
 /*
  * This function is a result of trying our very best to retain the
  * "avoid the write-fault handler" optimization. In can_change_pte_writable(),
Signature · 1 line
2.43.0
reviewer Reviewed-by on v3 3/9
#6 lore raw

Re: [PATCH v3 3/9] mm: move anon-exclusive batch helper to rmap.h

Quoted 10 lines · Dev Jain

In preparation for optimizing large folio unmapping, we need to reuse the page_anon_exclusive_batch helper in rmap.c and rmap.h and obey the existing use in mprotect.c .

Therefore, move it from mprotect.c to rmap.h.

While at it, change return type, start_idx and max_len to unsigned long type for future proofing against THP support at >= PUD level. Also shorten expected_anon_exclusive -> anon_exclusive.

I really think the expected prefix makes the semantics clearer, so I don't think it's necessary to remove it.

If you really find it too long, maybe just use expected_exclusive, since we're already in the anon context and know that it is definitely anon.

Another option might be to make *exclusive an output parameter. Then we wouldn't need to call
expected_anon_exclusive = PageAnonExclusive(first_page + batch_idx); in the caller.

static __always_inline unsigned long
page_anon_exclusive_batch(unsigned long start_idx,
unsigned long max_len, struct page *first_page, bool *exclusive)

Quoted 2 lines

Otherwise, the patch looks good to me.

Quoted 20 lines
---
 include/linux/rmap.h | 17 +++++++++++++++++
 mm/mprotect.c        | 18 +-----------------
 2 files changed, 18 insertions(+), 17 deletions(-)
include/linux/rmap.h +7 -0
index 74cca0e3c7264..62ef511a6175a 100644
@@ -106,6 +106,23 @@ enum ttu_flags {
 
 #ifdef CONFIG_MMU
 
+/*
+ * Get max length of consecutive PTEs pointing to PageAnonExclusive() pages or
+ * !PageAnonExclusive() pages, starting from start_idx. Caller must enforce
+ * that the PTEs point to consecutive pages of the same anon large folio.
+ */
+static __always_inline unsigned long page_anon_exclusive_batch(unsigned long start_idx,
+               unsigned long max_len, struct page *first_page, bool anon_exclusive)

include/linux/rmap.h enum ttu_flags { · line 125

Quoted 10 lines
include/linux/rmap.h +10 -0
+{
+       unsigned long idx;
+
+       for (idx = start_idx + 1; idx < start_idx + max_len; ++idx) {
+               if (anon_exclusive != PageAnonExclusive(first_page + idx))
+                       break;
+       }
+       return idx - start_idx;
+}
+

Thanks
Barry

author
#7 lore raw

Re: [PATCH v3 3/9] mm: move anon-exclusive batch helper to rmap.h

Quoted 15 lines · Barry Song
Quoted 10 lines · Dev Jain

In preparation for optimizing large folio unmapping, we need to reuse the page_anon_exclusive_batch helper in rmap.c and rmap.h and obey the existing use in mprotect.c .

Therefore, move it from mprotect.c to rmap.h.

While at it, change return type, start_idx and max_len to unsigned long type for future proofing against THP support at >= PUD level. Also shorten expected_anon_exclusive -> anon_exclusive.

I really think the expected prefix makes the semantics clearer, so I don't think it's necessary to remove it.

Quoted 8 lines

If you really find it too long, maybe just use expected_exclusive, since we're already in the anon context and know that it is definitely anon.

Another option might be to make *exclusive an output parameter. Then we wouldn't need to call
expected_anon_exclusive = PageAnonExclusive(first_page + batch_idx); in the caller.

Ah nice! This looks much cleaner, thanks.

Quoted 11 lines

static __always_inline unsigned long
page_anon_exclusive_batch(unsigned long start_idx,
unsigned long max_len, struct page *first_page, bool *exclusive)

Quoted 2 lines

Otherwise, the patch looks good to me.

Thanks.

Quoted 34 lines
Quoted 21 lines
---
 include/linux/rmap.h | 17 +++++++++++++++++
 mm/mprotect.c        | 18 +-----------------
 2 files changed, 18 insertions(+), 17 deletions(-)
include/linux/rmap.h +7 -0
index 74cca0e3c7264..62ef511a6175a 100644
@@ -106,6 +106,23 @@ enum ttu_flags {
 
 #ifdef CONFIG_MMU
 
+/*
+ * Get max length of consecutive PTEs pointing to PageAnonExclusive() pages or
+ * !PageAnonExclusive() pages, starting from start_idx. Caller must enforce
+ * that the PTEs point to consecutive pages of the same anon large folio.
+ */
+static __always_inline unsigned long page_anon_exclusive_batch(unsigned long start_idx,
+               unsigned long max_len, struct page *first_page, bool anon_exclusive)

include/linux/rmap.h enum ttu_flags { · line 125

Quoted 10 lines
include/linux/rmap.h +10 -0
+{
+       unsigned long idx;
+
+       for (idx = start_idx + 1; idx < start_idx + max_len; ++idx) {
+               if (anon_exclusive != PageAnonExclusive(first_page + idx))
+                       break;
+       }
+       return idx - start_idx;
+}
+

Thanks
Barry

author
#8 lore raw

PATCH v3 4/9 mm/rmap: Add batched version of folio_try_share_anon_rmap_pte

In reply to Dev Jain

To enable batched unmapping of anonymous folios, we need to handle the sharing of exclusive pages. Hence, a batched version of folio_try_share_anon_rmap_pte is required.

Currently, the sole purpose of nr_pages in __folio_try_share_anon_rmap is to do some rmap sanity checks. Now, clear the PageAnonExclusive bit on a batch of nr_pages. Refactor the function such that the clearing of the bit can be done at one place without duplication.

Note that __folio_try_share_anon_rmap can receive nr_pages == HPAGE_PMD_NR from the PMD path, but currently we only clear the bit on the head page. Retain this behaviour by setting nr_pages = 1 in case the caller is folio_try_share_anon_rmap_pmd.

While at it, convert nr_pages to unsigned long to future-proof from overflow in case P4D-huge mappings etc get supported down the road. I haven't made such a change in each function receiving nr_pages in try_to_unmap_one - perhaps this can be done incrementally.

Add two WARN's: check that the batch is entirely exclusive (for PMD callers, need to check only head page), and that there are only PTE/PMD paths converging into __folio_try_share_anon_rmap.

---
 include/linux/rmap.h | 56 ++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 17 deletions(-)
include/linux/rmap.h +39 -17
index 62ef511a6175a..91b5763ef466f 100644
@@ -723,17 +723,23 @@ static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio,
 }
 
 static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
-		struct page *page, int nr_pages, enum pgtable_level level)
+		struct page *page, unsigned long nr_pages, enum pgtable_level level)
 {
+	/* device private folios cannot get pinned via GUP. */
+	const bool pinnable = !folio_is_device_private(folio);
+
 	VM_WARN_ON_FOLIO(!folio_test_anon(folio), folio);
 	VM_WARN_ON_FOLIO(!PageAnonExclusive(page), folio);
+
 	__folio_rmap_sanity_checks(folio, page, nr_pages, level);
 
-	/* device private folios cannot get pinned via GUP. */
-	if (unlikely(folio_is_device_private(folio))) {
-		ClearPageAnonExclusive(page);
-		return 0;
-	}
+	VM_WARN_ON_ONCE(level != PGTABLE_LEVEL_PTE && level != PGTABLE_LEVEL_PMD);
+
+	/* We only clear anon-exclusive from head page of PMD folio. */
+	if (level == PGTABLE_LEVEL_PMD)
+		nr_pages = 1;
+
+	VM_WARN_ON_FOLIO(page_anon_exclusive_batch(0, nr_pages, page, true) != nr_pages, folio);
 
 	/*
 	 * We have to make sure that when we clear PageAnonExclusive, that
@@ -777,29 +783,38 @@ static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
 	 * so we use explicit ones here.
 	 */
 
-	/* Paired with the memory barrier in try_grab_folio(). */
-	if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
-		smp_mb();
+	if (likely(pinnable)) {
+		/* Paired with the memory barrier in try_grab_folio(). */
+		if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
+			smp_mb();
 
-	if (unlikely(folio_maybe_dma_pinned(folio)))
-		return -EBUSY;
-	ClearPageAnonExclusive(page);
+		if (unlikely(folio_maybe_dma_pinned(folio)))
+			return -EBUSY;
+	}
+
+	for (;;) {
+		ClearPageAnonExclusive(page);
+		if (--nr_pages == 0)
+			break;
+		page++;
+	}
 
 	/*
 	 * This is conceptually a smp_wmb() paired with the smp_rmb() in
 	 * gup_must_unshare().
 	 */
-	if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
+	if (likely(pinnable) && IS_ENABLED(CONFIG_HAVE_GUP_FAST))
 		smp_mb__after_atomic();
 	return 0;
 }
 
 /**
- * folio_try_share_anon_rmap_pte - try marking an exclusive anonymous page
- *				   mapped by a PTE possibly shared to prepare
+ * folio_try_share_anon_rmap_ptes - try marking exclusive anonymous pages
+ *				   mapped by PTEs possibly shared to prepare
  *				   for KSM or temporary unmapping
  * @folio:	The folio to share a mapping of
- * @page:	The mapped exclusive page
+ * @page:	The first mapped exclusive page of the batch in the folio
+ * @nr_pages:	The number of pages to share in the folio (batch size)
  *
  * The caller needs to hold the page table lock and has to have the page table
  * entries cleared/invalidated.
@@ -815,10 +830,17 @@ static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
  * Returns 0 if marking the mapped page possibly shared succeeded. Returns
  * -EBUSY otherwise.
  */
+static inline int folio_try_share_anon_rmap_ptes(struct folio *folio,
+		struct page *page, unsigned long nr_pages)
+{
+	return __folio_try_share_anon_rmap(folio, page, nr_pages,
+					   PGTABLE_LEVEL_PTE);
+}
+
 static inline int folio_try_share_anon_rmap_pte(struct folio *folio,
 		struct page *page)
 {
-	return __folio_try_share_anon_rmap(folio, page, 1, PGTABLE_LEVEL_PTE);
+	return folio_try_share_anon_rmap_ptes(folio, page, 1);
 }
 
 /**
Signature · 1 line
2.43.0
reviewer
#9 lore raw

Re: [PATCH v3 4/9] mm/rmap: Add batched version of folio_try_share_anon_rmap_pte

Quoted 20 lines · Dev Jain

To enable batched unmapping of anonymous folios, we need to handle the sharing of exclusive pages. Hence, a batched version of folio_try_share_anon_rmap_pte is required.

Currently, the sole purpose of nr_pages in __folio_try_share_anon_rmap is to do some rmap sanity checks. Now, clear the PageAnonExclusive bit on a batch of nr_pages. Refactor the function such that the clearing of the bit can be done at one place without duplication.

Note that __folio_try_share_anon_rmap can receive nr_pages == HPAGE_PMD_NR from the PMD path, but currently we only clear the bit on the head page. Retain this behaviour by setting nr_pages = 1 in case the caller is folio_try_share_anon_rmap_pmd.

While at it, convert nr_pages to unsigned long to future-proof from overflow in case P4D-huge mappings etc get supported down the road. I haven't made such a change in each function receiving nr_pages in try_to_unmap_one - perhaps this can be done incrementally.

Quoted 5 lines

Add two WARN's: check that the batch is entirely exclusive (for PMD callers, need to check only head page), and that there are only PTE/PMD paths converging into __folio_try_share_anon_rmap.

The patch looks correct to me, but it seems a bit hard to follow because of the pinnable handling.

Quoted 9 lines
---
 include/linux/rmap.h | 56 ++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 17 deletions(-)
include/linux/rmap.h +0 -0
index 62ef511a6175a..91b5763ef466f 100644

include/linux/rmap.h static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio, · line 730

Quoted 9 lines
include/linux/rmap.h +4 -1
@@ -723,17 +723,23 @@ static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio,
 }
 
 static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
-               struct page *page, int nr_pages, enum pgtable_level level)
+               struct page *page, unsigned long nr_pages, enum pgtable_level level)
 {
+       /* device private folios cannot get pinned via GUP. */
+       const bool pinnable = !folio_is_device_private(folio);
+

If we make a helper that does:

for (;;) {
     ClearPageAnonExclusive(page);
     if (--nr_pages == 0)
               break;
     page++;
}

wouldn't that allow us to avoid this ugly pinnable variant?

Then we can make a much smaller modification, which should also be much easier to follow? No?

Best Regards
Barry

author
#10 lore raw

Re: [PATCH v3 4/9] mm/rmap: Add batched version of folio_try_share_anon_rmap_pte

Quoted 58 lines · Barry Song
Quoted 20 lines · Dev Jain

To enable batched unmapping of anonymous folios, we need to handle the sharing of exclusive pages. Hence, a batched version of folio_try_share_anon_rmap_pte is required.

Currently, the sole purpose of nr_pages in __folio_try_share_anon_rmap is to do some rmap sanity checks. Now, clear the PageAnonExclusive bit on a batch of nr_pages. Refactor the function such that the clearing of the bit can be done at one place without duplication.

Note that __folio_try_share_anon_rmap can receive nr_pages == HPAGE_PMD_NR from the PMD path, but currently we only clear the bit on the head page. Retain this behaviour by setting nr_pages = 1 in case the caller is folio_try_share_anon_rmap_pmd.

While at it, convert nr_pages to unsigned long to future-proof from overflow in case P4D-huge mappings etc get supported down the road. I haven't made such a change in each function receiving nr_pages in try_to_unmap_one - perhaps this can be done incrementally.

Quoted 5 lines

Add two WARN's: check that the batch is entirely exclusive (for PMD callers, need to check only head page), and that there are only PTE/PMD paths converging into __folio_try_share_anon_rmap.

The patch looks correct to me, but it seems a bit hard to follow because of the pinnable handling.

Quoted 9 lines
---
 include/linux/rmap.h | 56 ++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 17 deletions(-)
include/linux/rmap.h +0 -0
index 62ef511a6175a..91b5763ef466f 100644

include/linux/rmap.h static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio, · line 730

Quoted 9 lines
include/linux/rmap.h +4 -1
@@ -723,17 +723,23 @@ static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio,
 }
 
 static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
-               struct page *page, int nr_pages, enum pgtable_level level)
+               struct page *page, unsigned long nr_pages, enum pgtable_level level)
 {
+       /* device private folios cannot get pinned via GUP. */
+       const bool pinnable = !folio_is_device_private(folio);
+

If we make a helper that does:

for (;;) {
     ClearPageAnonExclusive(page);
     if (--nr_pages == 0)
               break;
     page++;
}
Quoted 4 lines

wouldn't that allow us to avoid this ugly pinnable variant?

Then we can make a much smaller modification, which should also be much easier to follow? No?

David had suggested here:

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

The argument was that the helper would have no context of the pages we are operating on. Although you are correct that this makes the function harder to follow.

Maybe, revert to the original function, and have this helper folio_clear_pages_anon_exclusive, which will take the folio and do the page_anon_exclusive_batch = nr_pages sanity check, and add kerneldoc to it which says pages should be within folio?

The kerneldoc can be saved by calling __folio_rmap_sanity_checks in folio_clear_pages_anon_exclusive - that will cause duplication, but then it's all a VM_WARN_ON function so maybe we don't care?

Quoted 3 lines

Best Regards
Barry

reviewer Reviewed-by
#11 lore raw

Re: [PATCH v3 4/9] mm/rmap: Add batched version of folio_try_share_anon_rmap_pte

Quoted 75 lines · Dev Jain
Quoted 58 lines · Barry Song
Quoted 20 lines · Dev Jain

To enable batched unmapping of anonymous folios, we need to handle the sharing of exclusive pages. Hence, a batched version of folio_try_share_anon_rmap_pte is required.

Currently, the sole purpose of nr_pages in __folio_try_share_anon_rmap is to do some rmap sanity checks. Now, clear the PageAnonExclusive bit on a batch of nr_pages. Refactor the function such that the clearing of the bit can be done at one place without duplication.

Note that __folio_try_share_anon_rmap can receive nr_pages == HPAGE_PMD_NR from the PMD path, but currently we only clear the bit on the head page. Retain this behaviour by setting nr_pages = 1 in case the caller is folio_try_share_anon_rmap_pmd.

While at it, convert nr_pages to unsigned long to future-proof from overflow in case P4D-huge mappings etc get supported down the road. I haven't made such a change in each function receiving nr_pages in try_to_unmap_one - perhaps this can be done incrementally.

Quoted 5 lines

Add two WARN's: check that the batch is entirely exclusive (for PMD callers, need to check only head page), and that there are only PTE/PMD paths converging into __folio_try_share_anon_rmap.

The patch looks correct to me, but it seems a bit hard to follow because of the pinnable handling.

Quoted 9 lines
---
 include/linux/rmap.h | 56 ++++++++++++++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 17 deletions(-)
include/linux/rmap.h +0 -0
index 62ef511a6175a..91b5763ef466f 100644

include/linux/rmap.h static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio, · line 730

Quoted 9 lines
include/linux/rmap.h +4 -1
@@ -723,17 +723,23 @@ static inline int folio_try_dup_anon_rmap_pmd(struct folio *folio,
 }
 
 static __always_inline int __folio_try_share_anon_rmap(struct folio *folio,
-               struct page *page, int nr_pages, enum pgtable_level level)
+               struct page *page, unsigned long nr_pages, enum pgtable_level level)
 {
+       /* device private folios cannot get pinned via GUP. */
+       const bool pinnable = !folio_is_device_private(folio);
+

If we make a helper that does:

for (;;) {
     ClearPageAnonExclusive(page);
     if (--nr_pages == 0)
               break;
     page++;
}
Quoted 4 lines

wouldn't that allow us to avoid this ugly pinnable variant?

Then we can make a much smaller modification, which should also be much easier to follow? No?

David had suggested here:

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

The argument was that the helper would have no context of the pages we are operating on. Although you are correct that this makes the function harder to follow.

Quoted 9 lines

Maybe, revert to the original function, and have this helper folio_clear_pages_anon_exclusive, which will take the folio and do the page_anon_exclusive_batch = nr_pages sanity check, and add kerneldoc to it which says pages should be within folio?

The kerneldoc can be saved by calling __folio_rmap_sanity_checks in folio_clear_pages_anon_exclusive - that will cause duplication, but then it's all a VM_WARN_ON function so maybe we don't care?

Never mind. If you and David have already discussed this, then it is all good to go with your current code. Everyone has a different coding style and taste :-)

For the patch, feel free to add:

author
#12 lore raw

PATCH v3 5/9 mm/internal: rename swap offset helpers to softleaf offset

In reply to Dev Jain

In preparation for adding a helper to set softleaf PTEs in one go, generalize the helpers that advance a swap-entry offset so they operate on softleaf entries.

Softleaf entries use the same type/offset layout as swap entries, so advancing the offset works for swap softleaves and PFN-bearing softleaves. Preserve soft-dirty and uffd bits across the move. The swap-exclusive bit is meaningful only for swap softleaves, so guard that preservation with softleaf_is_swap().

---
 mm/internal.h | 31 ++++++++++++++++---------------
 mm/memory.c   |  4 ++--
 2 files changed, 18 insertions(+), 17 deletions(-)
mm/internal.h +16 -15
index 3b9fdb826162d..b8ec379b90bb0 100644
@@ -469,16 +469,16 @@ unsigned int folio_pte_batch(struct folio *folio, pte_t *ptep, pte_t pte,
 		unsigned int max_nr);
 
 /**
- * pte_move_swp_offset - Move the swap entry offset field of a swap pte
- *	 forward or backward by delta
- * @pte: The initial pte state; must be a swap entry
+ * pte_move_softleaf_offset - Move the softleaf entry offset field of a
+ * softleaf pte forward or backward by delta
+ * @pte: The initial pte state; must be a softleaf entry
  * @delta: The direction and the offset we are moving; forward if delta
  *	 is positive; backward if delta is negative
  *
- * Moves the swap offset, while maintaining all other fields, including
- * swap type, and any swp pte bits. The resulting pte is returned.
+ * Moves the softleaf offset, while maintaining all other fields, including
+ * softleaf type, and any softleaf pte bits. The resulting pte is returned.
  */
-static inline pte_t pte_move_swp_offset(pte_t pte, long delta)
+static inline pte_t pte_move_softleaf_offset(pte_t pte, long delta)
 {
 	const softleaf_t entry = softleaf_from_pte(pte);
 	pte_t new = __swp_entry_to_pte(__swp_entry(swp_type(entry),
@@ -486,7 +486,7 @@ static inline pte_t pte_move_swp_offset(pte_t pte, long delta)
 
 	if (pte_swp_soft_dirty(pte))
 		new = pte_swp_mksoft_dirty(new);
-	if (pte_swp_exclusive(pte))
+	if (softleaf_is_swap(entry) && pte_swp_exclusive(pte))
 		new = pte_swp_mkexclusive(new);
 	if (pte_swp_uffd(pte))
 		new = pte_swp_mkuffd(new);
@@ -496,15 +496,16 @@ static inline pte_t pte_move_swp_offset(pte_t pte, long delta)
 
 
 /**
- * pte_next_swp_offset - Increment the swap entry offset field of a swap pte.
- * @pte: The initial pte state; must be a swap entry.
+ * pte_next_softleaf_offset - Increment the softleaf entry offset field of a
+ * non-present pte.
+ * @pte: The initial pte state; must be a softleaf entry.
  *
- * Increments the swap offset, while maintaining all other fields, including
- * swap type, and any swp pte bits. The resulting pte is returned.
+ * Increments the softleaf offset, while maintaining all other fields, including
+ * softleaf type, and any softleaf pte bits. The resulting pte is returned.
  */
-static inline pte_t pte_next_swp_offset(pte_t pte)
+static inline pte_t pte_next_softleaf_offset(pte_t pte)
 {
-	return pte_move_swp_offset(pte, 1);
+	return pte_move_softleaf_offset(pte, 1);
 }
 
 /**
@@ -524,7 +525,7 @@ static inline pte_t pte_next_swp_offset(pte_t pte)
  */
 static inline int swap_pte_batch(pte_t *start_ptep, int max_nr, pte_t pte)
 {
-	pte_t expected_pte = pte_next_swp_offset(pte);
+	pte_t expected_pte = pte_next_softleaf_offset(pte);
 	const pte_t *end_ptep = start_ptep + max_nr;
 	pte_t *ptep = start_ptep + 1;
 
@@ -536,7 +537,7 @@ static inline int swap_pte_batch(pte_t *start_ptep, int max_nr, pte_t pte)
 
 		if (!pte_same(pte, expected_pte))
 			break;
-		expected_pte = pte_next_swp_offset(expected_pte);
+		expected_pte = pte_next_softleaf_offset(expected_pte);
 		ptep++;
 	}
 
mm/memory.c +2 -2
index a7e979bfc814b..60fd0a08b35f5 100644
@@ -4816,7 +4816,7 @@ static bool can_swapin_thp(struct vm_fault *vmf, pte_t *ptep, int nr_pages)
 	idx = (vmf->address - addr) / PAGE_SIZE;
 	pte = ptep_get(ptep);
 
-	if (!pte_same(pte, pte_move_swp_offset(vmf->orig_pte, -idx)))
+	if (!pte_same(pte, pte_move_softleaf_offset(vmf->orig_pte, -idx)))
 		return false;
 	/*
 	 * swap_read_folio() can't handle the case a large folio is hybridly
@@ -5131,7 +5131,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 
 		folio_ptep = vmf->pte - idx;
 		folio_pte = ptep_get(folio_ptep);
-		if (!pte_same(folio_pte, pte_move_swp_offset(vmf->orig_pte, -idx)) ||
+		if (!pte_same(folio_pte, pte_move_softleaf_offset(vmf->orig_pte, -idx)) ||
 		    swap_pte_batch(folio_ptep, nr, folio_pte) != nr)
 			goto check_folio;
 
Signature · 1 line
2.43.0
author
#13 lore raw

PATCH v3 6/9 mm/internal: add set_softleaf_ptes

In reply to Dev Jain

Currently we have a helper called set_ptes() which is used to set consecutive present ptes in the pgtables. To do the same operation but to set "consecutive" nonpresent (softleaf) ptes, add set_softleaf_ptes().

The softleaves which have a notion of consecutivity is swap softleaf, and those grouped by softeaf_has_pfn(). The latter is trivial; future code can convert present ptes pointing to the same large folio to swap softleaves with consecutive offsets using this helper.

The other case is softleaf markers. They do not have a notion of a swap offset or PFN, so future code can use set_softleaf_ptes() to store multiple (same) markers on the ptes.

---
 mm/internal.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
mm/internal.h +33 -0
index b8ec379b90bb0..0ffc9565c3273 100644
@@ -508,6 +508,39 @@ static inline pte_t pte_next_softleaf_offset(pte_t pte)
 	return pte_move_softleaf_offset(pte, 1);
 }
 
+/**
+ * set_softleaf_ptes - Set consecutive softleaf PTEs.
+ * @mm: Address space the PTEs belong to.
+ * @addr: Address of the first PTE.
+ * @ptep: Page table pointer for the first PTE.
+ * @pte: PTE to set for the first entry.
+ * @nr: Number of PTEs to set.
+ *
+ * Install @nr softleaf PTEs, advancing @pte when its softleaf entry
+ * represents consecutive offsets. Swap entries advance through swap offsets,
+ * PFN softleaf entries advance through PFNs (encoded by swap offset), and
+ * marker entries are repeated unchanged.
+ */
+static inline void set_softleaf_ptes(struct mm_struct *mm, unsigned long addr,
+		pte_t *ptep, pte_t pte, unsigned long nr)
+{
+	softleaf_t entry;
+	bool advance;
+
+	entry = softleaf_from_pte(pte);
+	advance = softleaf_is_swap(entry) || softleaf_has_pfn(entry);
+
+	for (;;) {
+		set_pte_at(mm, addr, ptep, pte);
+		if (--nr == 0)
+			break;
+		if (advance)
+			pte = pte_next_softleaf_offset(pte);
+		ptep++;
+		addr += PAGE_SIZE;
+	}
+}
+
 /**
  * swap_pte_batch - detect a PTE batch for a set of contiguous swap entries
  * @start_ptep: Page table pointer for the first entry.
Signature · 1 line
2.43.0
author
#14 lore raw

PATCH v3 7/9 mm/memory: use set_softleaf_ptes for uffd-wp markers

In reply to Dev Jain

Use set_softleaf_ptes() to store multiple uffd-wp marker entries instead of open coding the batched setting.

---
 mm/memory.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
mm/memory.c +2 -8
index 60fd0a08b35f5..bed5cd90356f4 100644
@@ -1700,14 +1700,8 @@ bool cond_install_uffd_wp_ptes(struct vm_area_struct *vma,
 	if (likely(!arm_uffd_pte))
 		return false;
 
-	for (;;) {
-		set_pte_at(vma->vm_mm, addr, ptep,
-			   make_pte_marker(PTE_MARKER_UFFD_WP));
-		if (--nr_ptes == 0)
-			break;
-		ptep++;
-		addr += PAGE_SIZE;
-	}
+	set_softleaf_ptes(vma->vm_mm, addr, ptep,
+			  make_pte_marker(PTE_MARKER_UFFD_WP), nr_ptes);
 
 	return true;
 }
Signature · 1 line
2.43.0
author
#15 lore raw

PATCH v3 8/9 mm/rmap: batch unmap anonymous swap-backed large folios

In reply to Dev Jain

Enable batch clearing of PTEs and batch swap setting of PTEs for anon swap-backed folio unmapping.

Processing all PTEs of a large folio in one go helps us batch across atomics (add_mm_counter() etc), barriers in __folio_try_share_anon_rmap(), and repeated calls to page_vma_mapped_walk(). In general, batching helps execute similar code together, making the path more memory and CPU friendly.

On arm64-contpte, batching also helps avoid redundant ptep_get() calls and TLB flushes while breaking the contpte mapping.

The handling of anon-exclusivity is very similar to commit cac1db8c3aad ("mm: optimize mprotect() by PTE batching"). Since folio_unmap_pte_batch() does not look at the bits of the underlying page, process sub-batches of PTEs pointing to pages with the same exclusivity state, and batch set only those PTEs to swap PTEs in one go.

Disable batching for sparc (because of arch_unmap_one), we will batch this later.

Rmap accounting and reference accounting must happen when anon folio unmap succeeds. If a large folio is only partially batched or a later sub-batch fails, account only the pages that were actually unmapped. Put that accounting in __ttu_anon_swapbacked_folio() itself instead of using goto jumps at the try_to_unmap_one() callsite.

Similarly, do the finish_folio_unmap_batch() in ttu_anon_folio() itself for the non-swapbacked lazyfree case.

If the batch length is less than the number of pages in the folio, skip over this batch. page_vma_mapped_walk() handles this: check_pte() returns true only if any of [pvmw->pfn, pvmw->pfn + nr_pages) is mapped by the PTE. Swap PTEs have no underlying PFN, so check_pte() returns false until the walk reaches the next present PTE to unmap.

Remove the finish_unmap label since no goto callers are left now.

---
 mm/rmap.c | 109 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 29 deletions(-)
mm/rmap.c +80 -29
index fa9fc8374fc26..cdda9efd80bc3 100644
@@ -1966,12 +1966,13 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
 	end_addr = pmd_addr_end(addr, vma->vm_end);
 	max_nr = (end_addr - addr) >> PAGE_SHIFT;
 
-	/* We only support lazyfree or file folios batching for now ... */
-	if (folio_test_anon(folio) && folio_test_swapbacked(folio))
+	if (pte_unused(pte))
 		return 1;
 
-	if (pte_unused(pte))
+#ifdef __HAVE_ARCH_UNMAP_ONE
+	if (folio_test_anon(folio) && folio_test_swapbacked(folio))
 		return 1;
+#endif
 
 	/*
 	 * If unmap fails, we need to restore the ptes. To avoid accidentally
@@ -2141,16 +2142,25 @@ static pte_t swp_pte_prepare(swp_entry_t entry, pte_t old_pte,
 	return swp_pte;
 }
 
-static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+static void finish_folio_unmap_batch(struct vm_area_struct *vma,
+		struct folio *folio, struct page *page, unsigned long nr_pages)
+{
+	folio_remove_rmap_ptes(folio, page, nr_pages, vma);
+	if (vma->vm_flags & VM_LOCKED)
+		mlock_drain_local();
+	folio_put_refs(folio, nr_pages);
+}
+
+static bool __ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
 		struct folio *folio, struct page *page, unsigned long address,
-		pte_t *ptep, pte_t pteval)
+		pte_t *ptep, pte_t pteval, unsigned long nr_pages,
+		bool anon_exclusive)
 {
-	const bool anon_exclusive = folio_test_anon(folio) &&
-				    PageAnonExclusive(page);
 	swp_entry_t entry = folio_page_swap_entry(folio, page);
 	struct mm_struct *mm = vma->vm_mm;
+	pte_t swp_pte;
 
-	if (folio_dup_swap_pages(folio, page, 1) < 0)
+	if (folio_dup_swap_pages(folio, page, nr_pages) < 0)
 		return false;
 
 	/*
@@ -2159,21 +2169,57 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
 	 * so we'll not check/care.
 	 */
 	if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-		folio_put_swap_pages(folio, page, 1);
+		VM_WARN_ON(nr_pages != 1);
+		folio_put_swap_pages(folio, page, nr_pages);
 		return false;
 	}
 
 	/* See folio_try_share_anon_rmap(): clear PTE first. */
-	if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
-		folio_put_swap_pages(folio, page, 1);
+	if (anon_exclusive &&
+	    folio_try_share_anon_rmap_ptes(folio, page, nr_pages)) {
+		folio_put_swap_pages(folio, page, nr_pages);
 		return false;
 	}
 
 	mm_prepare_for_swap_entries(mm);
-	dec_mm_counter(mm, MM_ANONPAGES);
-	inc_mm_counter(mm, MM_SWAPENTS);
-	set_pte_at(mm, address, ptep,
-		   swp_pte_prepare(entry, pteval, anon_exclusive));
+	add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
+	add_mm_counter(mm, MM_SWAPENTS, nr_pages);
+	swp_pte = swp_pte_prepare(entry, pteval, anon_exclusive);
+	set_softleaf_ptes(mm, address, ptep, swp_pte, nr_pages);
+	finish_folio_unmap_batch(vma, folio, page, nr_pages);
+	return true;
+}
+
+static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+		struct folio *folio, struct page *first_page,
+		unsigned long address, pte_t *ptep, pte_t pteval,
+		unsigned long nr_pages)
+{
+	unsigned long batch_idx = 0;
+
+	while (nr_pages) {
+		bool anon_exclusive = PageAnonExclusive(first_page + batch_idx);
+		unsigned long len = page_anon_exclusive_batch(batch_idx,
+				nr_pages, first_page, anon_exclusive);
+
+		if (!__ttu_anon_swapbacked_folio(vma, folio,
+				first_page + batch_idx, address, ptep, pteval,
+				len, anon_exclusive)) {
+			/* Restore the remaining PTEs that were cleared. */
+			set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+			return false;
+		}
+
+		nr_pages -= len;
+		if (!nr_pages)
+			break;
+
+		pteval = pte_advance_pfn(pteval, len);
+		address += len * PAGE_SIZE;
+		batch_idx += len;
+		ptep += len;
+	}
+
 	return true;
 }
 
@@ -2186,15 +2232,22 @@ static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
 	 * See handle_pte_fault() ...
 	 */
 	if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
-			 folio_test_swapcache(folio)))
+			 folio_test_swapcache(folio))) {
+		set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
 		return false;
+	}
 
-	if (!folio_test_swapbacked(folio))
-		return ttu_anon_lazyfree_folio(vma, folio, nr_pages);
+	if (!folio_test_swapbacked(folio)) {
+		if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
+			set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+			return false;
+		}
+		finish_folio_unmap_batch(vma, folio, page, nr_pages);
+		return true;
+	}
 
-	/* nr_pages > 1 not supported yet */
 	return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
-					 pteval);
+					 pteval, nr_pages);
 }
 
 /*
@@ -2374,13 +2427,14 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 			 */
 			dec_mm_counter(mm, mm_counter(folio));
 		} else if (folio_test_anon(folio)) {
+			/* finish_folio_unmap_batch handled internally */
 			if (!ttu_anon_folio(vma, folio, page, address,
-					    pvmw.pte, pteval, nr_pages)) {
-				set_ptes(mm, address, pvmw.pte, pteval, nr_pages);
+					    pvmw.pte, pteval, nr_pages))
 				goto walk_abort;
-			}
 
-			goto finish_unmap;
+			if (nr_pages == folio_nr_pages(folio))
+				goto walk_done;
+			continue;
 		} else {
 			/*
 			 * This is a locked file-backed folio,
@@ -2395,11 +2449,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
 			 */
 			add_mm_counter(mm, mm_counter_file(folio), -nr_pages);
 		}
-finish_unmap:
-		folio_remove_rmap_ptes(folio, page, nr_pages, vma);
-		if (vma->vm_flags & VM_LOCKED)
-			mlock_drain_local();
-		folio_put_refs(folio, nr_pages);
+
+		finish_folio_unmap_batch(vma, folio, page, nr_pages);
 
 		/*
 		 * If we are sure that we batched the entire folio and cleared
Signature · 1 line
2.43.0
reviewer
#16 lore raw

Re: [PATCH v3 8/9] mm/rmap: batch unmap anonymous swap-backed large folios

Quoted 171 lines · Dev Jain

Enable batch clearing of PTEs and batch swap setting of PTEs for anon swap-backed folio unmapping.

Processing all PTEs of a large folio in one go helps us batch across atomics (add_mm_counter() etc), barriers in __folio_try_share_anon_rmap(), and repeated calls to page_vma_mapped_walk(). In general, batching helps execute similar code together, making the path more memory and CPU friendly.

On arm64-contpte, batching also helps avoid redundant ptep_get() calls and TLB flushes while breaking the contpte mapping.

The handling of anon-exclusivity is very similar to commit cac1db8c3aad ("mm: optimize mprotect() by PTE batching"). Since folio_unmap_pte_batch() does not look at the bits of the underlying page, process sub-batches of PTEs pointing to pages with the same exclusivity state, and batch set only those PTEs to swap PTEs in one go.

Disable batching for sparc (because of arch_unmap_one), we will batch this later.

Rmap accounting and reference accounting must happen when anon folio unmap succeeds. If a large folio is only partially batched or a later sub-batch fails, account only the pages that were actually unmapped. Put that accounting in __ttu_anon_swapbacked_folio() itself instead of using goto jumps at the try_to_unmap_one() callsite.

Similarly, do the finish_folio_unmap_batch() in ttu_anon_folio() itself for the non-swapbacked lazyfree case.

If the batch length is less than the number of pages in the folio, skip over this batch. page_vma_mapped_walk() handles this: check_pte() returns true only if any of [pvmw->pfn, pvmw->pfn + nr_pages) is mapped by the PTE. Swap PTEs have no underlying PFN, so check_pte() returns false until the walk reaches the next present PTE to unmap.

Remove the finish_unmap label since no goto callers are left now.

---
 mm/rmap.c | 109 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 29 deletions(-)
mm/rmap.c +64 -16
index fa9fc8374fc26..cdda9efd80bc3 100644
@@ -1966,12 +1966,13 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
        end_addr = pmd_addr_end(addr, vma->vm_end);
        max_nr = (end_addr - addr) >> PAGE_SHIFT;
 
-       /* We only support lazyfree or file folios batching for now ... */
-       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
+       if (pte_unused(pte))
                return 1;
 
-       if (pte_unused(pte))
+#ifdef __HAVE_ARCH_UNMAP_ONE
+       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
                return 1;
+#endif
 
        /*
         * If unmap fails, we need to restore the ptes. To avoid accidentally
@@ -2141,16 +2142,25 @@ static pte_t swp_pte_prepare(swp_entry_t entry, pte_t old_pte,
        return swp_pte;
 }
 
-static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+static void finish_folio_unmap_batch(struct vm_area_struct *vma,
+               struct folio *folio, struct page *page, unsigned long nr_pages)
+{
+       folio_remove_rmap_ptes(folio, page, nr_pages, vma);
+       if (vma->vm_flags & VM_LOCKED)
+               mlock_drain_local();
+       folio_put_refs(folio, nr_pages);
+}
+
+static bool __ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
                struct folio *folio, struct page *page, unsigned long address,
-               pte_t *ptep, pte_t pteval)
+               pte_t *ptep, pte_t pteval, unsigned long nr_pages,
+               bool anon_exclusive)
 {
-       const bool anon_exclusive = folio_test_anon(folio) &&
-                                   PageAnonExclusive(page);
        swp_entry_t entry = folio_page_swap_entry(folio, page);
        struct mm_struct *mm = vma->vm_mm;
+       pte_t swp_pte;
 
-       if (folio_dup_swap_pages(folio, page, 1) < 0)
+       if (folio_dup_swap_pages(folio, page, nr_pages) < 0)
                return false;
 
        /*
@@ -2159,21 +2169,57 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
         * so we'll not check/care.
         */
        if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-               folio_put_swap_pages(folio, page, 1);
+               VM_WARN_ON(nr_pages != 1);
+               folio_put_swap_pages(folio, page, nr_pages);
                return false;
        }
 
        /* See folio_try_share_anon_rmap(): clear PTE first. */
-       if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
-               folio_put_swap_pages(folio, page, 1);
+       if (anon_exclusive &&
+           folio_try_share_anon_rmap_ptes(folio, page, nr_pages)) {
+               folio_put_swap_pages(folio, page, nr_pages);
                return false;
        }
 
        mm_prepare_for_swap_entries(mm);
-       dec_mm_counter(mm, MM_ANONPAGES);
-       inc_mm_counter(mm, MM_SWAPENTS);
-       set_pte_at(mm, address, ptep,
-                  swp_pte_prepare(entry, pteval, anon_exclusive));
+       add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
+       add_mm_counter(mm, MM_SWAPENTS, nr_pages);
+       swp_pte = swp_pte_prepare(entry, pteval, anon_exclusive);
+       set_softleaf_ptes(mm, address, ptep, swp_pte, nr_pages);
+       finish_folio_unmap_batch(vma, folio, page, nr_pages);
+       return true;
+}
+
+static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+               struct folio *folio, struct page *first_page,
+               unsigned long address, pte_t *ptep, pte_t pteval,
+               unsigned long nr_pages)
+{
+       unsigned long batch_idx = 0;
+
+       while (nr_pages) {
+               bool anon_exclusive = PageAnonExclusive(first_page + batch_idx);
+               unsigned long len = page_anon_exclusive_batch(batch_idx,
+                               nr_pages, first_page, anon_exclusive);
+
+               if (!__ttu_anon_swapbacked_folio(vma, folio,
+                               first_page + batch_idx, address, ptep, pteval,
+                               len, anon_exclusive)) {
+                       /* Restore the remaining PTEs that were cleared. */
+                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+                       return false;
+               }
+
+               nr_pages -= len;
+               if (!nr_pages)
+                       break;
+
+               pteval = pte_advance_pfn(pteval, len);
+               address += len * PAGE_SIZE;
+               batch_idx += len;
+               ptep += len;
+       }
+
        return true;
 }
 
@@ -2186,15 +2232,22 @@ static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
         * See handle_pte_fault() ...
         */
        if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
-                        folio_test_swapcache(folio)))
+                        folio_test_swapcache(folio))) {
+               set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                return false;
+       }
 

mm/rmap.c static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio, · line 2247

Quoted 10 lines
mm/rmap.c +8 -2
-       if (!folio_test_swapbacked(folio))
-               return ttu_anon_lazyfree_folio(vma, folio, nr_pages);
+       if (!folio_test_swapbacked(folio)) {
+               if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
+                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+                       return false;
+               }
+               finish_folio_unmap_batch(vma, folio, page, nr_pages);
+               return true;
+       }

Hi Dev,

if we look at the whole ttu_anon_folio() code:

static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
struct page *page, unsigned long address, pte_t *ptep, pte_t pteval, unsigned long nr_pages)

{
        /*
         * Store the swap location in the pte.
         * See handle_pte_fault() ...
         */
        if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
                         folio_test_swapcache(folio))) {
                set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                return false;
        }

        if (!folio_test_swapbacked(folio)) {
                if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
                        set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                        return false;
                }
                finish_folio_unmap_batch(vma, folio, page, nr_pages);
                return true;
        }

        return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
                                         pteval, nr_pages);
}

The comment "Store the swap location in the pte" is really out of place. Since we are already here, can we move it to the right place?

mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2430

Quoted 13 lines
mm/rmap.c +2 -2
-       /* nr_pages > 1 not supported yet */
        return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
-                                        pteval);
+                                        pteval, nr_pages);
 }
 
 /*
@@ -2374,13 +2427,14 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
                         */
                        dec_mm_counter(mm, mm_counter(folio));
                } else if (folio_test_anon(folio)) {
+                       /* finish_folio_unmap_batch handled internally */

let me paste the final code here:

                } else if (folio_test_anon(folio)) {
                        /* finish_folio_unmap_batch handled internally */
                        if (!ttu_anon_folio(vma, folio, page, address,
                                            pvmw.pte, pteval, nr_pages))
                                goto walk_abort;

                        if (nr_pages == folio_nr_pages(folio))
                                goto walk_done;
                        continue;
                } else {
                        ...
                }

                finish_folio_unmap_batch(vma, folio, page, nr_pages);

                /*
                 * If we are sure that we batched the entire folio and cleared
                 * all PTEs, we can just optimize and stop right here.
                 */
                if (nr_pages == folio_nr_pages(folio))
                        goto walk_done;
                continue;

I am really struggling to read the code here. For anon folios, we handle finish_folio_unmap_batch() internally, while for other folios we still do it outside. This feels a bit inconsistent.

Also, we are duplicating the code below twice:

                if (nr_pages == folio_nr_pages(folio))
                        goto walk_done;

Can't we just do finish_folio_unmap_batch() like the other cases?

That would also avoid the duplication.

Best Regards
Barry

author
#17 lore raw

Re: [PATCH v3 8/9] mm/rmap: batch unmap anonymous swap-backed large folios

Quoted 214 lines · Barry Song
Quoted 171 lines · Dev Jain

Enable batch clearing of PTEs and batch swap setting of PTEs for anon swap-backed folio unmapping.

Processing all PTEs of a large folio in one go helps us batch across atomics (add_mm_counter() etc), barriers in __folio_try_share_anon_rmap(), and repeated calls to page_vma_mapped_walk(). In general, batching helps execute similar code together, making the path more memory and CPU friendly.

On arm64-contpte, batching also helps avoid redundant ptep_get() calls and TLB flushes while breaking the contpte mapping.

The handling of anon-exclusivity is very similar to commit cac1db8c3aad ("mm: optimize mprotect() by PTE batching"). Since folio_unmap_pte_batch() does not look at the bits of the underlying page, process sub-batches of PTEs pointing to pages with the same exclusivity state, and batch set only those PTEs to swap PTEs in one go.

Disable batching for sparc (because of arch_unmap_one), we will batch this later.

Rmap accounting and reference accounting must happen when anon folio unmap succeeds. If a large folio is only partially batched or a later sub-batch fails, account only the pages that were actually unmapped. Put that accounting in __ttu_anon_swapbacked_folio() itself instead of using goto jumps at the try_to_unmap_one() callsite.

Similarly, do the finish_folio_unmap_batch() in ttu_anon_folio() itself for the non-swapbacked lazyfree case.

If the batch length is less than the number of pages in the folio, skip over this batch. page_vma_mapped_walk() handles this: check_pte() returns true only if any of [pvmw->pfn, pvmw->pfn + nr_pages) is mapped by the PTE. Swap PTEs have no underlying PFN, so check_pte() returns false until the walk reaches the next present PTE to unmap.

Remove the finish_unmap label since no goto callers are left now.

---
 mm/rmap.c | 109 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 29 deletions(-)
mm/rmap.c +64 -16
index fa9fc8374fc26..cdda9efd80bc3 100644
@@ -1966,12 +1966,13 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
        end_addr = pmd_addr_end(addr, vma->vm_end);
        max_nr = (end_addr - addr) >> PAGE_SHIFT;
 
-       /* We only support lazyfree or file folios batching for now ... */
-       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
+       if (pte_unused(pte))
                return 1;
 
-       if (pte_unused(pte))
+#ifdef __HAVE_ARCH_UNMAP_ONE
+       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
                return 1;
+#endif
 
        /*
         * If unmap fails, we need to restore the ptes. To avoid accidentally
@@ -2141,16 +2142,25 @@ static pte_t swp_pte_prepare(swp_entry_t entry, pte_t old_pte,
        return swp_pte;
 }
 
-static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+static void finish_folio_unmap_batch(struct vm_area_struct *vma,
+               struct folio *folio, struct page *page, unsigned long nr_pages)
+{
+       folio_remove_rmap_ptes(folio, page, nr_pages, vma);
+       if (vma->vm_flags & VM_LOCKED)
+               mlock_drain_local();
+       folio_put_refs(folio, nr_pages);
+}
+
+static bool __ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
                struct folio *folio, struct page *page, unsigned long address,
-               pte_t *ptep, pte_t pteval)
+               pte_t *ptep, pte_t pteval, unsigned long nr_pages,
+               bool anon_exclusive)
 {
-       const bool anon_exclusive = folio_test_anon(folio) &&
-                                   PageAnonExclusive(page);
        swp_entry_t entry = folio_page_swap_entry(folio, page);
        struct mm_struct *mm = vma->vm_mm;
+       pte_t swp_pte;
 
-       if (folio_dup_swap_pages(folio, page, 1) < 0)
+       if (folio_dup_swap_pages(folio, page, nr_pages) < 0)
                return false;
 
        /*
@@ -2159,21 +2169,57 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
         * so we'll not check/care.
         */
        if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-               folio_put_swap_pages(folio, page, 1);
+               VM_WARN_ON(nr_pages != 1);
+               folio_put_swap_pages(folio, page, nr_pages);
                return false;
        }
 
        /* See folio_try_share_anon_rmap(): clear PTE first. */
-       if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
-               folio_put_swap_pages(folio, page, 1);
+       if (anon_exclusive &&
+           folio_try_share_anon_rmap_ptes(folio, page, nr_pages)) {
+               folio_put_swap_pages(folio, page, nr_pages);
                return false;
        }
 
        mm_prepare_for_swap_entries(mm);
-       dec_mm_counter(mm, MM_ANONPAGES);
-       inc_mm_counter(mm, MM_SWAPENTS);
-       set_pte_at(mm, address, ptep,
-                  swp_pte_prepare(entry, pteval, anon_exclusive));
+       add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
+       add_mm_counter(mm, MM_SWAPENTS, nr_pages);
+       swp_pte = swp_pte_prepare(entry, pteval, anon_exclusive);
+       set_softleaf_ptes(mm, address, ptep, swp_pte, nr_pages);
+       finish_folio_unmap_batch(vma, folio, page, nr_pages);
+       return true;
+}
+
+static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+               struct folio *folio, struct page *first_page,
+               unsigned long address, pte_t *ptep, pte_t pteval,
+               unsigned long nr_pages)
+{
+       unsigned long batch_idx = 0;
+
+       while (nr_pages) {
+               bool anon_exclusive = PageAnonExclusive(first_page + batch_idx);
+               unsigned long len = page_anon_exclusive_batch(batch_idx,
+                               nr_pages, first_page, anon_exclusive);
+
+               if (!__ttu_anon_swapbacked_folio(vma, folio,
+                               first_page + batch_idx, address, ptep, pteval,
+                               len, anon_exclusive)) {
+                       /* Restore the remaining PTEs that were cleared. */
+                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+                       return false;
+               }
+
+               nr_pages -= len;
+               if (!nr_pages)
+                       break;
+
+               pteval = pte_advance_pfn(pteval, len);
+               address += len * PAGE_SIZE;
+               batch_idx += len;
+               ptep += len;
+       }
+
        return true;
 }
 
@@ -2186,15 +2232,22 @@ static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
         * See handle_pte_fault() ...
         */
        if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
-                        folio_test_swapcache(folio)))
+                        folio_test_swapcache(folio))) {
+               set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                return false;
+       }
 

mm/rmap.c static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio, · line 2247

Quoted 10 lines
mm/rmap.c +8 -2
-       if (!folio_test_swapbacked(folio))
-               return ttu_anon_lazyfree_folio(vma, folio, nr_pages);
+       if (!folio_test_swapbacked(folio)) {
+               if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
+                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+                       return false;
+               }
+               finish_folio_unmap_batch(vma, folio, page, nr_pages);
+               return true;
+       }

Hi Dev,

if we look at the whole ttu_anon_folio() code:

static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
struct page *page, unsigned long address, pte_t *ptep, pte_t pteval, unsigned long nr_pages)

{
        /*
         * Store the swap location in the pte.
         * See handle_pte_fault() ...
         */
        if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
                         folio_test_swapcache(folio))) {
                set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                return false;
        }

        if (!folio_test_swapbacked(folio)) {
                if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
                        set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                        return false;
                }
                finish_folio_unmap_batch(vma, folio, page, nr_pages);
                return true;
        }

        return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
                                         pteval, nr_pages);
}
Quoted 2 lines

The comment "Store the swap location in the pte" is really out of place. Since we are already here, can we move it to the right place?

I am not sure what is the "right place" you are referring to : ) More than that, this comments feels very out of date - it is well-know that the "swap location" is stored in the pte.

In fact, this comment starts on top of the wrong block, which is a debug assertion for "when we are here, swapbacked if and only if swapcache". So I'd rather just remove it.

Quoted 45 lines
Quoted 10 lines
mm/rmap.c +1 -2
-       /* nr_pages > 1 not supported yet */
        return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
-                                        pteval);
+                                        pteval, nr_pages);
 }
 
 /*

mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2430

Quoted 4 lines
mm/rmap.c +1 -0
@@ -2374,13 +2427,14 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
                         */
                        dec_mm_counter(mm, mm_counter(folio));
                } else if (folio_test_anon(folio)) {
+                       /* finish_folio_unmap_batch handled internally */

let me paste the final code here:

                } else if (folio_test_anon(folio)) {
                        /* finish_folio_unmap_batch handled internally */
                        if (!ttu_anon_folio(vma, folio, page, address,
                                            pvmw.pte, pteval, nr_pages))
                                goto walk_abort;

                        if (nr_pages == folio_nr_pages(folio))
                                goto walk_done;
                        continue;
                } else {
                        ...
                }

                finish_folio_unmap_batch(vma, folio, page, nr_pages);

                /*
                 * If we are sure that we batched the entire folio and cleared
                 * all PTEs, we can just optimize and stop right here.
                 */
                if (nr_pages == folio_nr_pages(folio))
                        goto walk_done;
                continue;

I am really struggling to read the code here. For anon folios, we handle finish_folio_unmap_batch() internally, while for other folios we still do it outside. This feels a bit inconsistent.

Quoted 6 lines

Also, we are duplicating the code below twice:

                if (nr_pages == folio_nr_pages(folio))
                        goto walk_done;

Can't we just do finish_folio_unmap_batch() like the other cases?

The core problem is that we may succeed on a partial range.

ttu_anon_swapbacked_folio can succeed on x < nr_pages. So for x, we should call finish_folio_unmap_batch. Either we do it my way, or, ttu_anon_folio returns the number of pages successfully unmapped = x, and then even in case of "failure" (we should now jump to walk_abort since we failed to unmap 'nr_pages - x' pages) we call finish_folio_unmap_batch for x pages.

IMHO that is uglier. I wanted to avoid this label jumping. I would be interested in hearing a better alternate : )

Quoted 5 lines

That would also avoid the duplication.

Best Regards
Barry

author
#18 lore raw

Re: [PATCH v3 8/9] mm/rmap: batch unmap anonymous swap-backed large folios

Quoted 289 lines · Dev Jain
Quoted 214 lines · Barry Song
Quoted 171 lines · Dev Jain

Enable batch clearing of PTEs and batch swap setting of PTEs for anon swap-backed folio unmapping.

Processing all PTEs of a large folio in one go helps us batch across atomics (add_mm_counter() etc), barriers in __folio_try_share_anon_rmap(), and repeated calls to page_vma_mapped_walk(). In general, batching helps execute similar code together, making the path more memory and CPU friendly.

On arm64-contpte, batching also helps avoid redundant ptep_get() calls and TLB flushes while breaking the contpte mapping.

The handling of anon-exclusivity is very similar to commit cac1db8c3aad ("mm: optimize mprotect() by PTE batching"). Since folio_unmap_pte_batch() does not look at the bits of the underlying page, process sub-batches of PTEs pointing to pages with the same exclusivity state, and batch set only those PTEs to swap PTEs in one go.

Disable batching for sparc (because of arch_unmap_one), we will batch this later.

Rmap accounting and reference accounting must happen when anon folio unmap succeeds. If a large folio is only partially batched or a later sub-batch fails, account only the pages that were actually unmapped. Put that accounting in __ttu_anon_swapbacked_folio() itself instead of using goto jumps at the try_to_unmap_one() callsite.

Similarly, do the finish_folio_unmap_batch() in ttu_anon_folio() itself for the non-swapbacked lazyfree case.

If the batch length is less than the number of pages in the folio, skip over this batch. page_vma_mapped_walk() handles this: check_pte() returns true only if any of [pvmw->pfn, pvmw->pfn + nr_pages) is mapped by the PTE. Swap PTEs have no underlying PFN, so check_pte() returns false until the walk reaches the next present PTE to unmap.

Remove the finish_unmap label since no goto callers are left now.

---
 mm/rmap.c | 109 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 29 deletions(-)
mm/rmap.c +64 -16
index fa9fc8374fc26..cdda9efd80bc3 100644
@@ -1966,12 +1966,13 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
        end_addr = pmd_addr_end(addr, vma->vm_end);
        max_nr = (end_addr - addr) >> PAGE_SHIFT;
 
-       /* We only support lazyfree or file folios batching for now ... */
-       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
+       if (pte_unused(pte))
                return 1;
 
-       if (pte_unused(pte))
+#ifdef __HAVE_ARCH_UNMAP_ONE
+       if (folio_test_anon(folio) && folio_test_swapbacked(folio))
                return 1;
+#endif
 
        /*
         * If unmap fails, we need to restore the ptes. To avoid accidentally
@@ -2141,16 +2142,25 @@ static pte_t swp_pte_prepare(swp_entry_t entry, pte_t old_pte,
        return swp_pte;
 }
 
-static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+static void finish_folio_unmap_batch(struct vm_area_struct *vma,
+               struct folio *folio, struct page *page, unsigned long nr_pages)
+{
+       folio_remove_rmap_ptes(folio, page, nr_pages, vma);
+       if (vma->vm_flags & VM_LOCKED)
+               mlock_drain_local();
+       folio_put_refs(folio, nr_pages);
+}
+
+static bool __ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
                struct folio *folio, struct page *page, unsigned long address,
-               pte_t *ptep, pte_t pteval)
+               pte_t *ptep, pte_t pteval, unsigned long nr_pages,
+               bool anon_exclusive)
 {
-       const bool anon_exclusive = folio_test_anon(folio) &&
-                                   PageAnonExclusive(page);
        swp_entry_t entry = folio_page_swap_entry(folio, page);
        struct mm_struct *mm = vma->vm_mm;
+       pte_t swp_pte;
 
-       if (folio_dup_swap_pages(folio, page, 1) < 0)
+       if (folio_dup_swap_pages(folio, page, nr_pages) < 0)
                return false;
 
        /*
@@ -2159,21 +2169,57 @@ static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
         * so we'll not check/care.
         */
        if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-               folio_put_swap_pages(folio, page, 1);
+               VM_WARN_ON(nr_pages != 1);
+               folio_put_swap_pages(folio, page, nr_pages);
                return false;
        }
 
        /* See folio_try_share_anon_rmap(): clear PTE first. */
-       if (anon_exclusive && folio_try_share_anon_rmap_pte(folio, page)) {
-               folio_put_swap_pages(folio, page, 1);
+       if (anon_exclusive &&
+           folio_try_share_anon_rmap_ptes(folio, page, nr_pages)) {
+               folio_put_swap_pages(folio, page, nr_pages);
                return false;
        }
 
        mm_prepare_for_swap_entries(mm);
-       dec_mm_counter(mm, MM_ANONPAGES);
-       inc_mm_counter(mm, MM_SWAPENTS);
-       set_pte_at(mm, address, ptep,
-                  swp_pte_prepare(entry, pteval, anon_exclusive));
+       add_mm_counter(mm, MM_ANONPAGES, -nr_pages);
+       add_mm_counter(mm, MM_SWAPENTS, nr_pages);
+       swp_pte = swp_pte_prepare(entry, pteval, anon_exclusive);
+       set_softleaf_ptes(mm, address, ptep, swp_pte, nr_pages);
+       finish_folio_unmap_batch(vma, folio, page, nr_pages);
+       return true;
+}
+
+static bool ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
+               struct folio *folio, struct page *first_page,
+               unsigned long address, pte_t *ptep, pte_t pteval,
+               unsigned long nr_pages)
+{
+       unsigned long batch_idx = 0;
+
+       while (nr_pages) {
+               bool anon_exclusive = PageAnonExclusive(first_page + batch_idx);
+               unsigned long len = page_anon_exclusive_batch(batch_idx,
+                               nr_pages, first_page, anon_exclusive);
+
+               if (!__ttu_anon_swapbacked_folio(vma, folio,
+                               first_page + batch_idx, address, ptep, pteval,
+                               len, anon_exclusive)) {
+                       /* Restore the remaining PTEs that were cleared. */
+                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+                       return false;
+               }
+
+               nr_pages -= len;
+               if (!nr_pages)
+                       break;
+
+               pteval = pte_advance_pfn(pteval, len);
+               address += len * PAGE_SIZE;
+               batch_idx += len;
+               ptep += len;
+       }
+
        return true;
 }
 
@@ -2186,15 +2232,22 @@ static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
         * See handle_pte_fault() ...
         */
        if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
-                        folio_test_swapcache(folio)))
+                        folio_test_swapcache(folio))) {
+               set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                return false;
+       }
 

mm/rmap.c static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio, · line 2247

Quoted 10 lines
mm/rmap.c +8 -2
-       if (!folio_test_swapbacked(folio))
-               return ttu_anon_lazyfree_folio(vma, folio, nr_pages);
+       if (!folio_test_swapbacked(folio)) {
+               if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
+                       set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
+                       return false;
+               }
+               finish_folio_unmap_batch(vma, folio, page, nr_pages);
+               return true;
+       }

Hi Dev,

if we look at the whole ttu_anon_folio() code:

static bool ttu_anon_folio(struct vm_area_struct *vma, struct folio *folio,
struct page *page, unsigned long address, pte_t *ptep, pte_t pteval, unsigned long nr_pages)

{
        /*
         * Store the swap location in the pte.
         * See handle_pte_fault() ...
         */
        if (WARN_ON_ONCE(folio_test_swapbacked(folio) !=
                         folio_test_swapcache(folio))) {
                set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                return false;
        }

        if (!folio_test_swapbacked(folio)) {
                if (!ttu_anon_lazyfree_folio(vma, folio, nr_pages)) {
                        set_ptes(vma->vm_mm, address, ptep, pteval, nr_pages);
                        return false;
                }
                finish_folio_unmap_batch(vma, folio, page, nr_pages);
                return true;
        }

        return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
                                         pteval, nr_pages);
}
Quoted 2 lines

The comment "Store the swap location in the pte" is really out of place. Since we are already here, can we move it to the right place?

I am not sure what is the "right place" you are referring to : ) More than that, this comments feels very out of date - it is well-know that the "swap location" is stored in the pte.

In fact, this comment starts on top of the wrong block, which is a debug assertion for "when we are here, swapbacked if and only if swapcache". So I'd rather just remove it.

Quoted 45 lines
Quoted 10 lines
mm/rmap.c +1 -2
-       /* nr_pages > 1 not supported yet */
        return ttu_anon_swapbacked_folio(vma, folio, page, address, ptep,
-                                        pteval);
+                                        pteval, nr_pages);
 }
 
 /*

mm/rmap.c static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma, · line 2430

Quoted 4 lines
mm/rmap.c +1 -0
@@ -2374,13 +2427,14 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
                         */
                        dec_mm_counter(mm, mm_counter(folio));
                } else if (folio_test_anon(folio)) {
+                       /* finish_folio_unmap_batch handled internally */

let me paste the final code here:

                } else if (folio_test_anon(folio)) {
                        /* finish_folio_unmap_batch handled internally */
                        if (!ttu_anon_folio(vma, folio, page, address,
                                            pvmw.pte, pteval, nr_pages))
                                goto walk_abort;

                        if (nr_pages == folio_nr_pages(folio))
                                goto walk_done;
                        continue;
                } else {
                        ...
                }

                finish_folio_unmap_batch(vma, folio, page, nr_pages);

                /*
                 * If we are sure that we batched the entire folio and cleared
                 * all PTEs, we can just optimize and stop right here.
                 */
                if (nr_pages == folio_nr_pages(folio))
                        goto walk_done;
                continue;

I am really struggling to read the code here. For anon folios, we handle finish_folio_unmap_batch() internally, while for other folios we still do it outside. This feels a bit inconsistent.

Quoted 6 lines

Also, we are duplicating the code below twice:

                if (nr_pages == folio_nr_pages(folio))
                        goto walk_done;

Can't we just do finish_folio_unmap_batch() like the other cases?

The core problem is that we may succeed on a partial range.

ttu_anon_swapbacked_folio can succeed on x < nr_pages. So for x, we should call finish_folio_unmap_batch. Either we do it my way, or, ttu_anon_folio returns the number of pages successfully unmapped = x, and then even in case of "failure" (we should now jump to walk_abort since we failed to unmap 'nr_pages - x' pages) we call finish_folio_unmap_batch for x pages.

Quoted 2 lines

IMHO that is uglier. I wanted to avoid this label jumping. I would be interested in hearing a better alternate : )

I did try this and it looks nicer actually. Will shift to this in v4, thanks!

Quoted 6 lines
Quoted 5 lines

That would also avoid the duplication.

Best Regards
Barry

author
#19 lore raw

PATCH v3 9/9 mm, sparc: batch arch_unmap_one()

In reply to Dev Jain

The anonymous large-folio unmap batching path can clear and replace multiple PTEs at once, but architectures like sparc need arch_unmap_one() to preserve per-page metadata before swap entries are installed. sparc uses this hook to save ADI tags.

Extend arch_unmap_one() to operate on a pte range.

sparc uses the count to walk the PTE range and save ADI tags for entries carrying _PAGE_MCD_4V. There is no reverse operation for a partially saved range: saving ADI tags only records a copy for later restore, so if a later entry fails and the core keeps the original PTEs, there is no architectural state to unsave.

---
 arch/sparc/include/asm/pgtable_64.h | 10 +++++++---
 arch/sparc/kernel/adi_64.c          | 21 +++++++++++++++++++++
 include/linux/pgtable.h             |  6 +++---
 mm/rmap.c                           | 11 +++--------
 4 files changed, 34 insertions(+), 14 deletions(-)
arch/sparc/include/asm/pgtable_64.h +7 -3
index 44d1333065a6a..f7dd1872884e4 100644
@@ -1037,6 +1037,9 @@ void adi_restore_tags(struct mm_struct *mm, struct vm_area_struct *vma,
 int adi_save_tags(struct mm_struct *mm, struct vm_area_struct *vma,
 		  unsigned long addr, pte_t oldpte);
 
+int adi_save_tags_range(struct mm_struct *mm, struct vm_area_struct *vma,
+			unsigned long addr, pte_t oldpte, unsigned long nr);
+
 #define __HAVE_ARCH_DO_SWAP_PAGE
 static inline void arch_do_swap_page(struct mm_struct *mm,
 				     struct vm_area_struct *vma,
@@ -1057,10 +1060,11 @@ static inline void arch_do_swap_page(struct mm_struct *mm,
 #define __HAVE_ARCH_UNMAP_ONE
 static inline int arch_unmap_one(struct mm_struct *mm,
 				 struct vm_area_struct *vma,
-				 unsigned long addr, pte_t oldpte)
+				 unsigned long addr, pte_t oldpte,
+				 unsigned long nr)
 {
-	if (adi_state.enabled && (pte_val(oldpte) & _PAGE_MCD_4V))
-		return adi_save_tags(mm, vma, addr, oldpte);
+	if (adi_state.enabled)
+		return adi_save_tags_range(mm, vma, addr, oldpte, nr);
 	return 0;
 }
 
arch/sparc/kernel/adi_64.c +21 -0
index 18036a43cf568..ffa378accefe3 100644
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/mm_types.h>
+#include <linux/pgtable.h>
 #include <asm/mdesc.h>
 #include <asm/adi_64.h>
 #include <asm/mmu_64.h>
@@ -394,3 +395,23 @@ int adi_save_tags(struct mm_struct *mm, struct vm_area_struct *vma,
 
 	return 0;
 }
+
+int adi_save_tags_range(struct mm_struct *mm, struct vm_area_struct *vma,
+			unsigned long addr, pte_t oldpte, unsigned long nr)
+{
+	unsigned long i;
+
+	for (i = 0; i < nr; i++, addr += PAGE_SIZE,
+	     oldpte = pte_next_pfn(oldpte)) {
+		int ret;
+
+		if (!(pte_val(oldpte) & _PAGE_MCD_4V))
+			continue;
+
+		ret = adi_save_tags(mm, vma, addr, oldpte);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
include/linux/pgtable.h +3 -3
index e3c8ab96941c5..d5c78dc938760 100644
@@ -1431,12 +1431,12 @@ static inline void arch_do_swap_page_nr(struct mm_struct *mm,
  * restored when the page is swapped back in. SPARC M7 and newer
  * processors support an ADI (Application Data Integrity) tag for the
  * page as metadata for the page. arch_unmap_one() can save this
- * metadata on a swap-out of a page.
+ * metadata on swap-out of one or more pages.
  */
 static inline int arch_unmap_one(struct mm_struct *mm,
 				  struct vm_area_struct *vma,
-				  unsigned long addr,
-				  pte_t orig_pte)
+				  unsigned long addr, pte_t orig_pte,
+				  unsigned long nr)
 {
 	return 0;
 }
mm/rmap.c +3 -8
index cdda9efd80bc3..774ddc75194b1 100644
@@ -1959,6 +1959,7 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
 
 	if (flags & TTU_HWPOISON)
 		return 1;
+
 	if (!folio_test_large(folio))
 		return 1;
 
@@ -1969,11 +1970,6 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
 	if (pte_unused(pte))
 		return 1;
 
-#ifdef __HAVE_ARCH_UNMAP_ONE
-	if (folio_test_anon(folio) && folio_test_swapbacked(folio))
-		return 1;
-#endif
-
 	/*
 	 * If unmap fails, we need to restore the ptes. To avoid accidentally
 	 * upgrading write permissions for ptes that were not originally
@@ -2168,8 +2164,7 @@ static bool __ttu_anon_swapbacked_folio(struct vm_area_struct *vma,
 	 * architectures where we could have PFN swap PTEs,
 	 * so we'll not check/care.
 	 */
-	if (arch_unmap_one(mm, vma, address, pteval) < 0) {
-		VM_WARN_ON(nr_pages != 1);
+	if (arch_unmap_one(mm, vma, address, pteval, nr_pages) < 0) {
 		folio_put_swap_pages(folio, page, nr_pages);
 		return false;
 	}
@@ -2746,7 +2741,7 @@ static bool try_to_migrate_one(struct folio *folio, struct vm_area_struct *vma,
 			 * architectures where we could have PFN swap PTEs,
 			 * so we'll not check/care.
 			 */
-			if (arch_unmap_one(mm, vma, address, pteval) < 0) {
+			if (arch_unmap_one(mm, vma, address, pteval, 1) < 0) {
 				if (folio_test_hugetlb(folio))
 					set_huge_pte_at(mm, address, pvmw.pte,
 							pteval, hsz);
Signature · 1 line
2.43.0
#20 lore raw
In reply to Dev Jain
Quoted 68 lines · Dev Jain

Speed up unmapping of anonymous swapbacked large folios by clearing the ptes, and setting swap ptes, in one go.

The following benchmark (stolen from Barry) is used to measure the time taken to swapout 256M worth of memory backed by 64K large folios:

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <errno.h>

#define SIZE_MB 256
#define SIZE_BYTES (SIZE_MB * 1024 * 1024)

 int main() {
     void *addr = mmap(NULL, SIZE_BYTES, PROT_READ | PROT_WRITE,
                       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
     if (addr == MAP_FAILED) {
         perror("mmap failed");
         return 1;
     }

     memset(addr, 0, SIZE_BYTES);

     struct timespec start, end;
     clock_gettime(CLOCK_MONOTONIC, &start);

     if (madvise(addr, SIZE_BYTES, MADV_PAGEOUT) != 0) {
         perror("madvise(MADV_PAGEOUT) failed");
         munmap(addr, SIZE_BYTES);
         return 1;
     }

     clock_gettime(CLOCK_MONOTONIC, &end);

     long duration_ns = (end.tv_sec - start.tv_sec) * 1e9 +
                        (end.tv_nsec - start.tv_nsec);
     printf("madvise(MADV_PAGEOUT) took %ld ns (%.3f ms)\n",
            duration_ns, duration_ns / 1e6);

     munmap(addr, SIZE_BYTES);
     return 0;

}

Performance as measured on a Linux VM on Apple M3 (arm64):

Vanilla - Mean: 37401913 ns, std dev: 12%
Patched - Mean: 17420282 ns, std dev: 11%

resulting in more than 2x speedup.

No regression observed on 4K folios.

Performance as measured on bare metal x86:

Vanilla - mean: 54986286 ns, std dev: 1.5%
Patched - mean: 51930795 ns, std dev: 3%

I tried magnifying the difference on x86 by using 1M large folios, but can't spot an obvious improvement (looks like my system is too fast to benefit from batched atomic operations!), hinting that the benefit lies mainly in the reduction of ptep_get() calls and the reduction of TLB flushes during contpte-unfolding, on arm64.

Quoted 4 lines

No regression is observed on 4K folios on x86 too.

---
Applies on mm-unstable.

Given that we are approaching rc6 and are flooded with stuff, this will target 7.5.

I have this on my todo list.

Signature · 3 lines
Cheers,

David