linux-kernel - Re: [RFC PATCH 06/26] mm: make free_p?d

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADrL8HX0rRY+2PWQHDSvL4yTqmyQ-tv9ouJXScKbUKDuC1yh2g@mail.gmail.com>
Date:   Tue, 12 Jul 2022 13:52:08 -0700
From:   James Houghton <jthoughton@...gle.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     Muchun Song <songmuchun@...edance.com>,
        Peter Xu <peterx@...hat.com>,
        David Hildenbrand <david@...hat.com>,
        David Rientjes <rientjes@...gle.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Mina Almasry <almasrymina@...gle.com>,
        Jue Wang <juew@...gle.com>,
        Manish Mishra <manish.mishra@...anix.com>,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 06/26] mm: make free_p?d_range functions public

On Tue, Jun 28, 2022 at 1:35 PM Mike Kravetz <mike.kravetz@...cle.com> wrote:
>
> On 06/24/22 17:36, James Houghton wrote:
> > This makes them usable for HugeTLB page table freeing operations.
> > After HugeTLB high-granularity mapping, the page table for a HugeTLB VMA
> > can get more complex, and these functions handle freeing page tables
> > generally.
> >
>
> Hmmmm?
>
> free_pgd_range is not generally called directly for hugetlb mappings.
> There is a wrapper hugetlb_free_pgd_range which can have architecture
> specific implementations.  It makes me wonder if these lower level
> routines can be directly used on hugetlb mappings.  My 'guess' is that any
> such details will be hidden in the callers.  Suspect this will become clear
> in later patches.

Thanks for pointing out hugetlb_free_pgd_range. I think I'll need to
change how freeing HugeTLB HGM PTEs is written, because as written, we
don't do any architecture-specific things. I think I have a good idea
for what I need to do, probably something like this: make
`hugetlb_free_range` overridable, and then provide an implementation
for it for all the architectures that
HAVE_ARCH_HUGETLB_FREE_PGD_RANGE. Making the regular `free_p?d_range`
functions public *does* help with implementing the regular/general
`hugetlb_free_range` function though, so I think this commit is still
useful.

- James

> --
> Mike Kravetz
>
> > Signed-off-by: James Houghton <jthoughton@...gle.com>
> > ---
> >  include/linux/mm.h | 7 +++++++
> >  mm/memory.c        | 8 ++++----
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index bc8f326be0ce..07f5da512147 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1847,6 +1847,13 @@ void unmap_vmas(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
> >
> >  struct mmu_notifier_range;
> >
> > +void free_pte_range(struct mmu_gather *tlb, pmd_t *pmd, unsigned long addr);
> > +void free_pmd_range(struct mmu_gather *tlb, pud_t *pud, unsigned long addr,
> > +             unsigned long end, unsigned long floor, unsigned long ceiling);
> > +void free_pud_range(struct mmu_gather *tlb, p4d_t *p4d, unsigned long addr,
> > +             unsigned long end, unsigned long floor, unsigned long ceiling);
> > +void free_p4d_range(struct mmu_gather *tlb, pgd_t *pgd, unsigned long addr,
> > +             unsigned long end, unsigned long floor, unsigned long ceiling);
> >  void free_pgd_range(struct mmu_gather *tlb, unsigned long addr,
> >               unsigned long end, unsigned long floor, unsigned long ceiling);
> >  int
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 7a089145cad4..bb3b9b5b94fb 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -227,7 +227,7 @@ static void check_sync_rss_stat(struct task_struct *task)
> >   * Note: this doesn't free the actual pages themselves. That
> >   * has been handled earlier when unmapping all the memory regions.
> >   */
> > -static void free_pte_range(struct mmu_gather *tlb, pmd_t *pmd,
> > +void free_pte_range(struct mmu_gather *tlb, pmd_t *pmd,
> >                          unsigned long addr)
> >  {
> >       pgtable_t token = pmd_pgtable(*pmd);
> > @@ -236,7 +236,7 @@ static void free_pte_range(struct mmu_gather *tlb, pmd_t *pmd,
> >       mm_dec_nr_ptes(tlb->mm);
> >  }
> >
> > -static inline void free_pmd_range(struct mmu_gather *tlb, pud_t *pud,
> > +inline void free_pmd_range(struct mmu_gather *tlb, pud_t *pud,
> >                               unsigned long addr, unsigned long end,
> >                               unsigned long floor, unsigned long ceiling)
> >  {
> > @@ -270,7 +270,7 @@ static inline void free_pmd_range(struct mmu_gather *tlb, pud_t *pud,
> >       mm_dec_nr_pmds(tlb->mm);
> >  }
> >
> > -static inline void free_pud_range(struct mmu_gather *tlb, p4d_t *p4d,
> > +inline void free_pud_range(struct mmu_gather *tlb, p4d_t *p4d,
> >                               unsigned long addr, unsigned long end,
> >                               unsigned long floor, unsigned long ceiling)
> >  {
> > @@ -304,7 +304,7 @@ static inline void free_pud_range(struct mmu_gather *tlb, p4d_t *p4d,
> >       mm_dec_nr_puds(tlb->mm);
> >  }
> >
> > -static inline void free_p4d_range(struct mmu_gather *tlb, pgd_t *pgd,
> > +inline void free_p4d_range(struct mmu_gather *tlb, pgd_t *pgd,
> >                               unsigned long addr, unsigned long end,
> >                               unsigned long floor, unsigned long ceiling)
> >  {
> > --
> > 2.37.0.rc0.161.g10f37bed90-goog
> >