| Message ID | 20210719130613.334901-9-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/debug_vm_pgtable: Enhancements | expand |
On 7/19/21 6:36 PM, Gavin Shan wrote: > This uses struct pgtable_debug_args in PMD modifying tests. The allocated > huge page is used when set_pmd_at() is used. The corresponding tests > are skipped if the huge page doesn't exist. Besides, the unused variable > @pmd_aligned in debug_vm_pgtable() is dropped. Please dont drop @pmd_aligned just yet. > > Signed-off-by: Gavin Shan <gshan@redhat.com> > --- > mm/debug_vm_pgtable.c | 102 ++++++++++++++++++++---------------------- > 1 file changed, 48 insertions(+), 54 deletions(-) > > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c > index eb6dda88e0d9..cec3cbf99a6b 100644 > --- a/mm/debug_vm_pgtable.c > +++ b/mm/debug_vm_pgtable.c > @@ -213,54 +213,54 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) > WARN_ON(!pmd_bad(pmd_mkhuge(pmd))); > } > > -static void __init pmd_advanced_tests(struct mm_struct *mm, > - struct vm_area_struct *vma, pmd_t *pmdp, > - unsigned long pfn, unsigned long vaddr, > - pgprot_t prot, pgtable_t pgtable) > +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) > { > pmd_t pmd; > + unsigned long vaddr = (args->vaddr & HPAGE_PMD_MASK); > > if (!has_transparent_hugepage()) > return; > > pr_debug("Validating PMD advanced\n"); > - /* Align the address wrt HPAGE_PMD_SIZE */ > - vaddr &= HPAGE_PMD_MASK; Please just leave these unchanged. If has_transparent_hugepage() evaluates negative, it skips the masking operation. As mentioned earlier please avoid changing the test in any manner during these transition patches. > + if (args->pmd_pfn == ULONG_MAX) { > + pr_debug("%s: Skipped\n", __func__); > + return; > + } Just return. Please dont call out "Skipped". > > - pgtable_trans_huge_deposit(mm, pmdp, pgtable); > + pgtable_trans_huge_deposit(args->mm, args->pmdp, args->start_ptep); > > - pmd = pfn_pmd(pfn, prot); > - set_pmd_at(mm, vaddr, pmdp, pmd); > - pmdp_set_wrprotect(mm, vaddr, pmdp); > - pmd = READ_ONCE(*pmdp); > + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); > + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); > + pmdp_set_wrprotect(args->mm, vaddr, args->pmdp); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(pmd_write(pmd)); > - pmdp_huge_get_and_clear(mm, vaddr, pmdp); > - pmd = READ_ONCE(*pmdp); > + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(!pmd_none(pmd)); > > - pmd = pfn_pmd(pfn, prot); > + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); > pmd = pmd_wrprotect(pmd); > pmd = pmd_mkclean(pmd); > - set_pmd_at(mm, vaddr, pmdp, pmd); > + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); > pmd = pmd_mkwrite(pmd); > pmd = pmd_mkdirty(pmd); > - pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1); > - pmd = READ_ONCE(*pmdp); > + pmdp_set_access_flags(args->vma, vaddr, args->pmdp, pmd, 1); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd))); > - pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1); > - pmd = READ_ONCE(*pmdp); > + pmdp_huge_get_and_clear_full(args->vma, vaddr, args->pmdp, 1); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(!pmd_none(pmd)); > > - pmd = pmd_mkhuge(pfn_pmd(pfn, prot)); > + pmd = pmd_mkhuge(pfn_pmd(args->pmd_pfn, args->page_prot)); > pmd = pmd_mkyoung(pmd); > - set_pmd_at(mm, vaddr, pmdp, pmd); > - pmdp_test_and_clear_young(vma, vaddr, pmdp); > - pmd = READ_ONCE(*pmdp); > + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); > + pmdp_test_and_clear_young(args->vma, vaddr, args->pmdp); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(pmd_young(pmd)); > > /* Clear the pte entries */ > - pmdp_huge_get_and_clear(mm, vaddr, pmdp); > - pgtable = pgtable_trans_huge_withdraw(mm, pmdp); > + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); > + pgtable_trans_huge_withdraw(args->mm, args->pmdp); > } > > static void __init pmd_leaf_tests(struct pgtable_debug_args *args) > @@ -417,12 +417,7 @@ static void __init pud_leaf_tests(struct pgtable_debug_args *args) { } > #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ > static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) { } > static void __init pud_basic_tests(struct pgtable_debug_args *args, int idx) { } > -static void __init pmd_advanced_tests(struct mm_struct *mm, > - struct vm_area_struct *vma, pmd_t *pmdp, > - unsigned long pfn, unsigned long vaddr, > - pgprot_t prot, pgtable_t pgtable) > -{ > -} > +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { } > static void __init pud_advanced_tests(struct mm_struct *mm, > struct vm_area_struct *vma, pud_t *pudp, > unsigned long pfn, unsigned long vaddr, > @@ -435,11 +430,11 @@ static void __init pmd_savedwrite_tests(struct pgtable_debug_args *args) { } > #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ > > #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP > -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) > +static void __init pmd_huge_tests(struct pgtable_debug_args *args) > { > pmd_t pmd; > > - if (!arch_vmap_pmd_supported(prot)) > + if (!arch_vmap_pmd_supported(args->page_prot)) > return; > > pr_debug("Validating PMD huge\n"); > @@ -447,10 +442,11 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) > * X86 defined pmd_set_huge() verifies that the given > * PMD is not a populated non-leaf entry. > */ > - WRITE_ONCE(*pmdp, __pmd(0)); > - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot)); > - WARN_ON(!pmd_clear_huge(pmdp)); > - pmd = READ_ONCE(*pmdp); > + WRITE_ONCE(*(args->pmdp), __pmd(0)); Possible extra braces. > + WARN_ON(!pmd_set_huge(args->pmdp, __pfn_to_phys(args->fixed_pmd_pfn), > + args->page_prot)); Dont break the line. > + WARN_ON(!pmd_clear_huge(args->pmdp)); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(!pmd_none(pmd)); > } > > @@ -473,7 +469,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) > WARN_ON(!pud_none(pud)); > } > #else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ > -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { } > +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { } > static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { } > #endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */ > > @@ -640,20 +636,19 @@ static void __init pte_clear_tests(struct pgtable_debug_args *args) > WARN_ON(!pte_none(pte)); > } > > -static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp) > +static void __init pmd_clear_tests(struct pgtable_debug_args *args) > { > - pmd_t pmd = READ_ONCE(*pmdp); > + pmd_t pmd = READ_ONCE(*(args->pmdp)); > > pr_debug("Validating PMD clear\n"); > pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE); > - WRITE_ONCE(*pmdp, pmd); > - pmd_clear(pmdp); > - pmd = READ_ONCE(*pmdp); > + WRITE_ONCE(*(args->pmdp), pmd); > + pmd_clear(args->pmdp); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(!pmd_none(pmd)); > } > > -static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, > - pgtable_t pgtable) > +static void __init pmd_populate_tests(struct pgtable_debug_args *args) > { > pmd_t pmd; > > @@ -662,8 +657,8 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, > * This entry points to next level page table page. > * Hence this must not qualify as pmd_bad(). > */ > - pmd_populate(mm, pmdp, pgtable); > - pmd = READ_ONCE(*pmdp); > + pmd_populate(args->mm, args->pmdp, args->start_ptep); > + pmd = READ_ONCE(*(args->pmdp)); > WARN_ON(pmd_bad(pmd)); > } > > @@ -1159,7 +1154,7 @@ static int __init debug_vm_pgtable(void) > pgtable_t saved_ptep; > pgprot_t prot; > phys_addr_t paddr; > - unsigned long vaddr, pmd_aligned; > + unsigned long vaddr; > unsigned long pud_aligned; > spinlock_t *ptl = NULL; > int idx, ret; > @@ -1194,7 +1189,6 @@ static int __init debug_vm_pgtable(void) > */ > paddr = __pa_symbol(&start_kernel); > > - pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; Please dont drop these just yet and wait until [PATCH 11/12]. > pud_aligned = (paddr & PUD_MASK) >> PAGE_SHIFT; > > pgdp = pgd_offset(mm, vaddr); > @@ -1281,11 +1275,11 @@ static int __init debug_vm_pgtable(void) > pte_advanced_tests(&args); > spin_unlock(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); > + ptl = pmd_lock(args.mm, args.pmdp); > + pmd_clear_tests(&args); > + pmd_advanced_tests(&args); > + pmd_huge_tests(&args); > + pmd_populate_tests(&args); > spin_unlock(ptl); > > ptl = pud_lock(mm, pudp); >
Hi Anshuman, On 7/22/21 3:45 PM, Anshuman Khandual wrote: > On 7/19/21 6:36 PM, Gavin Shan wrote: >> This uses struct pgtable_debug_args in PMD modifying tests. The allocated >> huge page is used when set_pmd_at() is used. The corresponding tests >> are skipped if the huge page doesn't exist. Besides, the unused variable >> @pmd_aligned in debug_vm_pgtable() is dropped. > > Please dont drop @pmd_aligned just yet. > We need do so. Otherwise, there is build warning to complain something like 'unused variable' after this patch is applied. >> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> mm/debug_vm_pgtable.c | 102 ++++++++++++++++++++---------------------- >> 1 file changed, 48 insertions(+), 54 deletions(-) >> >> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >> index eb6dda88e0d9..cec3cbf99a6b 100644 >> --- a/mm/debug_vm_pgtable.c >> +++ b/mm/debug_vm_pgtable.c >> @@ -213,54 +213,54 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) >> WARN_ON(!pmd_bad(pmd_mkhuge(pmd))); >> } >> >> -static void __init pmd_advanced_tests(struct mm_struct *mm, >> - struct vm_area_struct *vma, pmd_t *pmdp, >> - unsigned long pfn, unsigned long vaddr, >> - pgprot_t prot, pgtable_t pgtable) >> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) >> { >> pmd_t pmd; >> + unsigned long vaddr = (args->vaddr & HPAGE_PMD_MASK); >> >> if (!has_transparent_hugepage()) >> return; >> >> pr_debug("Validating PMD advanced\n"); >> - /* Align the address wrt HPAGE_PMD_SIZE */ >> - vaddr &= HPAGE_PMD_MASK; > > Please just leave these unchanged. If has_transparent_hugepage() evaluates > negative, it skips the masking operation. As mentioned earlier please avoid > changing the test in any manner during these transition patches. > Ok. >> + if (args->pmd_pfn == ULONG_MAX) { >> + pr_debug("%s: Skipped\n", __func__); >> + return; >> + } > > Just return. Please dont call out "Skipped". > Ok. >> >> - pgtable_trans_huge_deposit(mm, pmdp, pgtable); >> + pgtable_trans_huge_deposit(args->mm, args->pmdp, args->start_ptep); >> >> - pmd = pfn_pmd(pfn, prot); >> - set_pmd_at(mm, vaddr, pmdp, pmd); >> - pmdp_set_wrprotect(mm, vaddr, pmdp); >> - pmd = READ_ONCE(*pmdp); >> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >> + pmdp_set_wrprotect(args->mm, vaddr, args->pmdp); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(pmd_write(pmd)); >> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >> - pmd = READ_ONCE(*pmdp); >> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(!pmd_none(pmd)); >> >> - pmd = pfn_pmd(pfn, prot); >> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >> pmd = pmd_wrprotect(pmd); >> pmd = pmd_mkclean(pmd); >> - set_pmd_at(mm, vaddr, pmdp, pmd); >> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >> pmd = pmd_mkwrite(pmd); >> pmd = pmd_mkdirty(pmd); >> - pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1); >> - pmd = READ_ONCE(*pmdp); >> + pmdp_set_access_flags(args->vma, vaddr, args->pmdp, pmd, 1); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd))); >> - pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1); >> - pmd = READ_ONCE(*pmdp); >> + pmdp_huge_get_and_clear_full(args->vma, vaddr, args->pmdp, 1); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(!pmd_none(pmd)); >> >> - pmd = pmd_mkhuge(pfn_pmd(pfn, prot)); >> + pmd = pmd_mkhuge(pfn_pmd(args->pmd_pfn, args->page_prot)); >> pmd = pmd_mkyoung(pmd); >> - set_pmd_at(mm, vaddr, pmdp, pmd); >> - pmdp_test_and_clear_young(vma, vaddr, pmdp); >> - pmd = READ_ONCE(*pmdp); >> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >> + pmdp_test_and_clear_young(args->vma, vaddr, args->pmdp); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(pmd_young(pmd)); >> >> /* Clear the pte entries */ >> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >> - pgtable = pgtable_trans_huge_withdraw(mm, pmdp); >> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >> + pgtable_trans_huge_withdraw(args->mm, args->pmdp); >> } >> >> static void __init pmd_leaf_tests(struct pgtable_debug_args *args) >> @@ -417,12 +417,7 @@ static void __init pud_leaf_tests(struct pgtable_debug_args *args) { } >> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ >> static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) { } >> static void __init pud_basic_tests(struct pgtable_debug_args *args, int idx) { } >> -static void __init pmd_advanced_tests(struct mm_struct *mm, >> - struct vm_area_struct *vma, pmd_t *pmdp, >> - unsigned long pfn, unsigned long vaddr, >> - pgprot_t prot, pgtable_t pgtable) >> -{ >> -} >> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { } >> static void __init pud_advanced_tests(struct mm_struct *mm, >> struct vm_area_struct *vma, pud_t *pudp, >> unsigned long pfn, unsigned long vaddr, >> @@ -435,11 +430,11 @@ static void __init pmd_savedwrite_tests(struct pgtable_debug_args *args) { } >> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >> >> #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP >> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) >> { >> pmd_t pmd; >> >> - if (!arch_vmap_pmd_supported(prot)) >> + if (!arch_vmap_pmd_supported(args->page_prot)) >> return; >> >> pr_debug("Validating PMD huge\n"); >> @@ -447,10 +442,11 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >> * X86 defined pmd_set_huge() verifies that the given >> * PMD is not a populated non-leaf entry. >> */ >> - WRITE_ONCE(*pmdp, __pmd(0)); >> - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot)); >> - WARN_ON(!pmd_clear_huge(pmdp)); >> - pmd = READ_ONCE(*pmdp); >> + WRITE_ONCE(*(args->pmdp), __pmd(0)); > > Possible extra braces. > Will drop it in v4, thanks! >> + WARN_ON(!pmd_set_huge(args->pmdp, __pfn_to_phys(args->fixed_pmd_pfn), >> + args->page_prot)); > > Dont break the line. > Ok. >> + WARN_ON(!pmd_clear_huge(args->pmdp)); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(!pmd_none(pmd)); >> } >> >> @@ -473,7 +469,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) >> WARN_ON(!pud_none(pud)); >> } >> #else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ >> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { } >> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { } >> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { } >> #endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */ >> >> @@ -640,20 +636,19 @@ static void __init pte_clear_tests(struct pgtable_debug_args *args) >> WARN_ON(!pte_none(pte)); >> } >> >> -static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp) >> +static void __init pmd_clear_tests(struct pgtable_debug_args *args) >> { >> - pmd_t pmd = READ_ONCE(*pmdp); >> + pmd_t pmd = READ_ONCE(*(args->pmdp)); >> >> pr_debug("Validating PMD clear\n"); >> pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE); >> - WRITE_ONCE(*pmdp, pmd); >> - pmd_clear(pmdp); >> - pmd = READ_ONCE(*pmdp); >> + WRITE_ONCE(*(args->pmdp), pmd); >> + pmd_clear(args->pmdp); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(!pmd_none(pmd)); >> } >> >> -static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >> - pgtable_t pgtable) >> +static void __init pmd_populate_tests(struct pgtable_debug_args *args) >> { >> pmd_t pmd; >> >> @@ -662,8 +657,8 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >> * This entry points to next level page table page. >> * Hence this must not qualify as pmd_bad(). >> */ >> - pmd_populate(mm, pmdp, pgtable); >> - pmd = READ_ONCE(*pmdp); >> + pmd_populate(args->mm, args->pmdp, args->start_ptep); >> + pmd = READ_ONCE(*(args->pmdp)); >> WARN_ON(pmd_bad(pmd)); >> } >> >> @@ -1159,7 +1154,7 @@ static int __init debug_vm_pgtable(void) >> pgtable_t saved_ptep; >> pgprot_t prot; >> phys_addr_t paddr; >> - unsigned long vaddr, pmd_aligned; >> + unsigned long vaddr; >> unsigned long pud_aligned; >> spinlock_t *ptl = NULL; >> int idx, ret; >> @@ -1194,7 +1189,6 @@ static int __init debug_vm_pgtable(void) >> */ >> paddr = __pa_symbol(&start_kernel); >> >> - pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; > > Please dont drop these just yet and wait until [PATCH 11/12]. > Otherwise, it causes build warning: 'unused variable'. >> pud_aligned = (paddr & PUD_MASK) >> PAGE_SHIFT; >> >> pgdp = pgd_offset(mm, vaddr); >> @@ -1281,11 +1275,11 @@ static int __init debug_vm_pgtable(void) >> pte_advanced_tests(&args); >> spin_unlock(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); >> + ptl = pmd_lock(args.mm, args.pmdp); >> + pmd_clear_tests(&args); >> + pmd_advanced_tests(&args); >> + pmd_huge_tests(&args); >> + pmd_populate_tests(&args); >> spin_unlock(ptl); >> >> ptl = pud_lock(mm, pudp); >> > Thanks, Gavin
On 7/22/21 12:11 PM, Gavin Shan wrote: > Hi Anshuman, > > On 7/22/21 3:45 PM, Anshuman Khandual wrote: >> On 7/19/21 6:36 PM, Gavin Shan wrote: >>> This uses struct pgtable_debug_args in PMD modifying tests. The allocated >>> huge page is used when set_pmd_at() is used. The corresponding tests >>> are skipped if the huge page doesn't exist. Besides, the unused variable >>> @pmd_aligned in debug_vm_pgtable() is dropped. >> >> Please dont drop @pmd_aligned just yet. >> > > We need do so. Otherwise, there is build warning to complain > something like 'unused variable' after this patch is applied. > >>> >>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>> --- >>> mm/debug_vm_pgtable.c | 102 ++++++++++++++++++++---------------------- >>> 1 file changed, 48 insertions(+), 54 deletions(-) >>> >>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >>> index eb6dda88e0d9..cec3cbf99a6b 100644 >>> --- a/mm/debug_vm_pgtable.c >>> +++ b/mm/debug_vm_pgtable.c >>> @@ -213,54 +213,54 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) >>> WARN_ON(!pmd_bad(pmd_mkhuge(pmd))); >>> } >>> -static void __init pmd_advanced_tests(struct mm_struct *mm, >>> - struct vm_area_struct *vma, pmd_t *pmdp, >>> - unsigned long pfn, unsigned long vaddr, >>> - pgprot_t prot, pgtable_t pgtable) >>> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) >>> { >>> pmd_t pmd; >>> + unsigned long vaddr = (args->vaddr & HPAGE_PMD_MASK); >>> if (!has_transparent_hugepage()) >>> return; >>> pr_debug("Validating PMD advanced\n"); >>> - /* Align the address wrt HPAGE_PMD_SIZE */ >>> - vaddr &= HPAGE_PMD_MASK; >> >> Please just leave these unchanged. If has_transparent_hugepage() evaluates >> negative, it skips the masking operation. As mentioned earlier please avoid >> changing the test in any manner during these transition patches. >> > > Ok. > >>> + if (args->pmd_pfn == ULONG_MAX) { >>> + pr_debug("%s: Skipped\n", __func__); >>> + return; >>> + } >> >> Just return. Please dont call out "Skipped". >> > > Ok. > >>> - pgtable_trans_huge_deposit(mm, pmdp, pgtable); >>> + pgtable_trans_huge_deposit(args->mm, args->pmdp, args->start_ptep); >>> - pmd = pfn_pmd(pfn, prot); >>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>> - pmdp_set_wrprotect(mm, vaddr, pmdp); >>> - pmd = READ_ONCE(*pmdp); >>> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>> + pmdp_set_wrprotect(args->mm, vaddr, args->pmdp); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(pmd_write(pmd)); >>> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >>> - pmd = READ_ONCE(*pmdp); >>> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(!pmd_none(pmd)); >>> - pmd = pfn_pmd(pfn, prot); >>> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >>> pmd = pmd_wrprotect(pmd); >>> pmd = pmd_mkclean(pmd); >>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>> pmd = pmd_mkwrite(pmd); >>> pmd = pmd_mkdirty(pmd); >>> - pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1); >>> - pmd = READ_ONCE(*pmdp); >>> + pmdp_set_access_flags(args->vma, vaddr, args->pmdp, pmd, 1); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd))); >>> - pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1); >>> - pmd = READ_ONCE(*pmdp); >>> + pmdp_huge_get_and_clear_full(args->vma, vaddr, args->pmdp, 1); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(!pmd_none(pmd)); >>> - pmd = pmd_mkhuge(pfn_pmd(pfn, prot)); >>> + pmd = pmd_mkhuge(pfn_pmd(args->pmd_pfn, args->page_prot)); >>> pmd = pmd_mkyoung(pmd); >>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>> - pmdp_test_and_clear_young(vma, vaddr, pmdp); >>> - pmd = READ_ONCE(*pmdp); >>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>> + pmdp_test_and_clear_young(args->vma, vaddr, args->pmdp); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(pmd_young(pmd)); >>> /* Clear the pte entries */ >>> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >>> - pgtable = pgtable_trans_huge_withdraw(mm, pmdp); >>> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >>> + pgtable_trans_huge_withdraw(args->mm, args->pmdp); >>> } >>> static void __init pmd_leaf_tests(struct pgtable_debug_args *args) >>> @@ -417,12 +417,7 @@ static void __init pud_leaf_tests(struct pgtable_debug_args *args) { } >>> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ >>> static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) { } >>> static void __init pud_basic_tests(struct pgtable_debug_args *args, int idx) { } >>> -static void __init pmd_advanced_tests(struct mm_struct *mm, >>> - struct vm_area_struct *vma, pmd_t *pmdp, >>> - unsigned long pfn, unsigned long vaddr, >>> - pgprot_t prot, pgtable_t pgtable) >>> -{ >>> -} >>> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { } >>> static void __init pud_advanced_tests(struct mm_struct *mm, >>> struct vm_area_struct *vma, pud_t *pudp, >>> unsigned long pfn, unsigned long vaddr, >>> @@ -435,11 +430,11 @@ static void __init pmd_savedwrite_tests(struct pgtable_debug_args *args) { } >>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >>> #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP >>> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >>> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) >>> { >>> pmd_t pmd; >>> - if (!arch_vmap_pmd_supported(prot)) >>> + if (!arch_vmap_pmd_supported(args->page_prot)) >>> return; >>> pr_debug("Validating PMD huge\n"); >>> @@ -447,10 +442,11 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >>> * X86 defined pmd_set_huge() verifies that the given >>> * PMD is not a populated non-leaf entry. >>> */ >>> - WRITE_ONCE(*pmdp, __pmd(0)); >>> - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot)); >>> - WARN_ON(!pmd_clear_huge(pmdp)); >>> - pmd = READ_ONCE(*pmdp); >>> + WRITE_ONCE(*(args->pmdp), __pmd(0)); >> >> Possible extra braces. >> > > Will drop it in v4, thanks! > >>> + WARN_ON(!pmd_set_huge(args->pmdp, __pfn_to_phys(args->fixed_pmd_pfn), >>> + args->page_prot)); >> >> Dont break the line. >> > > Ok. > >>> + WARN_ON(!pmd_clear_huge(args->pmdp)); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(!pmd_none(pmd)); >>> } >>> @@ -473,7 +469,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) >>> WARN_ON(!pud_none(pud)); >>> } >>> #else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ >>> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { } >>> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { } >>> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { } >>> #endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */ >>> @@ -640,20 +636,19 @@ static void __init pte_clear_tests(struct pgtable_debug_args *args) >>> WARN_ON(!pte_none(pte)); >>> } >>> -static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp) >>> +static void __init pmd_clear_tests(struct pgtable_debug_args *args) >>> { >>> - pmd_t pmd = READ_ONCE(*pmdp); >>> + pmd_t pmd = READ_ONCE(*(args->pmdp)); >>> pr_debug("Validating PMD clear\n"); >>> pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE); >>> - WRITE_ONCE(*pmdp, pmd); >>> - pmd_clear(pmdp); >>> - pmd = READ_ONCE(*pmdp); >>> + WRITE_ONCE(*(args->pmdp), pmd); >>> + pmd_clear(args->pmdp); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(!pmd_none(pmd)); >>> } >>> -static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >>> - pgtable_t pgtable) >>> +static void __init pmd_populate_tests(struct pgtable_debug_args *args) >>> { >>> pmd_t pmd; >>> @@ -662,8 +657,8 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >>> * This entry points to next level page table page. >>> * Hence this must not qualify as pmd_bad(). >>> */ >>> - pmd_populate(mm, pmdp, pgtable); >>> - pmd = READ_ONCE(*pmdp); >>> + pmd_populate(args->mm, args->pmdp, args->start_ptep); >>> + pmd = READ_ONCE(*(args->pmdp)); >>> WARN_ON(pmd_bad(pmd)); >>> } >>> @@ -1159,7 +1154,7 @@ static int __init debug_vm_pgtable(void) >>> pgtable_t saved_ptep; >>> pgprot_t prot; >>> phys_addr_t paddr; >>> - unsigned long vaddr, pmd_aligned; >>> + unsigned long vaddr; >>> unsigned long pud_aligned; >>> spinlock_t *ptl = NULL; >>> int idx, ret; >>> @@ -1194,7 +1189,6 @@ static int __init debug_vm_pgtable(void) >>> */ >>> paddr = __pa_symbol(&start_kernel); >>> - pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; >> >> Please dont drop these just yet and wait until [PATCH 11/12]. >> > > Otherwise, it causes build warning: 'unused variable'. Why ? Just the evaluation of 'pmd_aligned' from 'paddr' should be enough to avoid such warning. 'pmd_aligned' need not be used afterwards.
Hi Anshuman, On 7/22/21 5:11 PM, Anshuman Khandual wrote: > On 7/22/21 12:11 PM, Gavin Shan wrote: >> On 7/22/21 3:45 PM, Anshuman Khandual wrote: >>> On 7/19/21 6:36 PM, Gavin Shan wrote: >>>> This uses struct pgtable_debug_args in PMD modifying tests. The allocated >>>> huge page is used when set_pmd_at() is used. The corresponding tests >>>> are skipped if the huge page doesn't exist. Besides, the unused variable >>>> @pmd_aligned in debug_vm_pgtable() is dropped. >>> >>> Please dont drop @pmd_aligned just yet. >>> >> >> We need do so. Otherwise, there is build warning to complain >> something like 'unused variable' after this patch is applied. >> >>>> >>>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>>> --- >>>> mm/debug_vm_pgtable.c | 102 ++++++++++++++++++++---------------------- >>>> 1 file changed, 48 insertions(+), 54 deletions(-) >>>> >>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >>>> index eb6dda88e0d9..cec3cbf99a6b 100644 >>>> --- a/mm/debug_vm_pgtable.c >>>> +++ b/mm/debug_vm_pgtable.c >>>> @@ -213,54 +213,54 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) >>>> WARN_ON(!pmd_bad(pmd_mkhuge(pmd))); >>>> } >>>> -static void __init pmd_advanced_tests(struct mm_struct *mm, >>>> - struct vm_area_struct *vma, pmd_t *pmdp, >>>> - unsigned long pfn, unsigned long vaddr, >>>> - pgprot_t prot, pgtable_t pgtable) >>>> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) >>>> { >>>> pmd_t pmd; >>>> + unsigned long vaddr = (args->vaddr & HPAGE_PMD_MASK); >>>> if (!has_transparent_hugepage()) >>>> return; >>>> pr_debug("Validating PMD advanced\n"); >>>> - /* Align the address wrt HPAGE_PMD_SIZE */ >>>> - vaddr &= HPAGE_PMD_MASK; >>> >>> Please just leave these unchanged. If has_transparent_hugepage() evaluates >>> negative, it skips the masking operation. As mentioned earlier please avoid >>> changing the test in any manner during these transition patches. >>> >> >> Ok. >> >>>> + if (args->pmd_pfn == ULONG_MAX) { >>>> + pr_debug("%s: Skipped\n", __func__); >>>> + return; >>>> + } >>> >>> Just return. Please dont call out "Skipped". >>> >> >> Ok. >> >>>> - pgtable_trans_huge_deposit(mm, pmdp, pgtable); >>>> + pgtable_trans_huge_deposit(args->mm, args->pmdp, args->start_ptep); >>>> - pmd = pfn_pmd(pfn, prot); >>>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>>> - pmdp_set_wrprotect(mm, vaddr, pmdp); >>>> - pmd = READ_ONCE(*pmdp); >>>> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >>>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>>> + pmdp_set_wrprotect(args->mm, vaddr, args->pmdp); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(pmd_write(pmd)); >>>> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >>>> - pmd = READ_ONCE(*pmdp); >>>> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(!pmd_none(pmd)); >>>> - pmd = pfn_pmd(pfn, prot); >>>> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >>>> pmd = pmd_wrprotect(pmd); >>>> pmd = pmd_mkclean(pmd); >>>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>>> pmd = pmd_mkwrite(pmd); >>>> pmd = pmd_mkdirty(pmd); >>>> - pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1); >>>> - pmd = READ_ONCE(*pmdp); >>>> + pmdp_set_access_flags(args->vma, vaddr, args->pmdp, pmd, 1); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd))); >>>> - pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1); >>>> - pmd = READ_ONCE(*pmdp); >>>> + pmdp_huge_get_and_clear_full(args->vma, vaddr, args->pmdp, 1); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(!pmd_none(pmd)); >>>> - pmd = pmd_mkhuge(pfn_pmd(pfn, prot)); >>>> + pmd = pmd_mkhuge(pfn_pmd(args->pmd_pfn, args->page_prot)); >>>> pmd = pmd_mkyoung(pmd); >>>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>>> - pmdp_test_and_clear_young(vma, vaddr, pmdp); >>>> - pmd = READ_ONCE(*pmdp); >>>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>>> + pmdp_test_and_clear_young(args->vma, vaddr, args->pmdp); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(pmd_young(pmd)); >>>> /* Clear the pte entries */ >>>> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >>>> - pgtable = pgtable_trans_huge_withdraw(mm, pmdp); >>>> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >>>> + pgtable_trans_huge_withdraw(args->mm, args->pmdp); >>>> } >>>> static void __init pmd_leaf_tests(struct pgtable_debug_args *args) >>>> @@ -417,12 +417,7 @@ static void __init pud_leaf_tests(struct pgtable_debug_args *args) { } >>>> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ >>>> static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) { } >>>> static void __init pud_basic_tests(struct pgtable_debug_args *args, int idx) { } >>>> -static void __init pmd_advanced_tests(struct mm_struct *mm, >>>> - struct vm_area_struct *vma, pmd_t *pmdp, >>>> - unsigned long pfn, unsigned long vaddr, >>>> - pgprot_t prot, pgtable_t pgtable) >>>> -{ >>>> -} >>>> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { } >>>> static void __init pud_advanced_tests(struct mm_struct *mm, >>>> struct vm_area_struct *vma, pud_t *pudp, >>>> unsigned long pfn, unsigned long vaddr, >>>> @@ -435,11 +430,11 @@ static void __init pmd_savedwrite_tests(struct pgtable_debug_args *args) { } >>>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >>>> #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP >>>> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >>>> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) >>>> { >>>> pmd_t pmd; >>>> - if (!arch_vmap_pmd_supported(prot)) >>>> + if (!arch_vmap_pmd_supported(args->page_prot)) >>>> return; >>>> pr_debug("Validating PMD huge\n"); >>>> @@ -447,10 +442,11 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >>>> * X86 defined pmd_set_huge() verifies that the given >>>> * PMD is not a populated non-leaf entry. >>>> */ >>>> - WRITE_ONCE(*pmdp, __pmd(0)); >>>> - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot)); >>>> - WARN_ON(!pmd_clear_huge(pmdp)); >>>> - pmd = READ_ONCE(*pmdp); >>>> + WRITE_ONCE(*(args->pmdp), __pmd(0)); >>> >>> Possible extra braces. >>> >> >> Will drop it in v4, thanks! >> >>>> + WARN_ON(!pmd_set_huge(args->pmdp, __pfn_to_phys(args->fixed_pmd_pfn), >>>> + args->page_prot)); >>> >>> Dont break the line. >>> >> >> Ok. >> >>>> + WARN_ON(!pmd_clear_huge(args->pmdp)); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(!pmd_none(pmd)); >>>> } >>>> @@ -473,7 +469,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) >>>> WARN_ON(!pud_none(pud)); >>>> } >>>> #else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ >>>> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { } >>>> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { } >>>> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { } >>>> #endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */ >>>> @@ -640,20 +636,19 @@ static void __init pte_clear_tests(struct pgtable_debug_args *args) >>>> WARN_ON(!pte_none(pte)); >>>> } >>>> -static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp) >>>> +static void __init pmd_clear_tests(struct pgtable_debug_args *args) >>>> { >>>> - pmd_t pmd = READ_ONCE(*pmdp); >>>> + pmd_t pmd = READ_ONCE(*(args->pmdp)); >>>> pr_debug("Validating PMD clear\n"); >>>> pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE); >>>> - WRITE_ONCE(*pmdp, pmd); >>>> - pmd_clear(pmdp); >>>> - pmd = READ_ONCE(*pmdp); >>>> + WRITE_ONCE(*(args->pmdp), pmd); >>>> + pmd_clear(args->pmdp); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(!pmd_none(pmd)); >>>> } >>>> -static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >>>> - pgtable_t pgtable) >>>> +static void __init pmd_populate_tests(struct pgtable_debug_args *args) >>>> { >>>> pmd_t pmd; >>>> @@ -662,8 +657,8 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >>>> * This entry points to next level page table page. >>>> * Hence this must not qualify as pmd_bad(). >>>> */ >>>> - pmd_populate(mm, pmdp, pgtable); >>>> - pmd = READ_ONCE(*pmdp); >>>> + pmd_populate(args->mm, args->pmdp, args->start_ptep); >>>> + pmd = READ_ONCE(*(args->pmdp)); >>>> WARN_ON(pmd_bad(pmd)); >>>> } >>>> @@ -1159,7 +1154,7 @@ static int __init debug_vm_pgtable(void) >>>> pgtable_t saved_ptep; >>>> pgprot_t prot; >>>> phys_addr_t paddr; >>>> - unsigned long vaddr, pmd_aligned; >>>> + unsigned long vaddr; >>>> unsigned long pud_aligned; >>>> spinlock_t *ptl = NULL; >>>> int idx, ret; >>>> @@ -1194,7 +1189,6 @@ static int __init debug_vm_pgtable(void) >>>> */ >>>> paddr = __pa_symbol(&start_kernel); >>>> - pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; >>> >>> Please dont drop these just yet and wait until [PATCH 11/12]. >>> >> >> Otherwise, it causes build warning: 'unused variable'. > > Why ? Just the evaluation of 'pmd_aligned' from 'paddr' should > be enough to avoid such warning. 'pmd_aligned' need not be used > afterwards. > It's not enough to avoid the warning. I apply the patches till this one (PATCH[v3 08/12]) and have additional code to keep @pmd_aligned, then I run into build warning as below: diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c index cec3cbf99a6b..961c9bb6fc7c 100644 --- a/mm/debug_vm_pgtable.c +++ b/mm/debug_vm_pgtable.c @@ -1154,7 +1154,7 @@ static int __init debug_vm_pgtable(void) pgtable_t saved_ptep; pgprot_t prot; phys_addr_t paddr; - unsigned long vaddr; + unsigned long vaddr, pmd_aligned; unsigned long pud_aligned; spinlock_t *ptl = NULL; int idx, ret; @@ -1189,6 +1189,7 @@ static int __init debug_vm_pgtable(void) */ paddr = __pa_symbol(&start_kernel); + pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; pud_aligned = (paddr & PUD_MASK) >> PAGE_SHIFT; pgdp = pgd_offset(mm, vaddr); [gwshan@gshan l]$ make W=1 mm/debug_vm_pgtable.o : mm/debug_vm_pgtable.c: In function ‘debug_vm_pgtable’: mm/debug_vm_pgtable.c:1157:23: warning: variable ‘pmd_aligned’ set but not used [-Wunused-but-set-variable] 1157 | unsigned long vaddr, pmd_aligned; | ^~~~~~~~~~~ By the way, 0-day is trying to build kernel with "W=1". It means the build warnings will be reported by 0-day if the unused variables aren't dropped from individual patches. It makes the review a bit harder. However, we still need to keep each individual patch complete to make 'git bisect' friendly. Thanks, Gavin
On 7/23/21 6:30 AM, Gavin Shan wrote: > Hi Anshuman, > > On 7/22/21 5:11 PM, Anshuman Khandual wrote: >> On 7/22/21 12:11 PM, Gavin Shan wrote: >>> On 7/22/21 3:45 PM, Anshuman Khandual wrote: >>>> On 7/19/21 6:36 PM, Gavin Shan wrote: >>>>> This uses struct pgtable_debug_args in PMD modifying tests. The allocated >>>>> huge page is used when set_pmd_at() is used. The corresponding tests >>>>> are skipped if the huge page doesn't exist. Besides, the unused variable >>>>> @pmd_aligned in debug_vm_pgtable() is dropped. >>>> >>>> Please dont drop @pmd_aligned just yet. >>>> >>> >>> We need do so. Otherwise, there is build warning to complain >>> something like 'unused variable' after this patch is applied. >>> >>>>> >>>>> Signed-off-by: Gavin Shan <gshan@redhat.com> >>>>> --- >>>>> mm/debug_vm_pgtable.c | 102 ++++++++++++++++++++---------------------- >>>>> 1 file changed, 48 insertions(+), 54 deletions(-) >>>>> >>>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >>>>> index eb6dda88e0d9..cec3cbf99a6b 100644 >>>>> --- a/mm/debug_vm_pgtable.c >>>>> +++ b/mm/debug_vm_pgtable.c >>>>> @@ -213,54 +213,54 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) >>>>> WARN_ON(!pmd_bad(pmd_mkhuge(pmd))); >>>>> } >>>>> -static void __init pmd_advanced_tests(struct mm_struct *mm, >>>>> - struct vm_area_struct *vma, pmd_t *pmdp, >>>>> - unsigned long pfn, unsigned long vaddr, >>>>> - pgprot_t prot, pgtable_t pgtable) >>>>> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) >>>>> { >>>>> pmd_t pmd; >>>>> + unsigned long vaddr = (args->vaddr & HPAGE_PMD_MASK); >>>>> if (!has_transparent_hugepage()) >>>>> return; >>>>> pr_debug("Validating PMD advanced\n"); >>>>> - /* Align the address wrt HPAGE_PMD_SIZE */ >>>>> - vaddr &= HPAGE_PMD_MASK; >>>> >>>> Please just leave these unchanged. If has_transparent_hugepage() evaluates >>>> negative, it skips the masking operation. As mentioned earlier please avoid >>>> changing the test in any manner during these transition patches. >>>> >>> >>> Ok. >>> >>>>> + if (args->pmd_pfn == ULONG_MAX) { >>>>> + pr_debug("%s: Skipped\n", __func__); >>>>> + return; >>>>> + } >>>> >>>> Just return. Please dont call out "Skipped". >>>> >>> >>> Ok. >>> >>>>> - pgtable_trans_huge_deposit(mm, pmdp, pgtable); >>>>> + pgtable_trans_huge_deposit(args->mm, args->pmdp, args->start_ptep); >>>>> - pmd = pfn_pmd(pfn, prot); >>>>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>>>> - pmdp_set_wrprotect(mm, vaddr, pmdp); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >>>>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>>>> + pmdp_set_wrprotect(args->mm, vaddr, args->pmdp); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(pmd_write(pmd)); >>>>> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(!pmd_none(pmd)); >>>>> - pmd = pfn_pmd(pfn, prot); >>>>> + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); >>>>> pmd = pmd_wrprotect(pmd); >>>>> pmd = pmd_mkclean(pmd); >>>>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>>>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>>>> pmd = pmd_mkwrite(pmd); >>>>> pmd = pmd_mkdirty(pmd); >>>>> - pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + pmdp_set_access_flags(args->vma, vaddr, args->pmdp, pmd, 1); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd))); >>>>> - pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + pmdp_huge_get_and_clear_full(args->vma, vaddr, args->pmdp, 1); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(!pmd_none(pmd)); >>>>> - pmd = pmd_mkhuge(pfn_pmd(pfn, prot)); >>>>> + pmd = pmd_mkhuge(pfn_pmd(args->pmd_pfn, args->page_prot)); >>>>> pmd = pmd_mkyoung(pmd); >>>>> - set_pmd_at(mm, vaddr, pmdp, pmd); >>>>> - pmdp_test_and_clear_young(vma, vaddr, pmdp); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); >>>>> + pmdp_test_and_clear_young(args->vma, vaddr, args->pmdp); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(pmd_young(pmd)); >>>>> /* Clear the pte entries */ >>>>> - pmdp_huge_get_and_clear(mm, vaddr, pmdp); >>>>> - pgtable = pgtable_trans_huge_withdraw(mm, pmdp); >>>>> + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); >>>>> + pgtable_trans_huge_withdraw(args->mm, args->pmdp); >>>>> } >>>>> static void __init pmd_leaf_tests(struct pgtable_debug_args *args) >>>>> @@ -417,12 +417,7 @@ static void __init pud_leaf_tests(struct pgtable_debug_args *args) { } >>>>> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ >>>>> static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) { } >>>>> static void __init pud_basic_tests(struct pgtable_debug_args *args, int idx) { } >>>>> -static void __init pmd_advanced_tests(struct mm_struct *mm, >>>>> - struct vm_area_struct *vma, pmd_t *pmdp, >>>>> - unsigned long pfn, unsigned long vaddr, >>>>> - pgprot_t prot, pgtable_t pgtable) >>>>> -{ >>>>> -} >>>>> +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { } >>>>> static void __init pud_advanced_tests(struct mm_struct *mm, >>>>> struct vm_area_struct *vma, pud_t *pudp, >>>>> unsigned long pfn, unsigned long vaddr, >>>>> @@ -435,11 +430,11 @@ static void __init pmd_savedwrite_tests(struct pgtable_debug_args *args) { } >>>>> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ >>>>> #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP >>>>> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >>>>> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) >>>>> { >>>>> pmd_t pmd; >>>>> - if (!arch_vmap_pmd_supported(prot)) >>>>> + if (!arch_vmap_pmd_supported(args->page_prot)) >>>>> return; >>>>> pr_debug("Validating PMD huge\n"); >>>>> @@ -447,10 +442,11 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) >>>>> * X86 defined pmd_set_huge() verifies that the given >>>>> * PMD is not a populated non-leaf entry. >>>>> */ >>>>> - WRITE_ONCE(*pmdp, __pmd(0)); >>>>> - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot)); >>>>> - WARN_ON(!pmd_clear_huge(pmdp)); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + WRITE_ONCE(*(args->pmdp), __pmd(0)); >>>> >>>> Possible extra braces. >>>> >>> >>> Will drop it in v4, thanks! >>> >>>>> + WARN_ON(!pmd_set_huge(args->pmdp, __pfn_to_phys(args->fixed_pmd_pfn), >>>>> + args->page_prot)); >>>> >>>> Dont break the line. >>>> >>> >>> Ok. >>> >>>>> + WARN_ON(!pmd_clear_huge(args->pmdp)); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(!pmd_none(pmd)); >>>>> } >>>>> @@ -473,7 +469,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) >>>>> WARN_ON(!pud_none(pud)); >>>>> } >>>>> #else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ >>>>> -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { } >>>>> +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { } >>>>> static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { } >>>>> #endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */ >>>>> @@ -640,20 +636,19 @@ static void __init pte_clear_tests(struct pgtable_debug_args *args) >>>>> WARN_ON(!pte_none(pte)); >>>>> } >>>>> -static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp) >>>>> +static void __init pmd_clear_tests(struct pgtable_debug_args *args) >>>>> { >>>>> - pmd_t pmd = READ_ONCE(*pmdp); >>>>> + pmd_t pmd = READ_ONCE(*(args->pmdp)); >>>>> pr_debug("Validating PMD clear\n"); >>>>> pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE); >>>>> - WRITE_ONCE(*pmdp, pmd); >>>>> - pmd_clear(pmdp); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + WRITE_ONCE(*(args->pmdp), pmd); >>>>> + pmd_clear(args->pmdp); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(!pmd_none(pmd)); >>>>> } >>>>> -static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >>>>> - pgtable_t pgtable) >>>>> +static void __init pmd_populate_tests(struct pgtable_debug_args *args) >>>>> { >>>>> pmd_t pmd; >>>>> @@ -662,8 +657,8 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, >>>>> * This entry points to next level page table page. >>>>> * Hence this must not qualify as pmd_bad(). >>>>> */ >>>>> - pmd_populate(mm, pmdp, pgtable); >>>>> - pmd = READ_ONCE(*pmdp); >>>>> + pmd_populate(args->mm, args->pmdp, args->start_ptep); >>>>> + pmd = READ_ONCE(*(args->pmdp)); >>>>> WARN_ON(pmd_bad(pmd)); >>>>> } >>>>> @@ -1159,7 +1154,7 @@ static int __init debug_vm_pgtable(void) >>>>> pgtable_t saved_ptep; >>>>> pgprot_t prot; >>>>> phys_addr_t paddr; >>>>> - unsigned long vaddr, pmd_aligned; >>>>> + unsigned long vaddr; >>>>> unsigned long pud_aligned; >>>>> spinlock_t *ptl = NULL; >>>>> int idx, ret; >>>>> @@ -1194,7 +1189,6 @@ static int __init debug_vm_pgtable(void) >>>>> */ >>>>> paddr = __pa_symbol(&start_kernel); >>>>> - pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; >>>> >>>> Please dont drop these just yet and wait until [PATCH 11/12]. >>>> >>> >>> Otherwise, it causes build warning: 'unused variable'. >> >> Why ? Just the evaluation of 'pmd_aligned' from 'paddr' should >> be enough to avoid such warning. 'pmd_aligned' need not be used >> afterwards. >> > > It's not enough to avoid the warning. I apply the patches till > this one (PATCH[v3 08/12]) and have additional code to keep > @pmd_aligned, then I run into build warning as below: > > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c > index cec3cbf99a6b..961c9bb6fc7c 100644 > --- a/mm/debug_vm_pgtable.c > +++ b/mm/debug_vm_pgtable.c > @@ -1154,7 +1154,7 @@ static int __init debug_vm_pgtable(void) > pgtable_t saved_ptep; > pgprot_t prot; > phys_addr_t paddr; > - unsigned long vaddr; > + unsigned long vaddr, pmd_aligned; > unsigned long pud_aligned; > spinlock_t *ptl = NULL; > int idx, ret; > @@ -1189,6 +1189,7 @@ static int __init debug_vm_pgtable(void) > */ > paddr = __pa_symbol(&start_kernel); > > + pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; > pud_aligned = (paddr & PUD_MASK) >> PAGE_SHIFT; > > pgdp = pgd_offset(mm, vaddr); > > [gwshan@gshan l]$ make W=1 mm/debug_vm_pgtable.o > : > mm/debug_vm_pgtable.c: In function ‘debug_vm_pgtable’: > mm/debug_vm_pgtable.c:1157:23: warning: variable ‘pmd_aligned’ set but not used [-Wunused-but-set-variable] > 1157 | unsigned long vaddr, pmd_aligned; > | ^~~~~~~~~~~ > > > By the way, 0-day is trying to build kernel with "W=1". It means the > build warnings will be reported by 0-day if the unused variables aren't > dropped from individual patches. It makes the review a bit harder. However, It is not about keeping the review process simple. But rather the proposed patch changing as much what is really required and nothing else. > we still need to keep each individual patch complete to make 'git bisect' > friendly. With W=1 if this throws up a build error, I guess we dont have choice here. We need to keep the build clean for each individual patch applies.
diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c index eb6dda88e0d9..cec3cbf99a6b 100644 --- a/mm/debug_vm_pgtable.c +++ b/mm/debug_vm_pgtable.c @@ -213,54 +213,54 @@ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) WARN_ON(!pmd_bad(pmd_mkhuge(pmd))); } -static void __init pmd_advanced_tests(struct mm_struct *mm, - struct vm_area_struct *vma, pmd_t *pmdp, - unsigned long pfn, unsigned long vaddr, - pgprot_t prot, pgtable_t pgtable) +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { pmd_t pmd; + unsigned long vaddr = (args->vaddr & HPAGE_PMD_MASK); if (!has_transparent_hugepage()) return; pr_debug("Validating PMD advanced\n"); - /* Align the address wrt HPAGE_PMD_SIZE */ - vaddr &= HPAGE_PMD_MASK; + if (args->pmd_pfn == ULONG_MAX) { + pr_debug("%s: Skipped\n", __func__); + return; + } - pgtable_trans_huge_deposit(mm, pmdp, pgtable); + pgtable_trans_huge_deposit(args->mm, args->pmdp, args->start_ptep); - pmd = pfn_pmd(pfn, prot); - set_pmd_at(mm, vaddr, pmdp, pmd); - pmdp_set_wrprotect(mm, vaddr, pmdp); - pmd = READ_ONCE(*pmdp); + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); + pmdp_set_wrprotect(args->mm, vaddr, args->pmdp); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(pmd_write(pmd)); - pmdp_huge_get_and_clear(mm, vaddr, pmdp); - pmd = READ_ONCE(*pmdp); + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(!pmd_none(pmd)); - pmd = pfn_pmd(pfn, prot); + pmd = pfn_pmd(args->pmd_pfn, args->page_prot); pmd = pmd_wrprotect(pmd); pmd = pmd_mkclean(pmd); - set_pmd_at(mm, vaddr, pmdp, pmd); + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); pmd = pmd_mkwrite(pmd); pmd = pmd_mkdirty(pmd); - pmdp_set_access_flags(vma, vaddr, pmdp, pmd, 1); - pmd = READ_ONCE(*pmdp); + pmdp_set_access_flags(args->vma, vaddr, args->pmdp, pmd, 1); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(!(pmd_write(pmd) && pmd_dirty(pmd))); - pmdp_huge_get_and_clear_full(vma, vaddr, pmdp, 1); - pmd = READ_ONCE(*pmdp); + pmdp_huge_get_and_clear_full(args->vma, vaddr, args->pmdp, 1); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(!pmd_none(pmd)); - pmd = pmd_mkhuge(pfn_pmd(pfn, prot)); + pmd = pmd_mkhuge(pfn_pmd(args->pmd_pfn, args->page_prot)); pmd = pmd_mkyoung(pmd); - set_pmd_at(mm, vaddr, pmdp, pmd); - pmdp_test_and_clear_young(vma, vaddr, pmdp); - pmd = READ_ONCE(*pmdp); + set_pmd_at(args->mm, vaddr, args->pmdp, pmd); + pmdp_test_and_clear_young(args->vma, vaddr, args->pmdp); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(pmd_young(pmd)); /* Clear the pte entries */ - pmdp_huge_get_and_clear(mm, vaddr, pmdp); - pgtable = pgtable_trans_huge_withdraw(mm, pmdp); + pmdp_huge_get_and_clear(args->mm, vaddr, args->pmdp); + pgtable_trans_huge_withdraw(args->mm, args->pmdp); } static void __init pmd_leaf_tests(struct pgtable_debug_args *args) @@ -417,12 +417,7 @@ static void __init pud_leaf_tests(struct pgtable_debug_args *args) { } #else /* !CONFIG_TRANSPARENT_HUGEPAGE */ static void __init pmd_basic_tests(struct pgtable_debug_args *args, int idx) { } static void __init pud_basic_tests(struct pgtable_debug_args *args, int idx) { } -static void __init pmd_advanced_tests(struct mm_struct *mm, - struct vm_area_struct *vma, pmd_t *pmdp, - unsigned long pfn, unsigned long vaddr, - pgprot_t prot, pgtable_t pgtable) -{ -} +static void __init pmd_advanced_tests(struct pgtable_debug_args *args) { } static void __init pud_advanced_tests(struct mm_struct *mm, struct vm_area_struct *vma, pud_t *pudp, unsigned long pfn, unsigned long vaddr, @@ -435,11 +430,11 @@ static void __init pmd_savedwrite_tests(struct pgtable_debug_args *args) { } #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { pmd_t pmd; - if (!arch_vmap_pmd_supported(prot)) + if (!arch_vmap_pmd_supported(args->page_prot)) return; pr_debug("Validating PMD huge\n"); @@ -447,10 +442,11 @@ static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) * X86 defined pmd_set_huge() verifies that the given * PMD is not a populated non-leaf entry. */ - WRITE_ONCE(*pmdp, __pmd(0)); - WARN_ON(!pmd_set_huge(pmdp, __pfn_to_phys(pfn), prot)); - WARN_ON(!pmd_clear_huge(pmdp)); - pmd = READ_ONCE(*pmdp); + WRITE_ONCE(*(args->pmdp), __pmd(0)); + WARN_ON(!pmd_set_huge(args->pmdp, __pfn_to_phys(args->fixed_pmd_pfn), + args->page_prot)); + WARN_ON(!pmd_clear_huge(args->pmdp)); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(!pmd_none(pmd)); } @@ -473,7 +469,7 @@ static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) WARN_ON(!pud_none(pud)); } #else /* !CONFIG_HAVE_ARCH_HUGE_VMAP */ -static void __init pmd_huge_tests(pmd_t *pmdp, unsigned long pfn, pgprot_t prot) { } +static void __init pmd_huge_tests(struct pgtable_debug_args *args) { } static void __init pud_huge_tests(pud_t *pudp, unsigned long pfn, pgprot_t prot) { } #endif /* CONFIG_HAVE_ARCH_HUGE_VMAP */ @@ -640,20 +636,19 @@ static void __init pte_clear_tests(struct pgtable_debug_args *args) WARN_ON(!pte_none(pte)); } -static void __init pmd_clear_tests(struct mm_struct *mm, pmd_t *pmdp) +static void __init pmd_clear_tests(struct pgtable_debug_args *args) { - pmd_t pmd = READ_ONCE(*pmdp); + pmd_t pmd = READ_ONCE(*(args->pmdp)); pr_debug("Validating PMD clear\n"); pmd = __pmd(pmd_val(pmd) | RANDOM_ORVALUE); - WRITE_ONCE(*pmdp, pmd); - pmd_clear(pmdp); - pmd = READ_ONCE(*pmdp); + WRITE_ONCE(*(args->pmdp), pmd); + pmd_clear(args->pmdp); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(!pmd_none(pmd)); } -static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, - pgtable_t pgtable) +static void __init pmd_populate_tests(struct pgtable_debug_args *args) { pmd_t pmd; @@ -662,8 +657,8 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp, * This entry points to next level page table page. * Hence this must not qualify as pmd_bad(). */ - pmd_populate(mm, pmdp, pgtable); - pmd = READ_ONCE(*pmdp); + pmd_populate(args->mm, args->pmdp, args->start_ptep); + pmd = READ_ONCE(*(args->pmdp)); WARN_ON(pmd_bad(pmd)); } @@ -1159,7 +1154,7 @@ static int __init debug_vm_pgtable(void) pgtable_t saved_ptep; pgprot_t prot; phys_addr_t paddr; - unsigned long vaddr, pmd_aligned; + unsigned long vaddr; unsigned long pud_aligned; spinlock_t *ptl = NULL; int idx, ret; @@ -1194,7 +1189,6 @@ static int __init debug_vm_pgtable(void) */ paddr = __pa_symbol(&start_kernel); - pmd_aligned = (paddr & PMD_MASK) >> PAGE_SHIFT; pud_aligned = (paddr & PUD_MASK) >> PAGE_SHIFT; pgdp = pgd_offset(mm, vaddr); @@ -1281,11 +1275,11 @@ static int __init debug_vm_pgtable(void) pte_advanced_tests(&args); spin_unlock(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); + ptl = pmd_lock(args.mm, args.pmdp); + pmd_clear_tests(&args); + pmd_advanced_tests(&args); + pmd_huge_tests(&args); + pmd_populate_tests(&args); spin_unlock(ptl); ptl = pud_lock(mm, pudp);
This uses struct pgtable_debug_args in PMD modifying tests. The allocated huge page is used when set_pmd_at() is used. The corresponding tests are skipped if the huge page doesn't exist. Besides, the unused variable @pmd_aligned in debug_vm_pgtable() is dropped. Signed-off-by: Gavin Shan <gshan@redhat.com> --- mm/debug_vm_pgtable.c | 102 ++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 54 deletions(-)