2022-10-21 17:22:24

by James Houghton

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

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 <[email protected]>
---
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


2022-12-08 01:14:12

by Mina Almasry

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

On Fri, Oct 21, 2022 at 9:37 AM James Houghton <[email protected]> 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 <[email protected]>

Reviewed-by: Mina Almasry <[email protected]>

> ---
> 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
>

2022-12-13 00:37:19

by Mike Kravetz

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

On 10/21/22 16:36, James Houghton 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 <[email protected]>
> ---
> 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(-)

Straight forward substitution,

Acked-by: Mike Kravetz <[email protected]>

--
Mike Kravetz