Implement arm64 specific huge_ptep_get()
Subsystems (2)
v2 cover Implement arm64 specific huge_ptep_get()
Hi,
As Mike pointed out [1], the huge_ptep_get() will only return one specific pte value for the CONT-PTE or CONT-PMD size hugetlb on ARM64 system, which will not take into account the subpages' dirty or young bits of a CONT-PTE/PMD size hugetlb page. That will make us miss dirty or young flags of a CONT-PTE/PMD size hugetlb page for those functions that want to check the dirty or young flags of a hugetlb page. For example, the gather_hugetlb_stats() will get inaccurate dirty hugetlb page statistics, and the DAMON for hugetlb monitoring will also get inaccurate access statistics.
To fix this issue, this patch set introduces an ARM64 specific huge_ptep_get() implementation, which will take into account any subpages' dirty or young bits.
[1] https://lore.kernel.org/linux-mm/[email protected]/
Changes from v1:
- Rebase on next-20220512.
- Update commit message suggested by Muchun.
- Add reviewed tag from Muchun.
Changes from RFC:
- Implement arm64 specific huge_ptep_get() instead of introducing a new interface.
- Add a new patch to convert huge_ptep_get() in hugetlbpage.c
Baolin Wang (2):
arm64/hugetlb: Use ptep_get() to get the pte value of a huge page arm64/hugetlb: Implement arm64 specific huge_ptep_get()
arch/arm64/include/asm/hugetlb.h | 2 ++ arch/arm64/mm/hugetlbpage.c | 32 ++++++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-)
Signature · 1 line
1.8.3.1
PATCH v2 2/2 arm64/hugetlb: Implement arm64 specific huge_ptep_get()
Now we use huge_ptep_get() to get the pte value of a hugetlb page, however it will only return one specific pte value for the CONT-PTE or CONT-PMD size hugetlb on ARM64 system, which can contain seravel continuous pte or pmd entries with same page table attributes. And it will not take into account the subpages' dirty or young bits of a CONT-PTE/PMD size hugetlb page.
So the huge_ptep_get() is inconsistent with huge_ptep_get_and_clear(), which already takes account the dirty or young bits for any subpages in this CONT-PTE/PMD size hugetlb [1]. Meanwhile we can miss dirty or young flags statistics for hugetlb pages with current huge_ptep_get(), such as the gather_hugetlb_stats() function, and CONT-PTE/PMD hugetlb monitoring with DAMON.
Thus define an ARM64 specific huge_ptep_get() implementation, that will take into account any subpages' dirty or young bits for CONT-PTE/PMD size hugetlb page, for those functions that want to check the dirty and young flags of a hugetlb page.
[1] https://lore.kernel.org/linux-mm/[email protected]/
--- arch/arm64/include/asm/hugetlb.h | 2 ++ arch/arm64/mm/hugetlbpage.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+)
arch/arm64/include/asm/hugetlb.h +2 -0
@@ -44,6 +44,8 @@ extern pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,#define __HAVE_ARCH_HUGE_PTE_CLEARextern void huge_pte_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep,unsigned long sz); +#define __HAVE_ARCH_HUGE_PTEP_GET +extern pte_t huge_ptep_get (pte_t *ptep);extern void set_huge_swap_pte_at (struct mm_struct *mm,unsigned long addr,pte_t *ptep,pte_t pte,unsigned long sz);#define set_huge_swap_pte_at set_huge_swap_pte_at
arch/arm64/mm/hugetlbpage.c +24 -0
@@ -158,6 +158,30 @@ static inline int num_contig_ptes(unsigned long size, size_t *pgsize)return contig_ptes; } +pte_t huge_ptep_get (pte_t *ptep) +{ +int ncontig, i; +size_t pgsize; +pte_t orig_pte =ptep_get (ptep); + +if (!pte_present (orig_pte) || !pte_cont (orig_pte)) +return orig_pte; + + ncontig =num_contig_ptes (page_size (pte_page (orig_pte)), &pgsize); + +for (i =0 ; i < ncontig; i++, ptep++) { +pte_t pte =ptep_get (ptep); + +if (pte_dirty (pte)) + orig_pte =pte_mkdirty (orig_pte); + +if (pte_young (pte)) + orig_pte =pte_mkyoung (orig_pte); + } + +return orig_pte; +} +/* * Changing some bits of contiguous entries requires us to follow a * Break-Before-Make approach, breaking the whole contiguous set
Signature · 1 line
1.8.3.1
PATCH v2 1/2 arm64/hugetlb: Use ptep_get() to get the pte value of a huge page
In reply to Baolin WangThe original huge_ptep_get() on ARM64 is just a wrapper of ptep_get(), which will not take into account any contig-PTEs dirty and access bits. Meanwhile we will implement a new ARM64-specific huge_ptep_get() interface in following patch, which will take into account any contig-PTEs dirty and access bits. To keep the same efficient logics to get the pte value, change to use ptep_get() as a preparation.
--- arch/arm64/mm/hugetlbpage.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/mm/hugetlbpage.c +4 -4
@@ -172,7 +172,7 @@ static pte_t get_clear_contig(struct mm_struct *mm, unsigned long pgsize, unsigned long ncontig) { -pte_t orig_pte =huge_ptep_get (ptep); +pte_t orig_pte =ptep_get (ptep);unsigned long i;for (i =0 ; i < ncontig; i++, addr += pgsize, ptep++) {
@@ -379,7 +379,7 @@ pte_t huge_ptep_get_and_clear(struct mm_struct *mm, {int ncontig;size_t pgsize; -pte_t orig_pte =huge_ptep_get (ptep); +pte_t orig_pte =ptep_get (ptep);if (!pte_cont (orig_pte))return ptep_get_and_clear (mm, addr, ptep);
@@ -402,11 +402,11 @@ static int __cont_access_flags_changed(pte_t *ptep, pte_t pte, int ncontig) {int i; -if (pte_write (pte) !=pte_write (huge_ptep_get (ptep))) +if (pte_write (pte) !=pte_write (ptep_get (ptep)))return 1 ;for (i =0 ; i < ncontig; i++) { -pte_t orig_pte =huge_ptep_get (ptep + i); +pte_t orig_pte =ptep_get (ptep + i);if (pte_dirty (pte) !=pte_dirty (orig_pte))return 1 ;
Signature · 1 line
1.8.3.1
Re: [PATCH v2 1/2] arm64/hugetlb: Use ptep_get() to get the pte value of a huge page
Quoted 1 line · Baolin Wang
The original
huge_ptep_get()on ARM64 is just a wrapper ofptep_get(),
Right, as arm64 does not enable __HAVE_ARCH_HUGE_PTEP_GET.
Quoted 1 line
which will not take into account any contig-PTEs dirty and access bits.
As expected being operating on a single table entry.
Quoted 3 lines
Meanwhile we will implement a new ARM64-specific
huge_ptep_get()interface in following patch, which will take into account any contig-PTEs dirty and access bits. To keep the same efficient logics to get the pte
s/logics/logic
Quoted 4 lines
value, change to use
ptep_get()as a preparation.Signed-off-by Baolin Wang <[email protected]>Reviewed-by Muchun Song <[email protected]>
LGTM
Quoted 40 lines
--- arch/arm64/mm/hugetlbpage.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/mm/hugetlbpage.c +4
-4@@ -172,7 +172,7 @@ static pte_t get_clear_contig(struct mm_struct *mm, unsigned long pgsize, unsigned long ncontig) { -pte_t orig_pte =huge_ptep_get (ptep); +pte_t orig_pte =ptep_get (ptep);unsigned long i;for (i =0 ; i < ncontig; i++, addr += pgsize, ptep++) {@@ -379,7 +379,7 @@ pte_t huge_ptep_get_and_clear(struct mm_struct *mm, {int ncontig;size_t pgsize; -pte_t orig_pte =huge_ptep_get (ptep); +pte_t orig_pte =ptep_get (ptep);if (!pte_cont (orig_pte))return ptep_get_and_clear (mm, addr, ptep);@@ -402,11 +402,11 @@ static int __cont_access_flags_changed(pte_t *ptep, pte_t pte, int ncontig) {int i; -if (pte_write (pte) !=pte_write (huge_ptep_get (ptep))) +if (pte_write (pte) !=pte_write (ptep_get (ptep)))return 1 ;for (i =0 ; i < ncontig; i++) { -pte_t orig_pte =huge_ptep_get (ptep + i); +pte_t orig_pte =ptep_get (ptep + i);if (pte_dirty (pte) !=pte_dirty (orig_pte))return 1 ;
Re: [PATCH v2 2/2] arm64/hugetlb: Implement arm64 specific huge_ptep_get()
In reply to Baolin WangQuoted 3 lines · Baolin Wang
Now we use
huge_ptep_get()to get the pte value of a hugetlb page, however it will only return one specific pte value for the CONT-PTE or CONT-PMD size hugetlb on ARM64 system, which can contain seravel
A small nit.
s/seravel/several
Quoted 11 lines
continuous pte or pmd entries with same page table attributes. And it will not take into account the subpages' dirty or young bits of a CONT-PTE/PMD size hugetlb page.
So the
huge_ptep_get()is inconsistent withhuge_ptep_get_and_clear(), which already takes account the dirty or young bits for any subpages in this CONT-PTE/PMD size hugetlb [1]. Meanwhile we can miss dirty or young flags statistics for hugetlb pages with currenthuge_ptep_get(), such as thegather_hugetlb_stats()function, and CONT-PTE/PMD hugetlb monitoring with DAMON.
Quoted 6 lines
Thus define an ARM64 specific
huge_ptep_get()implementation, that will take into account any subpages' dirty or young bits for CONT-PTE/PMD size hugetlb page, for those functions that want to check the dirty and young flags of a hugetlb page.
Might be worth mentioning that arm64 now enables __HAVE_ARCH_HUGE_PTEP_GET.
Quoted 30 lines
Suggested-by Muchun Song <[email protected]>Signed-off-by Baolin Wang <[email protected]>Reviewed-by Muchun Song <[email protected]>--- arch/arm64/include/asm/hugetlb.h | 2 ++ arch/arm64/mm/hugetlbpage.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+)arch/arm64/include/asm/hugetlb.h +2
-0@@ -44,6 +44,8 @@ extern pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,#define __HAVE_ARCH_HUGE_PTE_CLEARextern void huge_pte_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep,unsigned long sz); +#define __HAVE_ARCH_HUGE_PTEP_GET +extern pte_t huge_ptep_get (pte_t *ptep);extern void set_huge_swap_pte_at (struct mm_struct *mm,unsigned long addr,pte_t *ptep,pte_t pte,unsigned long sz);#define set_huge_swap_pte_at set_huge_swap_pte_atarch/arm64/mm/hugetlbpage.c +0
-0@@ -158,6 +158,30 @@ static inline int num_contig_ptes(unsigned long size, size_t *pgsize)return contig_ptes; }
arch/arm64/mm/hugetlbpage.c static inline int num_contig_ptes(unsigned long size, size_t *pgsize) · line 170
Quoted 10 lines
arch/arm64/mm/hugetlbpage.c +10
-0+pte_t huge_ptep_get (pte_t *ptep) +{ +int ncontig, i; +size_t pgsize; +pte_t orig_pte =ptep_get (ptep); + +if (!pte_present (orig_pte) || !pte_cont (orig_pte)) +return orig_pte; + + ncontig =num_contig_ptes (page_size (pte_page (orig_pte)), &pgsize);
Hmm, I guess there is no better way of deriving page size here.
Please drop the extra line here.
arch/arm64/mm/hugetlbpage.c static inline int num_contig_ptes(unsigned long size, size_t *pgsize) · line 180
Quoted 10 lines
arch/arm64/mm/hugetlbpage.c +10
-0+ +for (i =0 ; i < ncontig; i++, ptep++) { +pte_t pte =ptep_get (ptep); + +if (pte_dirty (pte)) + orig_pte =pte_mkdirty (orig_pte); + +if (pte_young (pte)) + orig_pte =pte_mkyoung (orig_pte); + }
Please drop the extra line here.
arch/arm64/mm/hugetlbpage.c static inline int num_contig_ptes(unsigned long size, size_t *pgsize) · line 187
Quoted 7 lines
arch/arm64/mm/hugetlbpage.c +4
-0+ +return orig_pte; +} +/* * Changing some bits of contiguous entries requires us to follow a * Break-Before-Make approach, breaking the whole contiguous set
Otherwise LGTM.
With those small changes accommodated,
Re: [PATCH v2 1/2] arm64/hugetlb: Use ptep_get() to get the pte value of a huge page
In reply to Anshuman KhandualQuoted 9 lines · Anshuman Khandual
Quoted 1 line · Baolin Wang
The original
huge_ptep_get()on ARM64 is just a wrapper ofptep_get(),Right, as arm64 does not enable
__HAVE_ARCH_HUGE_PTEP_GET.Quoted 1 line
which will not take into account any contig-PTEs dirty and access bits.
Quoted 7 lines
As expected being operating on a single table entry.
Quoted 3 lines
Meanwhile we will implement a new ARM64-specific
huge_ptep_get()interface in following patch, which will take into account any contig-PTEs dirty and access bits. To keep the same efficient logics to get the ptes/logics/logic
Will fix.
Quoted 9 lines
Quoted 5 lines
value, change to use
ptep_get()as a preparation.Signed-off-by Baolin Wang <[email protected]>Reviewed-by Muchun Song <[email protected]>LGTM
Reviewed-by Anshuman Khandual <[email protected]>
Thanks.
Re: [PATCH v2 2/2] arm64/hugetlb: Implement arm64 specific huge_ptep_get()
In reply to Anshuman KhandualQuoted 10 lines · Anshuman Khandual
Quoted 3 lines · Baolin Wang
Now we use
huge_ptep_get()to get the pte value of a hugetlb page, however it will only return one specific pte value for the CONT-PTE or CONT-PMD size hugetlb on ARM64 system, which can contain seravelA small nit.
s/seravel/several
Will fix.
Quoted 12 lines
Quoted 12 lines
continuous pte or pmd entries with same page table attributes. And it will not take into account the subpages' dirty or young bits of a CONT-PTE/PMD size hugetlb page.
So the
huge_ptep_get()is inconsistent withhuge_ptep_get_and_clear(), which already takes account the dirty or young bits for any subpages in this CONT-PTE/PMD size hugetlb [1]. Meanwhile we can miss dirty or young flags statistics for hugetlb pages with currenthuge_ptep_get(), such as thegather_hugetlb_stats()function, and CONT-PTE/PMD hugetlb monitoring with DAMON.
Quoted 8 lines
Quoted 6 lines
Thus define an ARM64 specific
huge_ptep_get()implementation, that will take into account any subpages' dirty or young bits for CONT-PTE/PMD size hugetlb page, for those functions that want to check the dirty and young flags of a hugetlb page.Might be worth mentioning that arm64 now enables
__HAVE_ARCH_HUGE_PTEP_GET.
Sure.
Quoted 44 lines
Quoted 31 lines
Suggested-by Muchun Song <[email protected]>Signed-off-by Baolin Wang <[email protected]>Reviewed-by Muchun Song <[email protected]>--- arch/arm64/include/asm/hugetlb.h | 2 ++ arch/arm64/mm/hugetlbpage.c | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+)arch/arm64/include/asm/hugetlb.h +2
-0@@ -44,6 +44,8 @@ extern pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,#define __HAVE_ARCH_HUGE_PTE_CLEARextern void huge_pte_clear (struct mm_struct *mm,unsigned long addr,pte_t *ptep,unsigned long sz); +#define __HAVE_ARCH_HUGE_PTEP_GET +extern pte_t huge_ptep_get (pte_t *ptep);extern void set_huge_swap_pte_at (struct mm_struct *mm,unsigned long addr,pte_t *ptep,pte_t pte,unsigned long sz);#define set_huge_swap_pte_at set_huge_swap_pte_atarch/arm64/mm/hugetlbpage.c +0
-0@@ -158,6 +158,30 @@ static inline int num_contig_ptes(unsigned long size, size_t *pgsize)return contig_ptes; }arch/arm64/mm/hugetlbpage.c static inline int num_contig_ptes(unsigned long size, size_t *pgsize) · line 170
Quoted 10 lines
arch/arm64/mm/hugetlbpage.c +10
-0+pte_t huge_ptep_get (pte_t *ptep) +{ +int ncontig, i; +size_t pgsize; +pte_t orig_pte =ptep_get (ptep); + +if (!pte_present (orig_pte) || !pte_cont (orig_pte)) +return orig_pte; + + ncontig =num_contig_ptes (page_size (pte_page (orig_pte)), &pgsize);Hmm, I guess there is no better way of deriving page size here.
Quoted 1 line
Please drop the extra line here.
OK.
Quoted 13 lines
arch/arm64/mm/hugetlbpage.c static inline int num_contig_ptes(unsigned long size, size_t *pgsize) · line 180
Quoted 11 lines
arch/arm64/mm/hugetlbpage.c +10
-0+ +for (i =0 ; i < ncontig; i++, ptep++) { +pte_t pte =ptep_get (ptep); + +if (pte_dirty (pte)) + orig_pte =pte_mkdirty (orig_pte); + +if (pte_young (pte)) + orig_pte =pte_mkyoung (orig_pte); + }Please drop the extra line here.
Sure.
Quoted 10 lines
arch/arm64/mm/hugetlbpage.c static inline int num_contig_ptes(unsigned long size, size_t *pgsize) · line 187
Quoted 8 lines
arch/arm64/mm/hugetlbpage.c +4
-0+ +return orig_pte; +} +/* * Changing some bits of contiguous entries requires us to follow a * Break-Before-Make approach, breaking the whole contiguous set Otherwise LGTM.
Quoted 3 lines
With those small changes accommodated,
Reviewed-by Anshuman Khandual <[email protected]>
Thanks for reviewing.