Re: [RFC PATCH v2 09/47] hugetlb: make huge_pte_lockptr take an explicit shift argument.

[Mina Almasry]

On Fri, Oct 21, 2022 at 9:37 AM James Houghton <jthoughton@xxxxxxxxxx> wrote:
>
> This is needed to handle PTL locking with high-granularity mapping. We
> won't always be using the PMD-level PTL even if we're using the 2M
> hugepage hstate. It's possible that we're dealing with 4K PTEs, in which
> case, we need to lock the PTL for the 4K PTE.
>
> Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>

Reviewed-by: Mina Almasry <almasrymina@xxxxxxxxxx>

> ---
>  arch/powerpc/mm/pgtable.c | 3 ++-
>  include/linux/hugetlb.h   | 9 ++++-----
>  mm/hugetlb.c              | 7 ++++---
>  mm/migrate.c              | 3 ++-
>  4 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index cb2dcdb18f8e..035a0df47af0 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -261,7 +261,8 @@ int huge_ptep_set_access_flags(struct vm_area_struct *vma,
>
>                 psize = hstate_get_psize(h);
>  #ifdef CONFIG_DEBUG_VM
> -               assert_spin_locked(huge_pte_lockptr(h, vma->vm_mm, ptep));
> +               assert_spin_locked(huge_pte_lockptr(huge_page_shift(h),
> +                                                   vma->vm_mm, ptep));
>  #endif
>
>  #else
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 6e0c36b08a0c..db3ed6095b1c 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -934,12 +934,11 @@ static inline gfp_t htlb_modify_alloc_mask(struct hstate *h, gfp_t gfp_mask)
>         return modified_mask;
>  }
>
> -static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
> +static inline spinlock_t *huge_pte_lockptr(unsigned int shift,
>                                            struct mm_struct *mm, pte_t *pte)
>  {
> -       if (huge_page_size(h) == PMD_SIZE)
> +       if (shift == PMD_SHIFT)
>                 return pmd_lockptr(mm, (pmd_t *) pte);
> -       VM_BUG_ON(huge_page_size(h) == PAGE_SIZE);
>         return &mm->page_table_lock;
>  }
>
> @@ -1144,7 +1143,7 @@ static inline gfp_t htlb_modify_alloc_mask(struct hstate *h, gfp_t gfp_mask)
>         return 0;
>  }
>
> -static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
> +static inline spinlock_t *huge_pte_lockptr(unsigned int shift,
>                                            struct mm_struct *mm, pte_t *pte)
>  {
>         return &mm->page_table_lock;
> @@ -1206,7 +1205,7 @@ static inline spinlock_t *huge_pte_lock(struct hstate *h,
>  {
>         spinlock_t *ptl;
>
> -       ptl = huge_pte_lockptr(h, mm, pte);
> +       ptl = huge_pte_lockptr(huge_page_shift(h), mm, pte);
>         spin_lock(ptl);
>         return ptl;
>  }
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a18143add956..ef7662bd0068 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4847,7 +4847,7 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
>                 }
>
>                 dst_ptl = huge_pte_lock(h, dst, dst_pte);
> -               src_ptl = huge_pte_lockptr(h, src, src_pte);
> +               src_ptl = huge_pte_lockptr(huge_page_shift(h), src, src_pte);
>                 spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
>                 entry = huge_ptep_get(src_pte);
>  again:
> @@ -4925,7 +4925,8 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
>
>                                 /* Install the new huge page if src pte stable */
>                                 dst_ptl = huge_pte_lock(h, dst, dst_pte);
> -                               src_ptl = huge_pte_lockptr(h, src, src_pte);
> +                               src_ptl = huge_pte_lockptr(huge_page_shift(h),
> +                                                          src, src_pte);
>                                 spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
>                                 entry = huge_ptep_get(src_pte);
>                                 if (!pte_same(src_pte_old, entry)) {
> @@ -4979,7 +4980,7 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,
>         pte_t pte;
>
>         dst_ptl = huge_pte_lock(h, mm, dst_pte);
> -       src_ptl = huge_pte_lockptr(h, mm, src_pte);
> +       src_ptl = huge_pte_lockptr(huge_page_shift(h), mm, src_pte);
>
>         /*
>          * We don't have to worry about the ordering of src and dst ptlocks
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 1457cdbb7828..a0105fa6e3b2 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -334,7 +334,8 @@ void __migration_entry_wait_huge(pte_t *ptep, spinlock_t *ptl)
>
>  void migration_entry_wait_huge(struct vm_area_struct *vma, pte_t *pte)
>  {
> -       spinlock_t *ptl = huge_pte_lockptr(hstate_vma(vma), vma->vm_mm, pte);
> +       spinlock_t *ptl = huge_pte_lockptr(huge_page_shift(hstate_vma(vma)),
> +                                          vma->vm_mm, pte);
>
>         __migration_entry_wait_huge(pte, ptl);
>  }
> --
> 2.38.0.135.g90850a2211-goog
>