Re: [v5 02/15] mm/huge_memory: add device-private THP support to PMD operations

[David Hildenbrand]

> > > > Couldn't we do here
> > > >
> > > > if (!pmd_present(pmdval))
> > > >       goto nomap;
> > > >
> > > > To replace the original pmd_none() .. check.
> > > >
> > > > A page table must always be present IIRC.
> > >
> > > I am not sure about the pmd_none(), a page table may not be present, I've not audited
> > > the callers. But I think we can do
> >
> > IIRC page tables must always have the present bit set. So we can just simplify to the single pmd_present() check.
>
> Not sure about that one, are you happy if we follow it up later with a separate fix

Taking a look at other page table walkers, they couldn't possibly work if
tables would not be pmd_present().

E.g., walk_pmd_range() in mm/pagewalk.c has

if (walk->vma)
	split_huge_pmd(walk->vma, pmd, addr);
else if (pmd_leaf(*pmd) || !pmd_present(*pmd))
	continue; /* Nothing to do. */

err = walk_pte_range(pmd, addr, next, walk);
if (err)
	break;


If a PTE table would not be pmd_present() we would never ever call walk_pte_range().

So I am pretty sure that this can just be:

diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
index 567e2d084071e..1916d22aaf1f1 100644
--- a/mm/pgtable-generic.c
+++ b/mm/pgtable-generic.c
@@ -290,7 +290,7 @@ pte_t *___pte_offset_map(pmd_t *pmd, unsigned long addr, pmd_t *pmdvalp)
 
        if (pmdvalp)
                *pmdvalp = pmdval;
-       if (unlikely(pmd_none(pmdval) || is_pmd_migration_entry(pmdval)))
+       if (unlikely(!pmd_present(pmdval)))
                goto nomap;
        if (unlikely(pmd_trans_huge(pmdval)))
                goto nomap;

Unless I am missing something important :)

--
Cheers

David / dhildenb