| Message ID | 20200827080438.315345-11-aneesh.kumar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/debug_vm_pgtable fixes | expand |
On 08/27/2020 01:34 PM, Aneesh Kumar K.V wrote: > Make sure we call pte accessors with correct lock held. > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > --- > mm/debug_vm_pgtable.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c > index 78c8af3445ac..0a6e771ebd13 100644 > --- a/mm/debug_vm_pgtable.c > +++ b/mm/debug_vm_pgtable.c > @@ -1039,33 +1039,39 @@ static int __init debug_vm_pgtable(void) > pmd_thp_tests(pmd_aligned, prot); > pud_thp_tests(pud_aligned, prot); > > + hugetlb_basic_tests(pte_aligned, prot); > + This moved back again. As pointed out earlier, there will be a bisect problem and so this patch must be folded back with the previous one. > /* > * Page table modifying tests > */ > - pte_clear_tests(mm, ptep, vaddr); > - pmd_clear_tests(mm, pmdp); > - pud_clear_tests(mm, pudp); > - p4d_clear_tests(mm, p4dp); > - pgd_clear_tests(mm, pgdp); > > ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl); > + pte_clear_tests(mm, ptep, vaddr); > pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); > - pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep); > - pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot); > - hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); > - > + pte_unmap_unlock(ptep, ptl); > > + ptl = pmd_lock(mm, pmdp); > + pmd_clear_tests(mm, pmdp); > + pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep); > pmd_huge_tests(pmdp, pmd_aligned, prot); > + pmd_populate_tests(mm, pmdp, saved_ptep); > + spin_unlock(ptl); > + > + ptl = pud_lock(mm, pudp); > + pud_clear_tests(mm, pudp); > + pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot); > pud_huge_tests(pudp, pud_aligned, prot); > + pud_populate_tests(mm, pudp, saved_pmdp); > + spin_unlock(ptl); > > - pte_unmap_unlock(ptep, ptl); > + hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); > > - pmd_populate_tests(mm, pmdp, saved_ptep); > - pud_populate_tests(mm, pudp, saved_pmdp); > + spin_lock(&mm->page_table_lock); > + p4d_clear_tests(mm, p4dp); > + pgd_clear_tests(mm, pgdp); > p4d_populate_tests(mm, p4dp, saved_pudp); > pgd_populate_tests(mm, pgdp, saved_p4dp); > - > - hugetlb_basic_tests(pte_aligned, prot); > + spin_unlock(&mm->page_table_lock); > > p4d_free(mm, saved_p4dp); > pud_free(mm, saved_pudp); > Overall, grouping together dynamic tests at various page table levels and taking a corresponding lock, makes sense. Commit message for the resultant patch should include (a) Test classification (b) Grouping by function for the static tests, by page table level for the dynamic tests (c) Locking requirement for the dynamic tests each level etc.
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c index 78c8af3445ac..0a6e771ebd13 100644 --- a/mm/debug_vm_pgtable.c +++ b/mm/debug_vm_pgtable.c @@ -1039,33 +1039,39 @@ static int __init debug_vm_pgtable(void) pmd_thp_tests(pmd_aligned, prot); pud_thp_tests(pud_aligned, prot); + hugetlb_basic_tests(pte_aligned, prot); + /* * Page table modifying tests */ - pte_clear_tests(mm, ptep, vaddr); - pmd_clear_tests(mm, pmdp); - pud_clear_tests(mm, pudp); - p4d_clear_tests(mm, p4dp); - pgd_clear_tests(mm, pgdp); ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl); + pte_clear_tests(mm, ptep, vaddr); pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); - pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep); - pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot); - hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); - + pte_unmap_unlock(ptep, ptl); + ptl = pmd_lock(mm, pmdp); + pmd_clear_tests(mm, pmdp); + pmd_advanced_tests(mm, vma, pmdp, pmd_aligned, vaddr, prot, saved_ptep); pmd_huge_tests(pmdp, pmd_aligned, prot); + pmd_populate_tests(mm, pmdp, saved_ptep); + spin_unlock(ptl); + + ptl = pud_lock(mm, pudp); + pud_clear_tests(mm, pudp); + pud_advanced_tests(mm, vma, pudp, pud_aligned, vaddr, prot); pud_huge_tests(pudp, pud_aligned, prot); + pud_populate_tests(mm, pudp, saved_pmdp); + spin_unlock(ptl); - pte_unmap_unlock(ptep, ptl); + hugetlb_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); - pmd_populate_tests(mm, pmdp, saved_ptep); - pud_populate_tests(mm, pudp, saved_pmdp); + spin_lock(&mm->page_table_lock); + p4d_clear_tests(mm, p4dp); + pgd_clear_tests(mm, pgdp); p4d_populate_tests(mm, p4dp, saved_pudp); pgd_populate_tests(mm, pgdp, saved_p4dp); - - hugetlb_basic_tests(pte_aligned, prot); + spin_unlock(&mm->page_table_lock); p4d_free(mm, saved_p4dp); pud_free(mm, saved_pudp);
Make sure we call pte accessors with correct lock held. Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> --- mm/debug_vm_pgtable.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)