Pass through the unmap_desc to free_pgtables() because it almost has
everything necessary and is already on the stack.
Updates testing code as necessary.
No functional changes intended.
Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
mm/internal.h | 5 +----
mm/memory.c | 21 ++++++++++-----------
mm/mmap.c | 6 +++---
mm/vma.c | 7 ++-----
tools/testing/vma/vma_internal.h | 11 ++---------
5 files changed, 18 insertions(+), 32 deletions(-)
diff --git a/mm/internal.h b/mm/internal.h
index 1239944f2830a..f22329967e908 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -445,10 +445,7 @@ bool __folio_end_writeback(struct folio *folio);
void deactivate_file_folio(struct folio *folio);
void folio_activate(struct folio *folio);
-void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
- struct vm_area_struct *start_vma, unsigned long floor,
- unsigned long ceiling, unsigned long tree_max,
- bool mm_wr_locked);
+void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc);
void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
diff --git a/mm/memory.c b/mm/memory.c
index 8d4d976311037..98c5ffd28a109 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -387,15 +387,14 @@ void free_pgd_range(struct mmu_gather *tlb,
* The tree_max differs from the ceiling when a dup_mmap() failed and the tree
* has unrelated data to the mm_struct being torn down.
*/
-void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
- struct vm_area_struct *vma, unsigned long floor,
- unsigned long ceiling, unsigned long tree_max,
- bool mm_wr_locked)
+void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc)
{
struct unlink_vma_file_batch vb;
+ struct ma_state *mas = desc->mas;
+ struct vm_area_struct *vma = desc->first;
/* underflow can happen and is fine */
- WARN_ON_ONCE(tree_max - 1 > ceiling - 1);
+ WARN_ON_ONCE(desc->tree_max - 1 > desc->last_pgaddr - 1);
tlb_free_vmas(tlb);
@@ -407,13 +406,13 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
* Note: USER_PGTABLES_CEILING may be passed as ceiling and may
* be 0. This will underflow and is okay.
*/
- next = mas_find(mas, tree_max - 1);
+ next = mas_find(mas, desc->tree_max - 1);
/*
* Hide vma from rmap and truncate_pagecache before freeing
* pgtables
*/
- if (mm_wr_locked)
+ if (desc->mm_wr_locked)
vma_start_write(vma);
unlink_anon_vmas(vma);
@@ -425,16 +424,16 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
*/
while (next && next->vm_start <= vma->vm_end + PMD_SIZE) {
vma = next;
- next = mas_find(mas, tree_max - 1);
- if (mm_wr_locked)
+ next = mas_find(mas, desc->tree_max - 1);
+ if (desc->mm_wr_locked)
vma_start_write(vma);
unlink_anon_vmas(vma);
unlink_file_vma_batch_add(&vb, vma);
}
unlink_file_vma_batch_final(&vb);
- free_pgd_range(tlb, addr, vma->vm_end,
- floor, next ? next->vm_start : ceiling);
+ free_pgd_range(tlb, addr, vma->vm_end, desc->first_pgaddr,
+ next ? next->vm_start : desc->last_pgaddr);
vma = next;
} while (vma);
}
diff --git a/mm/mmap.c b/mm/mmap.c
index 6011f62b0a294..9908481452780 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1311,10 +1311,10 @@ void exit_mmap(struct mm_struct *mm)
*/
mm_flags_set(MMF_OOM_SKIP, mm);
mmap_write_lock(mm);
+ unmap.mm_wr_locked = true;
mt_clear_in_rcu(&mm->mm_mt);
- vma_iter_set(&vmi, vma->vm_end);
- free_pgtables(&tlb, &vmi.mas, vma, FIRST_USER_ADDRESS,
- USER_PGTABLES_CEILING, USER_PGTABLES_CEILING, true);
+ vma_iter_set(&vmi, unmap.tree_reset);
+ free_pgtables(&tlb, &unmap);
tlb_finish_mmu(&tlb);
/*
diff --git a/mm/vma.c b/mm/vma.c
index ad64cd9795ef3..ba155a539d160 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -476,16 +476,13 @@ void remove_vma(struct vm_area_struct *vma)
void unmap_region(struct unmap_desc *desc)
{
struct mm_struct *mm = desc->first->vm_mm;
- struct ma_state *mas = desc->mas;
struct mmu_gather tlb;
tlb_gather_mmu(&tlb, mm);
update_hiwater_rss(mm);
unmap_vmas(&tlb, desc);
- mas_set(mas, desc->tree_reset);
- free_pgtables(&tlb, mas, desc->first, desc->first_pgaddr,
- desc->last_pgaddr, desc->tree_max,
- desc->mm_wr_locked);
+ mas_set(desc->mas, desc->tree_reset);
+ free_pgtables(&tlb, desc);
tlb_finish_mmu(&tlb);
}
diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
index d73ad4747d40a..435c5a24480bc 100644
--- a/tools/testing/vma/vma_internal.h
+++ b/tools/testing/vma/vma_internal.h
@@ -892,17 +892,10 @@ static inline void unmap_vmas(struct mmu_gather *tlb, struct unmap_desc *unmap)
(void)unmap;
}
-static inline void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
- struct vm_area_struct *vma, unsigned long floor,
- unsigned long ceiling, unsigned long tree_max,
- bool mm_wr_locked)
+static inline void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc)
{
(void)tlb;
- (void)mas;
- (void)vma;
- (void)floor;
- (void)ceiling;
- (void)mm_wr_locked;
+ (void)desc;
}
static inline void mapping_unmap_writable(struct address_space *)
--
2.47.2On Tue, Sep 09, 2025 at 03:09:45PM -0400, Liam R. Howlett wrote:
> Pass through the unmap_desc to free_pgtables() because it almost has
> everything necessary and is already on the stack.
>
> Updates testing code as necessary.
>
> No functional changes intended.
>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
To be clear, I _love_ the use of this helper struct. All commentary is more
so details. what you're doing here is great... :)
Other than nits below this looks fine so on basis of addressing them:
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> mm/internal.h | 5 +----
> mm/memory.c | 21 ++++++++++-----------
> mm/mmap.c | 6 +++---
> mm/vma.c | 7 ++-----
> tools/testing/vma/vma_internal.h | 11 ++---------
> 5 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 1239944f2830a..f22329967e908 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -445,10 +445,7 @@ bool __folio_end_writeback(struct folio *folio);
> void deactivate_file_folio(struct folio *folio);
> void folio_activate(struct folio *folio);
>
> -void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> - struct vm_area_struct *start_vma, unsigned long floor,
> - unsigned long ceiling, unsigned long tree_max,
> - bool mm_wr_locked);
> +void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc);
>
> void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 8d4d976311037..98c5ffd28a109 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -387,15 +387,14 @@ void free_pgd_range(struct mmu_gather *tlb,
> * The tree_max differs from the ceiling when a dup_mmap() failed and the tree
> * has unrelated data to the mm_struct being torn down.
> */
> -void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> - struct vm_area_struct *vma, unsigned long floor,
> - unsigned long ceiling, unsigned long tree_max,
> - bool mm_wr_locked)
> +void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc)
God this is nice.
> {
> struct unlink_vma_file_batch vb;
> + struct ma_state *mas = desc->mas;
> + struct vm_area_struct *vma = desc->first;
>
> /* underflow can happen and is fine */
> - WARN_ON_ONCE(tree_max - 1 > ceiling - 1);
> + WARN_ON_ONCE(desc->tree_max - 1 > desc->last_pgaddr - 1);
>
> tlb_free_vmas(tlb);
>
> @@ -407,13 +406,13 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> * Note: USER_PGTABLES_CEILING may be passed as ceiling and may
> * be 0. This will underflow and is okay.
> */
> - next = mas_find(mas, tree_max - 1);
> + next = mas_find(mas, desc->tree_max - 1);
>
> /*
> * Hide vma from rmap and truncate_pagecache before freeing
> * pgtables
> */
> - if (mm_wr_locked)
> + if (desc->mm_wr_locked)
> vma_start_write(vma);
> unlink_anon_vmas(vma);
>
> @@ -425,16 +424,16 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> */
> while (next && next->vm_start <= vma->vm_end + PMD_SIZE) {
> vma = next;
> - next = mas_find(mas, tree_max - 1);
> - if (mm_wr_locked)
> + next = mas_find(mas, desc->tree_max - 1);
> + if (desc->mm_wr_locked)
> vma_start_write(vma);
> unlink_anon_vmas(vma);
> unlink_file_vma_batch_add(&vb, vma);
> }
> unlink_file_vma_batch_final(&vb);
>
> - free_pgd_range(tlb, addr, vma->vm_end,
> - floor, next ? next->vm_start : ceiling);
> + free_pgd_range(tlb, addr, vma->vm_end, desc->first_pgaddr,
> + next ? next->vm_start : desc->last_pgaddr);
Hm, actually, isn't ceiling exclusive? So 'last_pgaddr' seems a bit
misleading?
> vma = next;
> } while (vma);
> }
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 6011f62b0a294..9908481452780 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1311,10 +1311,10 @@ void exit_mmap(struct mm_struct *mm)
> */
> mm_flags_set(MMF_OOM_SKIP, mm);
> mmap_write_lock(mm);
> + unmap.mm_wr_locked = true;
> mt_clear_in_rcu(&mm->mm_mt);
> - vma_iter_set(&vmi, vma->vm_end);
> - free_pgtables(&tlb, &vmi.mas, vma, FIRST_USER_ADDRESS,
> - USER_PGTABLES_CEILING, USER_PGTABLES_CEILING, true);
> + vma_iter_set(&vmi, unmap.tree_reset);
> + free_pgtables(&tlb, &unmap);
Lovely!
> tlb_finish_mmu(&tlb);
>
> /*
> diff --git a/mm/vma.c b/mm/vma.c
> index ad64cd9795ef3..ba155a539d160 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -476,16 +476,13 @@ void remove_vma(struct vm_area_struct *vma)
> void unmap_region(struct unmap_desc *desc)
> {
> struct mm_struct *mm = desc->first->vm_mm;
> - struct ma_state *mas = desc->mas;
> struct mmu_gather tlb;
>
> tlb_gather_mmu(&tlb, mm);
> update_hiwater_rss(mm);
> unmap_vmas(&tlb, desc);
> - mas_set(mas, desc->tree_reset);
> - free_pgtables(&tlb, mas, desc->first, desc->first_pgaddr,
> - desc->last_pgaddr, desc->tree_max,
> - desc->mm_wr_locked);
> + mas_set(desc->mas, desc->tree_reset);
> + free_pgtables(&tlb, desc);
> tlb_finish_mmu(&tlb);
> }
>
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index d73ad4747d40a..435c5a24480bc 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -892,17 +892,10 @@ static inline void unmap_vmas(struct mmu_gather *tlb, struct unmap_desc *unmap)
> (void)unmap;
> }
>
> -static inline void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> - struct vm_area_struct *vma, unsigned long floor,
> - unsigned long ceiling, unsigned long tree_max,
> - bool mm_wr_locked)
> +static inline void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc)
> {
> (void)tlb;
> - (void)mas;
> - (void)vma;
> - (void)floor;
> - (void)ceiling;
> - (void)mm_wr_locked;
> + (void)desc;
> }
Again, let's drop the (void)'s here. Will let Vlasta fix up the conflict I
guess ;)
>
> static inline void mapping_unmap_writable(struct address_space *)
> --
> 2.47.2
>
On Tue, Sep 9, 2025 at 12:10 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> Pass through the unmap_desc to free_pgtables() because it almost has
> everything necessary and is already on the stack.
>
> Updates testing code as necessary.
>
> No functional changes intended.
>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Reviewed-by: Suren Baghdasaryan <surenb@google.com>
> ---
> mm/internal.h | 5 +----
> mm/memory.c | 21 ++++++++++-----------
> mm/mmap.c | 6 +++---
> mm/vma.c | 7 ++-----
> tools/testing/vma/vma_internal.h | 11 ++---------
> 5 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 1239944f2830a..f22329967e908 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -445,10 +445,7 @@ bool __folio_end_writeback(struct folio *folio);
> void deactivate_file_folio(struct folio *folio);
> void folio_activate(struct folio *folio);
>
> -void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> - struct vm_area_struct *start_vma, unsigned long floor,
> - unsigned long ceiling, unsigned long tree_max,
> - bool mm_wr_locked);
> +void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc);
>
> void pmd_install(struct mm_struct *mm, pmd_t *pmd, pgtable_t *pte);
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 8d4d976311037..98c5ffd28a109 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -387,15 +387,14 @@ void free_pgd_range(struct mmu_gather *tlb,
> * The tree_max differs from the ceiling when a dup_mmap() failed and the tree
"ceiling" is not a parameter anymore, so the comments should also change.
> * has unrelated data to the mm_struct being torn down.
> */
> -void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> - struct vm_area_struct *vma, unsigned long floor,
> - unsigned long ceiling, unsigned long tree_max,
> - bool mm_wr_locked)
> +void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc)
> {
> struct unlink_vma_file_batch vb;
> + struct ma_state *mas = desc->mas;
> + struct vm_area_struct *vma = desc->first;
>
> /* underflow can happen and is fine */
> - WARN_ON_ONCE(tree_max - 1 > ceiling - 1);
> + WARN_ON_ONCE(desc->tree_max - 1 > desc->last_pgaddr - 1);
>
> tlb_free_vmas(tlb);
>
> @@ -407,13 +406,13 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> * Note: USER_PGTABLES_CEILING may be passed as ceiling and may
> * be 0. This will underflow and is okay.
> */
> - next = mas_find(mas, tree_max - 1);
> + next = mas_find(mas, desc->tree_max - 1);
>
> /*
> * Hide vma from rmap and truncate_pagecache before freeing
> * pgtables
> */
> - if (mm_wr_locked)
> + if (desc->mm_wr_locked)
> vma_start_write(vma);
> unlink_anon_vmas(vma);
>
> @@ -425,16 +424,16 @@ void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> */
> while (next && next->vm_start <= vma->vm_end + PMD_SIZE) {
> vma = next;
> - next = mas_find(mas, tree_max - 1);
> - if (mm_wr_locked)
> + next = mas_find(mas, desc->tree_max - 1);
> + if (desc->mm_wr_locked)
> vma_start_write(vma);
> unlink_anon_vmas(vma);
> unlink_file_vma_batch_add(&vb, vma);
> }
> unlink_file_vma_batch_final(&vb);
>
> - free_pgd_range(tlb, addr, vma->vm_end,
> - floor, next ? next->vm_start : ceiling);
> + free_pgd_range(tlb, addr, vma->vm_end, desc->first_pgaddr,
> + next ? next->vm_start : desc->last_pgaddr);
Much better names IMO. Thank you!
> vma = next;
> } while (vma);
> }
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 6011f62b0a294..9908481452780 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1311,10 +1311,10 @@ void exit_mmap(struct mm_struct *mm)
> */
> mm_flags_set(MMF_OOM_SKIP, mm);
> mmap_write_lock(mm);
> + unmap.mm_wr_locked = true;
> mt_clear_in_rcu(&mm->mm_mt);
> - vma_iter_set(&vmi, vma->vm_end);
> - free_pgtables(&tlb, &vmi.mas, vma, FIRST_USER_ADDRESS,
> - USER_PGTABLES_CEILING, USER_PGTABLES_CEILING, true);
> + vma_iter_set(&vmi, unmap.tree_reset);
> + free_pgtables(&tlb, &unmap);
> tlb_finish_mmu(&tlb);
>
> /*
> diff --git a/mm/vma.c b/mm/vma.c
> index ad64cd9795ef3..ba155a539d160 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -476,16 +476,13 @@ void remove_vma(struct vm_area_struct *vma)
> void unmap_region(struct unmap_desc *desc)
> {
> struct mm_struct *mm = desc->first->vm_mm;
> - struct ma_state *mas = desc->mas;
> struct mmu_gather tlb;
>
> tlb_gather_mmu(&tlb, mm);
> update_hiwater_rss(mm);
> unmap_vmas(&tlb, desc);
> - mas_set(mas, desc->tree_reset);
> - free_pgtables(&tlb, mas, desc->first, desc->first_pgaddr,
> - desc->last_pgaddr, desc->tree_max,
> - desc->mm_wr_locked);
> + mas_set(desc->mas, desc->tree_reset);
> + free_pgtables(&tlb, desc);
> tlb_finish_mmu(&tlb);
> }
>
> diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_internal.h
> index d73ad4747d40a..435c5a24480bc 100644
> --- a/tools/testing/vma/vma_internal.h
> +++ b/tools/testing/vma/vma_internal.h
> @@ -892,17 +892,10 @@ static inline void unmap_vmas(struct mmu_gather *tlb, struct unmap_desc *unmap)
> (void)unmap;
> }
>
> -static inline void free_pgtables(struct mmu_gather *tlb, struct ma_state *mas,
> - struct vm_area_struct *vma, unsigned long floor,
> - unsigned long ceiling, unsigned long tree_max,
> - bool mm_wr_locked)
> +static inline void free_pgtables(struct mmu_gather *tlb, struct unmap_desc *desc)
> {
> (void)tlb;
> - (void)mas;
> - (void)vma;
> - (void)floor;
> - (void)ceiling;
> - (void)mm_wr_locked;
> + (void)desc;
> }
>
> static inline void mapping_unmap_writable(struct address_space *)
> --
> 2.47.2
>
© 2016 - 2025 Red Hat, Inc.