[RFC PATCH v1] mm/madvise: prefetch private file COW swap entries

[zhaozhengzhuo]

Hi Mike,

Thanks for the report and for offering to let me take a first pass at the
fix.  I reproduced the behaviour and prepared the RFC below.  I would
appreciate feedback from you and the mm maintainers on both the scope and the
locking model before I prepare a non-RFC revision.

MADV_WILLNEED currently uses file readahead whenever a VMA has vm_file.  For
a MAP_PRIVATE file mapping, however, a write fault creates anonymous COW
folios whose swap entries live only in the PTE.  File readahead cannot
prefetch those modified contents, so the pages are still read synchronously
when the application later touches them.

This RFC walks PTEs for private file VMAs that have an anon_vma and schedules
reads for their swap entries before continuing with the existing file or
shmem path.  The anon_vma check avoids an additional walk for private file
mappings that have never taken a COW fault.  The existing shmem XArray path
is retained; private shmem COW PTEs are covered by the same walk.

The change also avoids draining the current CPU's LRU pagevec when the new
PTE walk did not find a folio.  The existing shmem path is otherwise left
unchanged in this RFC.

One detail from testing is worth calling out.  Mike's original reproducer
uses a file under /tmp, and /tmp is tmpfs (shmem) on the test VM.  I initially
tested a version that excluded shmem, but that version did not fix the
original reproducer: after a private shmem write fault, the modified
anonymous COW page has a swap entry only in the PTE, while the original
shmem page's swap entry is tracked in the shmem XArray.  Consequently, the
PTE walk is needed for private shmem COW pages even though
shmem_swapin_range() must remain for pages tracked by the shmem XArray.  The
RFC therefore covers private shmem as well as regular file mappings, while
leaving the existing shmem path in place.

Test setup
----------

I tested on x86-64 with the mm-unstable tree at e3b5239afe1b, using the
following workload:

  * 64 MiB MAP_PRIVATE mapping;
  * write one byte in every page to create private COW folios;
  * MADV_PAGEOUT, followed by approximately 1 GiB of memory pressure;
  * MADV_WILLNEED, then a page-by-page read of the mapping;
  * 2.3 GiB VM RAM and a 2.5 GiB disk-backed swap device.

The reproducer is based on Mike's original program.  I changed the backing
file to a regular XFS file for the first comparison, and added timing and
pswpin counters.  I also ran the same workload with a memfd to exercise the
private-shmem case.  The complete source is not repeated here because it is
already present in the original thread.

Results
-------

For one representative regular-file run, the counters and wall-clock times
were:

                         MADV_WILLNEED       subsequent touch
  ---------------------  -----------------  -----------------
  baseline pswpin        +2,578             +16,718
  RFC v1 pswpin          +17,610            +0
  baseline time          16.5 ms            752 ms
  RFC v1 time            274 ms             55 ms

The exact values vary with swap pressure and asynchronous I/O completion.
Across additional runs, the RFC version prefetched approximately 17,000 to
19,000 pages during MADV_WILLNEED, while the subsequent touch usually
performed no additional swap-in.

The private-shmem test also moved the majority of the COW swap-in into the
MADV_WILLNEED phase, while retaining shmem_swapin_range() for pages tracked
only by the shmem XArray.

For mappings with no swap entries, repeated advice calls showed no measurable
extra cost for private-none or private-clean mappings because of the
anon_vma guard.  A private-COW mapping paid for the additional PTE walk; the
observed extra wall time was roughly 20 us for 64 MiB, 0.1--0.2 ms for
256 MiB, and 0.5--0.6 ms for 1 GiB on this VM.

On an x86-64 VM with a 64 MiB mapping, 2.3 GiB RAM and 2.5 GiB swap, the
unpatched kernel read 2,578 pages during MADV_WILLNEED and 16,718 pages on
the subsequent touch.  This version read 17,610 pages during MADV_WILLNEED
and none on the touch in the corresponding run.  The advice/touch wall time
was 16.5/752 ms before and 274/55 ms after; values vary with swap pressure.
The same test with a memfd (private shmem) also prefetched the COW pages,
while retaining the existing shmem XArray path.

Locking and open questions
--------------------------

The PTE walk keeps the mmap read lock, as the existing anonymous
MADV_WILLNEED path does.  Swap reads for ordinary block devices are normally
submitted asynchronously, but folio allocation, zswap, and synchronous swap
devices can still make this path block.  I have not attempted to collect
addresses, drop mmap_lock, and perform the reads later: doing that safely
would require pinning or revalidating the VMA, PTEs, swap entries, and NUMA
policy after the unlock.

Could the maintainers please advise on the following points?

  1. Is reusing the existing mmap-lock and PTE-walk model acceptable for this
     small fix, or should this wait for a more general swap-in redesign?
  2. Should private shmem COW PTEs be included in this change, or should the
     first version be limited to regular file mappings?
  3. Is the anon_vma guard and conditional LRU drain the right trade-off, or
     would you prefer a smaller change that keeps the existing drain calls?

I have built mm/madvise.o, run git diff --check, and run checkpatch.pl --strict
on this version.  Any review of the implementation, test methodology, or
the proposed scope would be very helpful before I send a v2.

Fixes: 1998cc048901 ("mm: make madvise(MADV_WILLNEED) support swap file prefetch")
Reported-by: Mike Kaplinskiy <mike@xxxxxxxxx>
Link: https://lore.kernel.org/linux-mm/CABeknB_S2XJSHFgnHdgnN0rjzHhH4oQJs_APq9fvxHztQ_pgiA@xxxxxxxxxxxxxx/
Signed-off-by: zhaozhengzhuo <zhaozhengzhuo@xxxxxxxxxxxxx>
---
 mm/madvise.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 73c2901b9adb..96cbce6c7f8b 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -193,10 +193,16 @@ static int madvise_update_vma(vm_flags_t new_flags,
 }
 
 #ifdef CONFIG_SWAP
+struct swapin_walk_ctx {
+	struct vm_area_struct *vma;
+	bool swapped;
+};
+
 static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start,
 		unsigned long end, struct mm_walk *walk)
 {
-	struct vm_area_struct *vma = walk->private;
+	struct swapin_walk_ctx *swc = walk->private;
+	struct vm_area_struct *vma = swc->vma;
 	struct swap_io_ctx ctx = {};
 	pte_t *ptep = NULL;
 	spinlock_t *ptl;
@@ -223,8 +229,10 @@ static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start,
 
 		folio = read_swap_cache_async(&ctx, entry, GFP_HIGHUSER_MOVABLE,
 					vma, addr);
-		if (folio)
+		if (folio) {
+			swc->swapped = true;
 			folio_put(folio);
+		}
 	}
 
 	if (ptep)
@@ -297,10 +305,22 @@ static long madvise_willneed(struct madvise_behavior *madv_behavior)
 	loff_t offset;
 
 #ifdef CONFIG_SWAP
-	if (!file) {
-		walk_page_range_vma(vma, start, end, &swapin_walk_ops, vma);
-		lru_add_drain(); /* Push any new pages onto the LRU now */
-		return 0;
+	bool private_file = file && !(vma->vm_flags & VM_SHARED);
+
+	/*
+	 * A private file mapping can contain anonymous COW pages. Once such
+	 * pages are swapped out, their PTEs contain swap entries even though
+	 * the VMA still has vm_file set. Prefetch those pages as well; file
+	 * readahead can only fetch the original file contents.
+	 */
+	if (!file || (private_file && vma->anon_vma)) {
+		struct swapin_walk_ctx ctx = { .vma = vma };
+
+		walk_page_range_vma(vma, start, end, &swapin_walk_ops, &ctx);
+		if (ctx.swapped)
+			lru_add_drain(); /* Push any new pages onto the LRU now */
+		if (!file)
+			return 0;
 	}
 
 	if (shmem_mapping(file->f_mapping)) {
-- 
2.43.0