Re: [PATCH] mm/madvise: swap in CoW'd MAP_PRIVATE-file mappings on MADV_WILLNEED

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Fri, Sep 04, 2026 at 04:24:59PM +0100, Pedro Falcato wrote:
> On Fri, Sep 04, 2026 at 04:12:53PM +0100, Lorenzo Stoakes (ARM) wrote:
> > Currently MADV_WILLNEED treats file-backed and pure anonymous mappings
> > entirely separately - using POSIX_FADV_WILLNEED (equivalent of a readahead)
> > for the former and a tree walk and swap in to swap cache for the latter.
> >
> > MAP_PRIVATE-file backed mappings straddle the two and currently get treated
> > as if they were purely file-backed, meaning any swapped out private pages
> > remain swapped out.
> >
> > Resolve the issue by explicitly checking for CoW'd MAP_PRIVATE-file backed
> > mappings and performing both walks in this case.
> >
> > Since the logic checks for vma->anon_vma this means un-CoW'd
> > MAP_PRIVATE-file backed mappings retain only the single file walk.
> >
> > Reported-by: Mike Kaplinskiy <mike@recall.ai>
> > Closes: https://lore.kernel.org/all/CABeknB_S2XJSHFgnHdgnN0rjzHhH4oQJs_APq9fvxHztQ_pgiA@mail.gmail.com/
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  mm/madvise.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index 73c2901b9adb..d0510dd49dde 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -297,10 +297,12 @@ static long madvise_willneed(struct madvise_behavior *madv_behavior)
> >  	loff_t offset;
> >
> >  #ifdef CONFIG_SWAP
> > -	if (!file) {
> > +	if (!file || (vma_is_cow_mapping(vma) && vma->anon_vma)) {
>
> Couldn't this all be simplified to
>
> 	if (vma->anon_vma) {
>
> ? swapin needs anon pages to have been faulted-in. Non-cow mappings won't
> have an anon_vma, nor will fully empty anonymous VMAs (and that's fine).
> Right?

Hmm good point :)

Though the

	if (!file)
		return 0;

Would have to be outside of the block to avoid an anon unfaulted (nop) from
being skipped.

And it's a real improvement to have unfaulted anon skip...

But I find that version is documenting what's going on a lot less.

Since both anon and MAP_PRIVATE file-backed are CoW mappings we could just
reference that.

Andrew - could you swap the patch out in-place with below? Thanks!

Cheers, Lorenzo

----8<----
From a706374a1186b6b4409a0e760e8728bcadba0c96 Mon Sep 17 00:00:00 2001
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
Date: Thu, 3 Sep 2026 20:08:39 +0100
Subject: [PATCH] mm/madvise: swap in CoW'd MAP_PRIVATE-file mappings on
 MADV_WILLNEED

Currently MADV_WILLNEED treats file-backed and pure anonymous mappings
entirely separately - using POSIX_FADV_WILLNEED (equivalent of a readahead)
for the former and a tree walk and swap in to swap cache for the latter.

MAP_PRIVATE-file backed mappings straddle the two and currently get treated
as if they were purely file-backed, meaning any swapped out private pages
remain swapped out.

Resolve the issue by explicitly checking for CoW'd MAP_PRIVATE-file backed
mappings and performing both walks in this case.

Since the logic checks for vma->anon_vma this means un-CoW'd
MAP_PRIVATE-file backed mappings retain only the single file walk.

Reported-by: Mike Kaplinskiy <mike@recall.ai>
Closes: https://lore.kernel.org/all/CABeknB_S2XJSHFgnHdgnN0rjzHhH4oQJs_APq9fvxHztQ_pgiA@mail.gmail.com/
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/madvise.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 73c2901b9adb..963337f93a7a 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -297,11 +297,12 @@ static long madvise_willneed(struct madvise_behavior *madv_behavior)
 	loff_t offset;

 #ifdef CONFIG_SWAP
-	if (!file) {
+	if (vma_is_cow_mapping(vma) && vma->anon_vma) {
 		walk_page_range_vma(vma, start, end, &swapin_walk_ops, vma);
 		lru_add_drain(); /* Push any new pages onto the LRU now */
-		return 0;
 	}
+	if (!file)
+		return 0;

 	if (shmem_mapping(file->f_mapping)) {
 		shmem_swapin_range(vma, start, end, file->f_mapping);
--
2.55.0