[PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Barry Song]

From: Barry Song <v-songbaohua@oppo.com>

We've already found the VMA within madvise_walk_vmas() before calling
specific madvise behavior functions like madvise_free_single_vma().
So calling walk_page_range() and doing find_vma() again seems
unnecessary. It also prevents potential optimizations in those madvise
callbacks, particularly the use of dedicated per-VMA locking.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Cc: "Liam R. Howlett" <Liam.Howlett@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jann Horn <jannh@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>
Cc: Lokesh Gidra <lokeshgidra@google.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Tangquan Zheng <zhengtangquan@oppo.com>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 -v2:
  * Also extend the modification to callbacks beyond
    madvise_free_single_vma() since the code flow is
    the same - Dev, Lorenzo
 -rfc:
  https://lore.kernel.org/linux-mm/20250603013154.5905-1-21cnbao@gmail.com/

 mm/madvise.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 5f7a66a1617e..56d9ca2557b9 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -282,7 +282,7 @@ static long madvise_willneed(struct vm_area_struct *vma,
 	*prev = vma;
 #ifdef CONFIG_SWAP
 	if (!file) {
-		walk_page_range(vma->vm_mm, start, end, &swapin_walk_ops, 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;
 	}
@@ -581,7 +581,7 @@ static void madvise_cold_page_range(struct mmu_gather *tlb,
 	};
 
 	tlb_start_vma(tlb, vma);
-	walk_page_range(vma->vm_mm, addr, end, &cold_walk_ops, &walk_private);
+	walk_page_range_vma(vma, addr, end, &cold_walk_ops, &walk_private);
 	tlb_end_vma(tlb, vma);
 }
 
@@ -619,7 +619,7 @@ static void madvise_pageout_page_range(struct mmu_gather *tlb,
 	};
 
 	tlb_start_vma(tlb, vma);
-	walk_page_range(vma->vm_mm, addr, end, &cold_walk_ops, &walk_private);
+	walk_page_range_vma(vma, addr, end, &cold_walk_ops, &walk_private);
 	tlb_end_vma(tlb, vma);
 }
 
@@ -825,7 +825,7 @@ static int madvise_free_single_vma(struct madvise_behavior *madv_behavior,
 
 	mmu_notifier_invalidate_range_start(&range);
 	tlb_start_vma(tlb, vma);
-	walk_page_range(vma->vm_mm, range.start, range.end,
+	walk_page_range_vma(vma, range.start, range.end,
 			&madvise_free_walk_ops, tlb);
 	tlb_end_vma(tlb, vma);
 	mmu_notifier_invalidate_range_end(&range);
@@ -1160,7 +1160,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
 		unsigned long nr_pages = 0;
 
 		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
-		err = walk_page_range_mm(vma->vm_mm, start, end,
+		err = walk_page_range_vma(vma, start, end,
 					 &guard_install_walk_ops, &nr_pages);
 		if (err < 0)
 			return err;
@@ -1244,7 +1244,7 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
 	if (!is_valid_guard_vma(vma, /* allow_locked = */true))
 		return -EINVAL;
 
-	return walk_page_range(vma->vm_mm, start, end,
+	return walk_page_range_vma(vma, start, end,
 			       &guard_remove_walk_ops, NULL);
 }
 
-- 
2.39.3 (Apple Git-146)

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Harry Yoo]

On Thu, Jun 05, 2025 at 08:31:44PM +1200, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> We've already found the VMA within madvise_walk_vmas() before calling
> specific madvise behavior functions like madvise_free_single_vma().
> So calling walk_page_range() and doing find_vma() again seems
> unnecessary. It also prevents potential optimizations in those madvise
> callbacks, particularly the use of dedicated per-VMA locking.
> 
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
> Cc: "Liam R. Howlett" <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jann Horn <jannh@google.com>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Lokesh Gidra <lokeshgidra@google.com>
> Cc: Dev Jain <dev.jain@arm.com>
> Cc: Tangquan Zheng <zhengtangquan@oppo.com>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>

Nice!

Looks good to me,
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

-- 
Cheers,
Harry / Hyeonggon

> ---
>  -v2:
>   * Also extend the modification to callbacks beyond
>     madvise_free_single_vma() since the code flow is
>     the same - Dev, Lorenzo
>  -rfc:
>   https://lore.kernel.org/linux-mm/20250603013154.5905-1-21cnbao@gmail.com/
> 
>  mm/madvise.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 5f7a66a1617e..56d9ca2557b9 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -282,7 +282,7 @@ static long madvise_willneed(struct vm_area_struct *vma,
>  	*prev = vma;
>  #ifdef CONFIG_SWAP
>  	if (!file) {
> -		walk_page_range(vma->vm_mm, start, end, &swapin_walk_ops, 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;
>  	}
> @@ -581,7 +581,7 @@ static void madvise_cold_page_range(struct mmu_gather *tlb,
>  	};
>  
>  	tlb_start_vma(tlb, vma);
> -	walk_page_range(vma->vm_mm, addr, end, &cold_walk_ops, &walk_private);
> +	walk_page_range_vma(vma, addr, end, &cold_walk_ops, &walk_private);
>  	tlb_end_vma(tlb, vma);
>  }
>  
> @@ -619,7 +619,7 @@ static void madvise_pageout_page_range(struct mmu_gather *tlb,
>  	};
>  
>  	tlb_start_vma(tlb, vma);
> -	walk_page_range(vma->vm_mm, addr, end, &cold_walk_ops, &walk_private);
> +	walk_page_range_vma(vma, addr, end, &cold_walk_ops, &walk_private);
>  	tlb_end_vma(tlb, vma);
>  }
>  
> @@ -825,7 +825,7 @@ static int madvise_free_single_vma(struct madvise_behavior *madv_behavior,
>  
>  	mmu_notifier_invalidate_range_start(&range);
>  	tlb_start_vma(tlb, vma);
> -	walk_page_range(vma->vm_mm, range.start, range.end,
> +	walk_page_range_vma(vma, range.start, range.end,
>  			&madvise_free_walk_ops, tlb);
>  	tlb_end_vma(tlb, vma);
>  	mmu_notifier_invalidate_range_end(&range);
> @@ -1160,7 +1160,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
>  		unsigned long nr_pages = 0;
>  
>  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> -		err = walk_page_range_mm(vma->vm_mm, start, end,
> +		err = walk_page_range_vma(vma, start, end,
>  					 &guard_install_walk_ops, &nr_pages);
>  		if (err < 0)
>  			return err;
> @@ -1244,7 +1244,7 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
>  	if (!is_valid_guard_vma(vma, /* allow_locked = */true))
>  		return -EINVAL;
>  
> -	return walk_page_range(vma->vm_mm, start, end,
> +	return walk_page_range_vma(vma, start, end,
>  			       &guard_remove_walk_ops, NULL);
>  }
>  
> -- 
> 2.39.3 (Apple Git-146)
> 
> 

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Dev Jain]

On 05/06/25 2:01 pm, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
>
> We've already found the VMA within madvise_walk_vmas() before calling
> specific madvise behavior functions like madvise_free_single_vma().
> So calling walk_page_range() and doing find_vma() again seems
> unnecessary. It also prevents potential optimizations in those madvise
> callbacks, particularly the use of dedicated per-VMA locking.
>
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
> Cc: "Liam R. Howlett" <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jann Horn <jannh@google.com>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Lokesh Gidra <lokeshgidra@google.com>
> Cc: Dev Jain <dev.jain@arm.com>
> Cc: Tangquan Zheng <zhengtangquan@oppo.com>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> ---


Reviewed-by: Dev Jain <dev.jain@arm.com>

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Vlastimil Babka]

On 6/5/25 10:31, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> We've already found the VMA within madvise_walk_vmas() before calling
> specific madvise behavior functions like madvise_free_single_vma().
> So calling walk_page_range() and doing find_vma() again seems
> unnecessary. It also prevents potential optimizations in those madvise
> callbacks, particularly the use of dedicated per-VMA locking.
> 
> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>
> Cc: "Liam R. Howlett" <Liam.Howlett@oracle.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jann Horn <jannh@google.com>
> Cc: Suren Baghdasaryan <surenb@google.com>
> Cc: Lokesh Gidra <lokeshgidra@google.com>
> Cc: Dev Jain <dev.jain@arm.com>
> Cc: Tangquan Zheng <zhengtangquan@oppo.com>
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

> @@ -1160,7 +1160,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
>  		unsigned long nr_pages = 0;
>  
>  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> -		err = walk_page_range_mm(vma->vm_mm, start, end,
> +		err = walk_page_range_vma(vma, start, end,
>  					 &guard_install_walk_ops, &nr_pages);

Nit: breaks the parameter alignment. Do we care? It's Lorenzo's code so
maybe not ;P

>  		if (err < 0)
>  			return err;
> @@ -1244,7 +1244,7 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
>  	if (!is_valid_guard_vma(vma, /* allow_locked = */true))
>  		return -EINVAL;
>  
> -	return walk_page_range(vma->vm_mm, start, end,
> +	return walk_page_range_vma(vma, start, end,
>  			       &guard_remove_walk_ops, NULL);

Same.

>  }
>  

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Lorenzo Stoakes]

On Thu, Jun 05, 2025 at 04:12:48PM +0200, Vlastimil Babka wrote:
> On 6/5/25 10:31, Barry Song wrote:
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > We've already found the VMA within madvise_walk_vmas() before calling
> > specific madvise behavior functions like madvise_free_single_vma().
> > So calling walk_page_range() and doing find_vma() again seems
> > unnecessary. It also prevents potential optimizations in those madvise
> > callbacks, particularly the use of dedicated per-VMA locking.
> >
> > Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Acked-by: David Hildenbrand <david@redhat.com>
> > Reviewed-by: Oscar Salvador <osalvador@suse.de>
> > Cc: "Liam R. Howlett" <Liam.Howlett@oracle.com>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Jann Horn <jannh@google.com>
> > Cc: Suren Baghdasaryan <surenb@google.com>
> > Cc: Lokesh Gidra <lokeshgidra@google.com>
> > Cc: Dev Jain <dev.jain@arm.com>
> > Cc: Tangquan Zheng <zhengtangquan@oppo.com>
> > Signed-off-by: Barry Song <v-songbaohua@oppo.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
> Thanks!
>
> > @@ -1160,7 +1160,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
> >  		unsigned long nr_pages = 0;
> >
> >  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> > -		err = walk_page_range_mm(vma->vm_mm, start, end,
> > +		err = walk_page_range_vma(vma, start, end,
> >  					 &guard_install_walk_ops, &nr_pages);
>
> Nit: breaks the parameter alignment. Do we care? It's Lorenzo's code so
> maybe not ;P

OMG!!

Nah it's fine leave it as-is :P

Can always go fix it up at some later date... or make it more sane with just tab
alignment...

>
> >  		if (err < 0)
> >  			return err;
> > @@ -1244,7 +1244,7 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
> >  	if (!is_valid_guard_vma(vma, /* allow_locked = */true))
> >  		return -EINVAL;
> >
> > -	return walk_page_range(vma->vm_mm, start, end,
> > +	return walk_page_range_vma(vma, start, end,
> >  			       &guard_remove_walk_ops, NULL);
>
> Same.
>
> >  }
> >
>

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Ryan Roberts]

On 05/06/2025 09:31, Barry Song wrote:
> From: Barry Song <v-songbaohua@oppo.com>
> 
> We've already found the VMA within madvise_walk_vmas() before calling
> specific madvise behavior functions like madvise_free_single_vma().
> So calling walk_page_range() and doing find_vma() again seems
> unnecessary. It also prevents potential optimizations in those madvise
> callbacks, particularly the use of dedicated per-VMA locking.

FYI it looks like this patch breaks all the guard-region mm selftests with:

# guard-regions.c:719:split_merge:Expected madvise(ptr, 10 * page_size,
MADV_GUARD_INSTALL) (-1) == 0 (0)

Am I the only one that runs these things? :)

[...]

>  mm/madvise.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

[...]

> @@ -1160,7 +1160,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
>  		unsigned long nr_pages = 0;
>  
>  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> -		err = walk_page_range_mm(vma->vm_mm, start, end,
> +		err = walk_page_range_vma(vma, start, end,
>  					 &guard_install_walk_ops, &nr_pages);

IIRC walk_page_range_mm() is an internal API that allows the install_pte()
callback, and the other (public) APIs explicitly disallow it, so presumably
walk_page_range_vma() is now returning an error due to install_pte != NULL?

Thanks,
Ryan

>  		if (err < 0)
>  			return err;
> @@ -1244,7 +1244,7 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
>  	if (!is_valid_guard_vma(vma, /* allow_locked = */true))
>  		return -EINVAL;
>  
> -	return walk_page_range(vma->vm_mm, start, end,
> +	return walk_page_range_vma(vma, start, end,
>  			       &guard_remove_walk_ops, NULL);
>  }
>  

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Lorenzo Stoakes]

Andrew - could you drop this patch until this is fixed? Whoops!

On Mon, Jun 09, 2025 at 10:52:47AM +0100, Ryan Roberts wrote:
> On 05/06/2025 09:31, Barry Song wrote:
> > From: Barry Song <v-songbaohua@oppo.com>
> >
> > We've already found the VMA within madvise_walk_vmas() before calling
> > specific madvise behavior functions like madvise_free_single_vma().
> > So calling walk_page_range() and doing find_vma() again seems
> > unnecessary. It also prevents potential optimizations in those madvise
> > callbacks, particularly the use of dedicated per-VMA locking.
>
> FYI it looks like this patch breaks all the guard-region mm selftests with:
>
> # guard-regions.c:719:split_merge:Expected madvise(ptr, 10 * page_size,
> MADV_GUARD_INSTALL) (-1) == 0 (0)
>
> Am I the only one that runs these things? :)

I normally do :) the one time I don't... :P

>
> [...]
>
> >  mm/madvise.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
>
> [...]
>
> > @@ -1160,7 +1160,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
> >  		unsigned long nr_pages = 0;
> >
> >  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> > -		err = walk_page_range_mm(vma->vm_mm, start, end,
> > +		err = walk_page_range_vma(vma, start, end,
> >  					 &guard_install_walk_ops, &nr_pages);
>
> IIRC walk_page_range_mm() is an internal API that allows the install_pte()
> callback, and the other (public) APIs explicitly disallow it, so presumably
> walk_page_range_vma() is now returning an error due to install_pte != NULL?
>

Yeah dear god I missed this oops!

Yeah Barry - could you revert this change for the guard region bits please? So
this is intentional as we do not want anything non-mm to have access to
install_pte.

I'll maybe have a think about this in terms of whether we want to do this a
different way but for now this will fix the issue and get the patch in.

Otherwise patch is ok AFAICT...

With that fixed feel free to propagate tag to a v2.

> Thanks,
> Ryan
>
> >  		if (err < 0)
> >  			return err;
> > @@ -1244,7 +1244,7 @@ static long madvise_guard_remove(struct vm_area_struct *vma,
> >  	if (!is_valid_guard_vma(vma, /* allow_locked = */true))
> >  		return -EINVAL;
> >
> > -	return walk_page_range(vma->vm_mm, start, end,
> > +	return walk_page_range_vma(vma, start, end,
> >  			       &guard_remove_walk_ops, NULL);
> >  }
> >
>

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Barry Song]

> Yeah dear god I missed this oops!
>
> Yeah Barry - could you revert this change for the guard region bits please? So
> this is intentional as we do not want anything non-mm to have access to
> install_pte.

All my fault! I wrote a multi-process/thread test to issue lots of madvise  
calls, but it looks like I missed INSTALL_GUARD.

Thanks, Ryan & Lorenzo! Does Andrew prefer to pick up the fix below, or  
would it be better to send a new version? He’s handled fixes like this in  
the past—happy to resend if needed.

From: Barry Song <v-songbaohua@oppo.com>
Date: Mon, 9 Jun 2025 22:42:13 +1200
Subject: [PATCH] mm: madvise: revert the walk_page_range_vma change for
 MADV_GUARD_INSTALL

Fix the broken MADV_GUARD_INSTALL reported by Ryan.
 # guard-regions.c:719:split_merge:Expected madvise(ptr, 10 * page_size,
 MADV_GUARD_INSTALL) (-1) == 0 (0)

Reported-by: Ryan Roberts <ryan.roberts@arm.com>
Closes: https://lore.kernel.org/linux-mm/671f8164-a90b-48d7-9446-359eb9493500@arm.com/
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
 mm/madvise.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 8382614b71d1..381eedde8f6d 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -1141,7 +1141,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
 		unsigned long nr_pages = 0;
 
 		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
-		err = walk_page_range_vma(vma, start, end,
+		err = walk_page_range_mm(vma->vm_mm, start, end,
 					 &guard_install_walk_ops, &nr_pages);
 		if (err < 0)
 			return err;
-- 
2.39.3 (Apple Git-146)

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Ryan Roberts]

On 09/06/2025 11:55, Barry Song wrote:
>> Yeah dear god I missed this oops!
>>
>> Yeah Barry - could you revert this change for the guard region bits please? So
>> this is intentional as we do not want anything non-mm to have access to
>> install_pte.
> 
> All my fault! I wrote a multi-process/thread test to issue lots of madvise  
> calls, but it looks like I missed INSTALL_GUARD.
> 
> Thanks, Ryan & Lorenzo! Does Andrew prefer to pick up the fix below, or  
> would it be better to send a new version? He’s handled fixes like this in  
> the past—happy to resend if needed.
> 
> From: Barry Song <v-songbaohua@oppo.com>
> Date: Mon, 9 Jun 2025 22:42:13 +1200
> Subject: [PATCH] mm: madvise: revert the walk_page_range_vma change for
>  MADV_GUARD_INSTALL
> 
> Fix the broken MADV_GUARD_INSTALL reported by Ryan.
>  # guard-regions.c:719:split_merge:Expected madvise(ptr, 10 * page_size,
>  MADV_GUARD_INSTALL) (-1) == 0 (0)
> 
> Reported-by: Ryan Roberts <ryan.roberts@arm.com>
> Closes: https://lore.kernel.org/linux-mm/671f8164-a90b-48d7-9446-359eb9493500@arm.com/
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>

FWIW, the rest of the original patch looks good to me, ands this fix looks
correct, so:

Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

> ---
>  mm/madvise.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 8382614b71d1..381eedde8f6d 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1141,7 +1141,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
>  		unsigned long nr_pages = 0;
>  
>  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> -		err = walk_page_range_vma(vma, start, end,
> +		err = walk_page_range_mm(vma->vm_mm, start, end,
>  					 &guard_install_walk_ops, &nr_pages);
>  		if (err < 0)
>  			return err;

Re: [PATCH v2] mm: madvise: use walk_page_range_vma() instead of walk_page_range()

[Lorenzo Stoakes]

On Mon, Jun 09, 2025 at 10:55:13PM +1200, Barry Song wrote:
> > Yeah dear god I missed this oops!
> >
> > Yeah Barry - could you revert this change for the guard region bits please? So
> > this is intentional as we do not want anything non-mm to have access to
> > install_pte.
>
> All my fault! I wrote a multi-process/thread test to issue lots of madvise
> calls, but it looks like I missed INSTALL_GUARD.
>
> Thanks, Ryan & Lorenzo! Does Andrew prefer to pick up the fix below, or
> would it be better to send a new version? He’s handled fixes like this in
> the past—happy to resend if needed.

I'm totally fine with this thanks!

I can't speak for Andrew of course but he highlighted at LSF/MM his
preference for incremental fixes like this so I would say this is probably
his preference also :)

>
> From: Barry Song <v-songbaohua@oppo.com>
> Date: Mon, 9 Jun 2025 22:42:13 +1200
> Subject: [PATCH] mm: madvise: revert the walk_page_range_vma change for
>  MADV_GUARD_INSTALL
>
> Fix the broken MADV_GUARD_INSTALL reported by Ryan.
>  # guard-regions.c:719:split_merge:Expected madvise(ptr, 10 * page_size,
>  MADV_GUARD_INSTALL) (-1) == 0 (0)
>
> Reported-by: Ryan Roberts <ryan.roberts@arm.com>
> Closes: https://lore.kernel.org/linux-mm/671f8164-a90b-48d7-9446-359eb9493500@arm.com/
> Signed-off-by: Barry Song <v-songbaohua@oppo.com>
> ---
>  mm/madvise.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 8382614b71d1..381eedde8f6d 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -1141,7 +1141,7 @@ static long madvise_guard_install(struct vm_area_struct *vma,
>  		unsigned long nr_pages = 0;
>
>  		/* Returns < 0 on error, == 0 if success, > 0 if zap needed. */
> -		err = walk_page_range_vma(vma, start, end,
> +		err = walk_page_range_mm(vma->vm_mm, start, end,
>  					 &guard_install_walk_ops, &nr_pages);

Actually ack that only the install needs it, we can keep this fix for the
remove, as that doesn't of course use .install_pte().

Also it's kind of neat to only use the internal one in the instance we need
that and the 'public' interface when we don't.

>  		if (err < 0)
>  			return err;
> --
> 2.39.3 (Apple Git-146)
>

I also did actually test this this time :P and can confirm it fixes the issue.