On 08/12/2020 12:03 PM, Aneesh Kumar K.V wrote: > set_pte_at() should not be used to set a pte entry at locations that > already holds a valid pte entry. Architectures like ppc64 don't do TLB > invalidate in set_pte_at() and hence expect it to be used to set locations > that are not a valid PTE. Even though set_pte_at() is not really a arch page table helper and very much arch specific, I just wonder why this deviation on ppc64 as compared to other platforms. Detecting such semantics variation across platforms is an objective of this test. As small nit. Please follow the existing subject format for all patches in here. It will improve readability and also help understand these changes better, later on. mm/debug_vm_pgtable: <Specify changes to an individual test> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxx> > --- > mm/debug_vm_pgtable.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c > index 4c32063a8acf..02a7c20aa4a2 100644 > --- a/mm/debug_vm_pgtable.c > +++ b/mm/debug_vm_pgtable.c > @@ -81,8 +81,6 @@ static void __init pte_advanced_tests(struct mm_struct *mm, > pte = ptep_get(ptep); > WARN_ON(pte_write(pte)); > > - pte = pfn_pte(pfn, prot); > - set_pte_at(mm, vaddr, ptep, pte); > ptep_get_and_clear(mm, vaddr, ptep); > pte = ptep_get(ptep); > WARN_ON(!pte_none(pte)); This makes sense. But could you please fold this code stanza with the previous one in order to imply that 'ptep' did have some valid entry before being cleared and checked against pte_none(). > @@ -97,12 +95,14 @@ static void __init pte_advanced_tests(struct mm_struct *mm, > pte = ptep_get(ptep); > WARN_ON(!(pte_write(pte) && pte_dirty(pte))); > > - pte = pfn_pte(pfn, prot); > - set_pte_at(mm, vaddr, ptep, pte); > ptep_get_and_clear_full(mm, vaddr, ptep, 1); > pte = ptep_get(ptep); > WARN_ON(!pte_none(pte)); Same, please fold back. > > + /* > + * We should clear pte before we do set_pte_at > + */ > + pte = ptep_get_and_clear(mm, vaddr, ptep); > pte = pte_mkyoung(pte); > set_pte_at(mm, vaddr, ptep, pte); > ptep_test_and_clear_young(vma, vaddr, ptep); > The comment above should also explain details that are mentioned in the commit message i.e how platforms such as ppc64 expects a clear pte entry for set_pte_at() to work.
