[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ixs4k5ynmp5mpsk67fjulvy57ih52pfpngrvsqdiz7p66x6rrn@2lgxj4e7xpp3>
Date: Wed, 14 Jan 2026 11:32:31 -0500
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, maple-tree@...ts.infradead.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
David Hildenbrand <david@...hat.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, Michal Hocko <mhocko@...e.com>,
Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
Charan Teja Kalla <quic_charante@...cinc.com>,
shikemeng@...weicloud.com, kasong@...cent.com, nphamcs@...il.com,
bhe@...hat.com, baohua@...nel.org, chrisl@...nel.org,
Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH v1 9/9] mm: Use unmap_desc struct for freeing page tables.
* Suren Baghdasaryan <surenb@...gle.com> [250909 18:27]:
> On Tue, Sep 9, 2025 at 12:10 PM Liam R. Howlett <Liam.Howlett@...cle.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@...cle.com>
>
> Reviewed-by: Suren Baghdasaryan <surenb@...gle.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.
Thanks, I'll fix it.
>
> > * 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
> >
Powered by blists - more mailing lists