| Message ID | 20230801023145.17026-3-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: hugetlb: fix mremap tlb flush | expand |
> On Aug 1, 2023, at 10:31, Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > > It is better to use huge page size instead of PAGE_SIZE > for stride when flush hugepage, which reduces the loop > in __flush_tlb_range(). > > Let's support arch's flush_hugetlb_tlb_range(), which is > used in hugetlb_unshare_all_pmds(), move_hugetlb_page_tables() > and hugetlb_change_protection() for now. > > Note, for hugepages based on contiguous bit, it has to be > invalidated individually since the contiguous PTE bit is > just a hint, the hardware may or may not take it into account. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> Reviewed-by: Muchun Song <songmuchun@bytedance.com>
On Tue, Aug 01, 2023 at 10:31:45AM +0800, Kefeng Wang wrote: > +#define __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE > +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, > + unsigned long start, > + unsigned long end) > +{ > + unsigned long stride = huge_page_size(hstate_vma(vma)); > + > + if (stride != PMD_SIZE && stride != PUD_SIZE) > + stride = PAGE_SIZE; > + __flush_tlb_range(vma, start, end, stride, false, 0); We could use some hints here for the tlb_level (2 for pmd, 1 for pud). Regarding the last_level argument to __flush_tlb_range(), I think it needs to stay false since this function is also called on the hugetlb_unshare_pmds() path where the pud is cleared and needs invalidating. That said, maybe you can rewrite it as a switch statement and call flush_pmd_tlb_range() or flush_pud_tlb_range() (just make sure these are defined when CONFIG_HUGETLBFS is enabled).
On 2023/8/1 19:09, Catalin Marinas wrote: > On Tue, Aug 01, 2023 at 10:31:45AM +0800, Kefeng Wang wrote: >> +#define __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE >> +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, >> + unsigned long start, >> + unsigned long end) >> +{ >> + unsigned long stride = huge_page_size(hstate_vma(vma)); >> + >> + if (stride != PMD_SIZE && stride != PUD_SIZE) >> + stride = PAGE_SIZE; >> + __flush_tlb_range(vma, start, end, stride, false, 0); > > We could use some hints here for the tlb_level (2 for pmd, 1 for pud). > Regarding the last_level argument to __flush_tlb_range(), I think it > needs to stay false since this function is also called on the > hugetlb_unshare_pmds() path where the pud is cleared and needs > invalidating. > > That said, maybe you can rewrite it as a switch statement and call > flush_pmd_tlb_range() or flush_pud_tlb_range() (just make sure these are > defined when CONFIG_HUGETLBFS is enabled). > How about this way, not involved with thp ？ diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h index e5c2e3dd9cf0..a7ce59d3388e 100644 --- a/arch/arm64/include/asm/hugetlb.h +++ b/arch/arm64/include/asm/hugetlb.h @@ -66,10 +66,22 @@ static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, unsigned long end) { unsigned long stride = huge_page_size(hstate_vma(vma)); + int tlb_level = 0; - if (stride != PMD_SIZE && stride != PUD_SIZE) + switch (stride) { +#ifndef __PAGETABLE_PMD_FOLDED + case PUD_SIZE: + tlb_level = 1; + break; +#endif + case PMD_SIZE: + tlb_level = 2; + break; + default: stride = PAGE_SIZE; - __flush_tlb_range(vma, start, end, stride, false, 0); + } + + __flush_tlb_range(vma, start, end, stride, false, tlb_level); }
On 2023/8/1 19:22, Kefeng Wang wrote: > > > On 2023/8/1 19:09, Catalin Marinas wrote: >> On Tue, Aug 01, 2023 at 10:31:45AM +0800, Kefeng Wang wrote: >>> +#define __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE >>> +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, >>> + unsigned long start, >>> + unsigned long end) >>> +{ >>> + unsigned long stride = huge_page_size(hstate_vma(vma)); >>> + >>> + if (stride != PMD_SIZE && stride != PUD_SIZE) >>> + stride = PAGE_SIZE; >>> + __flush_tlb_range(vma, start, end, stride, false, 0); >> >> We could use some hints here for the tlb_level (2 for pmd, 1 for pud). >> Regarding the last_level argument to __flush_tlb_range(), I think it >> needs to stay false since this function is also called on the >> hugetlb_unshare_pmds() path where the pud is cleared and needs >> invalidating. >> > That said, maybe you can rewrite it as a switch statement and call >> flush_pmd_tlb_range() or flush_pud_tlb_range() (just make sure these are >> defined when CONFIG_HUGETLBFS is enabled). I try the way your mentioned, it won't change much, will send v3, thanks.
diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h index 6a4a1ab8eb23..e5c2e3dd9cf0 100644 --- a/arch/arm64/include/asm/hugetlb.h +++ b/arch/arm64/include/asm/hugetlb.h @@ -60,4 +60,16 @@ extern void huge_ptep_modify_prot_commit(struct vm_area_struct *vma, #include <asm-generic/hugetlb.h> +#define __HAVE_ARCH_FLUSH_HUGETLB_TLB_RANGE +static inline void flush_hugetlb_tlb_range(struct vm_area_struct *vma, + unsigned long start, + unsigned long end) +{ + unsigned long stride = huge_page_size(hstate_vma(vma)); + + if (stride != PMD_SIZE && stride != PUD_SIZE) + stride = PAGE_SIZE; + __flush_tlb_range(vma, start, end, stride, false, 0); +} + #endif /* __ASM_HUGETLB_H */
It is better to use huge page size instead of PAGE_SIZE for stride when flush hugepage, which reduces the loop in __flush_tlb_range(). Let's support arch's flush_hugetlb_tlb_range(), which is used in hugetlb_unshare_all_pmds(), move_hugetlb_page_tables() and hugetlb_change_protection() for now. Note, for hugepages based on contiguous bit, it has to be invalidated individually since the contiguous PTE bit is just a hint, the hardware may or may not take it into account. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- arch/arm64/include/asm/hugetlb.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)