| Message ID | 20220330112543.863-1-steve.capper@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] tlb: hugetlb: Add more sizes to tlb_remove_huge_tlb_entry | expand |
On 30.03.22 13:25, Steve Capper wrote: > tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when > updating the mmu_gather structure. > > Unfortunately on arm64 there are two additional huge page sizes that > need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user > attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced > due to the fact that the tlb structure hasn't been correctly updated by > the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. > > This patch adds inequality logic to the generic implementation of > tlb_remove_huge_tlb_entry s.t. CONT_PTE_SIZE and CONT_PMD_SIZE are > effectively covered on arm64. Also, as well as ptes, pmds and puds; > p4ds are now considered too. > > Reported-by: David Hildenbrand <david@redhat.com> > Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Link: https://lore.kernel.org/linux-mm/811c5c8e-b3a2-85d2-049c-717f17c3a03a@redhat.com/ > Signed-off-by: Steve Capper <steve.capper@arm.com> > > --- > > Changed in V2: instead of doing the per-arch implementation of > tlb_remove_huge_tlb_entry we add to the generic implmentation, as > suggested by PeterZ. > > This works well on arm64 with contiguous PTEs/PMDs. Does this look > reasonable to the ppc folk? > > Cheers, > -- > Steve > --- > include/asm-generic/tlb.h | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 2c68a545ffa7..71942a1c642d 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -565,10 +565,14 @@ static inline void tlb_flush_p4d_range(struct mmu_gather *tlb, > #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ > do { \ > unsigned long _sz = huge_page_size(h); \ > - if (_sz == PMD_SIZE) \ > - tlb_flush_pmd_range(tlb, address, _sz); \ > - else if (_sz == PUD_SIZE) \ > + if (_sz >= P4D_SIZE) \ > + tlb_flush_p4d_range(tlb, address, _sz); \ > + else if (_sz >= PUD_SIZE) \ > tlb_flush_pud_range(tlb, address, _sz); \ > + else if (_sz >= PMD_SIZE) \ > + tlb_flush_pmd_range(tlb, address, _sz); \ > + else \ > + tlb_flush_pte_range(tlb, address, _sz); \ > __tlb_remove_tlb_entry(tlb, ptep, address); \ > } while (0) > LGTM Acked-by: David Hildenbrand <david@redhat.com>
On 3/30/22 16:55, Steve Capper wrote: > tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when > updating the mmu_gather structure. > > Unfortunately on arm64 there are two additional huge page sizes that > need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user > attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced > due to the fact that the tlb structure hasn't been correctly updated by > the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. > > This patch adds inequality logic to the generic implementation of > tlb_remove_huge_tlb_entry s.t. CONT_PTE_SIZE and CONT_PMD_SIZE are > effectively covered on arm64. Also, as well as ptes, pmds and puds; > p4ds are now considered too. > > Reported-by: David Hildenbrand <david@redhat.com> > Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Link: https://lore.kernel.org/linux-mm/811c5c8e-b3a2-85d2-049c-717f17c3a03a@redhat.com/ > Signed-off-by: Steve Capper <steve.capper@arm.com> > > --- > > Changed in V2: instead of doing the per-arch implementation of > tlb_remove_huge_tlb_entry we add to the generic implmentation, as > suggested by PeterZ. > > This works well on arm64 with contiguous PTEs/PMDs. Does this look > reasonable to the ppc folk? I am wondering whether this should take care of gigantic PGDIR_SIZE based HugeTLB pages as well ? Although, it will require another new helper tlb_flush_pgd_range(). Any thoughts ? > > Cheers, > -- > Steve > --- > include/asm-generic/tlb.h | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 2c68a545ffa7..71942a1c642d 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -565,10 +565,14 @@ static inline void tlb_flush_p4d_range(struct mmu_gather *tlb, > #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ > do { \ > unsigned long _sz = huge_page_size(h); \ > - if (_sz == PMD_SIZE) \ > - tlb_flush_pmd_range(tlb, address, _sz); \ > - else if (_sz == PUD_SIZE) \ > + if (_sz >= P4D_SIZE) \ > + tlb_flush_p4d_range(tlb, address, _sz); \ > + else if (_sz >= PUD_SIZE) \ > tlb_flush_pud_range(tlb, address, _sz); \ > + else if (_sz >= PMD_SIZE) \ > + tlb_flush_pmd_range(tlb, address, _sz); \ > + else \ > + tlb_flush_pte_range(tlb, address, _sz); \ > __tlb_remove_tlb_entry(tlb, ptep, address); \ > } while (0) >
On 3/30/22 16:55, Steve Capper wrote: > tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when > updating the mmu_gather structure. > > Unfortunately on arm64 there are two additional huge page sizes that > need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user > attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced > due to the fact that the tlb structure hasn't been correctly updated by > the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. > > This patch adds inequality logic to the generic implementation of > tlb_remove_huge_tlb_entry s.t. CONT_PTE_SIZE and CONT_PMD_SIZE are > effectively covered on arm64. Also, as well as ptes, pmds and puds; > p4ds are now considered too. > > Reported-by: David Hildenbrand <david@redhat.com> > Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Link: https://lore.kernel.org/linux-mm/811c5c8e-b3a2-85d2-049c-717f17c3a03a@redhat.com/ > Signed-off-by: Steve Capper <steve.capper@arm.com> Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com> > > --- > > Changed in V2: instead of doing the per-arch implementation of > tlb_remove_huge_tlb_entry we add to the generic implmentation, as > suggested by PeterZ. > > This works well on arm64 with contiguous PTEs/PMDs. Does this look > reasonable to the ppc folk? > > Cheers, > -- > Steve > --- > include/asm-generic/tlb.h | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 2c68a545ffa7..71942a1c642d 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -565,10 +565,14 @@ static inline void tlb_flush_p4d_range(struct mmu_gather *tlb, > #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ > do { \ > unsigned long _sz = huge_page_size(h); \ > - if (_sz == PMD_SIZE) \ > - tlb_flush_pmd_range(tlb, address, _sz); \ > - else if (_sz == PUD_SIZE) \ > + if (_sz >= P4D_SIZE) \ > + tlb_flush_p4d_range(tlb, address, _sz); \ > + else if (_sz >= PUD_SIZE) \ > tlb_flush_pud_range(tlb, address, _sz); \ > + else if (_sz >= PMD_SIZE) \ > + tlb_flush_pmd_range(tlb, address, _sz); \ > + else \ > + tlb_flush_pte_range(tlb, address, _sz); \ > __tlb_remove_tlb_entry(tlb, ptep, address); \ > } while (0) >
On Wed, Mar 30, 2022 at 12:25:43PM +0100, Steve Capper wrote: > tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when > updating the mmu_gather structure. > > Unfortunately on arm64 there are two additional huge page sizes that > need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user > attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced > due to the fact that the tlb structure hasn't been correctly updated by > the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. > > This patch adds inequality logic to the generic implementation of > tlb_remove_huge_tlb_entry s.t. CONT_PTE_SIZE and CONT_PMD_SIZE are > effectively covered on arm64. Also, as well as ptes, pmds and puds; > p4ds are now considered too. > > Reported-by: David Hildenbrand <david@redhat.com> > Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Link: https://lore.kernel.org/linux-mm/811c5c8e-b3a2-85d2-049c-717f17c3a03a@redhat.com/ > Signed-off-by: Steve Capper <steve.capper@arm.com> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
On Wed, Mar 30, 2022 at 07:34:16PM +0530, Anshuman Khandual wrote: > I am wondering whether this should take care of gigantic PGDIR_SIZE > based HugeTLB pages as well ? Although, it will require another new > helper tlb_flush_pgd_range(). Any thoughts ? I'm thinking that should be straight forward, you need a ->cleared_pgds along with all that, but yeah.
On Wed, Mar 30, 2022 at 12:25:43PM +0100, Steve Capper wrote: > tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when > updating the mmu_gather structure. > > Unfortunately on arm64 there are two additional huge page sizes that > need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user > attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced > due to the fact that the tlb structure hasn't been correctly updated by > the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. > > This patch adds inequality logic to the generic implementation of > tlb_remove_huge_tlb_entry s.t. CONT_PTE_SIZE and CONT_PMD_SIZE are > effectively covered on arm64. Also, as well as ptes, pmds and puds; > p4ds are now considered too. > > Reported-by: David Hildenbrand <david@redhat.com> > Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Link: https://lore.kernel.org/linux-mm/811c5c8e-b3a2-85d2-049c-717f17c3a03a@redhat.com/ > Signed-off-by: Steve Capper <steve.capper@arm.com> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > include/asm-generic/tlb.h | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h > index 2c68a545ffa7..71942a1c642d 100644 > --- a/include/asm-generic/tlb.h > +++ b/include/asm-generic/tlb.h > @@ -565,10 +565,14 @@ static inline void tlb_flush_p4d_range(struct mmu_gather *tlb, > #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ > do { \ > unsigned long _sz = huge_page_size(h); \ > - if (_sz == PMD_SIZE) \ > - tlb_flush_pmd_range(tlb, address, _sz); \ > - else if (_sz == PUD_SIZE) \ > + if (_sz >= P4D_SIZE) \ > + tlb_flush_p4d_range(tlb, address, _sz); \ > + else if (_sz >= PUD_SIZE) \ > tlb_flush_pud_range(tlb, address, _sz); \ > + else if (_sz >= PMD_SIZE) \ > + tlb_flush_pmd_range(tlb, address, _sz); \ > + else \ > + tlb_flush_pte_range(tlb, address, _sz); \ > __tlb_remove_tlb_entry(tlb, ptep, address); \ > } while (0) > > -- > 2.35.1 >
On Wed, 30 Mar 2022 12:25:43 +0100, Steve Capper wrote: > tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when > updating the mmu_gather structure. > > Unfortunately on arm64 there are two additional huge page sizes that > need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user > attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced > due to the fact that the tlb structure hasn't been correctly updated by > the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. > > [...] Applied to arm64 (for-next/fixes), thanks! [1/1] tlb: hugetlb: Add more sizes to tlb_remove_huge_tlb_entry https://git.kernel.org/arm64/c/697a1d44af8b Cheers,
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h index 2c68a545ffa7..71942a1c642d 100644 --- a/include/asm-generic/tlb.h +++ b/include/asm-generic/tlb.h @@ -565,10 +565,14 @@ static inline void tlb_flush_p4d_range(struct mmu_gather *tlb, #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ do { \ unsigned long _sz = huge_page_size(h); \ - if (_sz == PMD_SIZE) \ - tlb_flush_pmd_range(tlb, address, _sz); \ - else if (_sz == PUD_SIZE) \ + if (_sz >= P4D_SIZE) \ + tlb_flush_p4d_range(tlb, address, _sz); \ + else if (_sz >= PUD_SIZE) \ tlb_flush_pud_range(tlb, address, _sz); \ + else if (_sz >= PMD_SIZE) \ + tlb_flush_pmd_range(tlb, address, _sz); \ + else \ + tlb_flush_pte_range(tlb, address, _sz); \ __tlb_remove_tlb_entry(tlb, ptep, address); \ } while (0)
tlb_remove_huge_tlb_entry only considers PMD_SIZE and PUD_SIZE when updating the mmu_gather structure. Unfortunately on arm64 there are two additional huge page sizes that need to be covered: CONT_PTE_SIZE and CONT_PMD_SIZE. Where an end-user attempts to employ contiguous huge pages, a VM_BUG_ON can be experienced due to the fact that the tlb structure hasn't been correctly updated by the relevant tlb_flush_p.._range() call from tlb_remove_huge_tlb_entry. This patch adds inequality logic to the generic implementation of tlb_remove_huge_tlb_entry s.t. CONT_PTE_SIZE and CONT_PMD_SIZE are effectively covered on arm64. Also, as well as ptes, pmds and puds; p4ds are now considered too. Reported-by: David Hildenbrand <david@redhat.com> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Anshuman Khandual <anshuman.khandual@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Link: https://lore.kernel.org/linux-mm/811c5c8e-b3a2-85d2-049c-717f17c3a03a@redhat.com/ Signed-off-by: Steve Capper <steve.capper@arm.com> --- Changed in V2: instead of doing the per-arch implementation of tlb_remove_huge_tlb_entry we add to the generic implmentation, as suggested by PeterZ. This works well on arm64 with contiguous PTEs/PMDs. Does this look reasonable to the ppc folk? Cheers, -- Steve --- include/asm-generic/tlb.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)