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

[Pedro Falcato]

On Fri, Sep 04, 2026 at 05:01:37PM +0100, Lorenzo Stoakes (ARM) wrote:
> 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@xxxxxxxxx>
> > > Closes: https://lore.kernel.org/all/CABeknB_S2XJSHFgnHdgnN0rjzHhH4oQJs_APq9fvxHztQ_pgiA@xxxxxxxxxxxxxx/
> > > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> > > ---
> > >  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@xxxxxxxxxx>
> 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@xxxxxxxxx>
> Closes: https://lore.kernel.org/all/CABeknB_S2XJSHFgnHdgnN0rjzHhH4oQJs_APq9fvxHztQ_pgiA@xxxxxxxxxxxxxx/
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

LGTM, thanks!

Reviewed-by: Pedro Falcato <pfalcato@xxxxxxx>

-- 
Pedro