Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > On Thu, Jun 10, 2021 at 1:36 AM Aneesh Kumar K.V > <aneesh.kumar@xxxxxxxxxxxxx> wrote: >> >> @@ -306,8 +306,7 @@ static bool move_normal_pud(struct vm_area_struct *vma, unsigned long old_addr, >> >> VM_BUG_ON(!pud_none(*new_pud)); >> >> - /* Set the new pud */ >> - set_pud_at(mm, new_addr, new_pud, pud); >> + pud_populate(mm, new_pud, (pmd_t *)pud_page_vaddr(pud)); >> flush_tlb_range(vma, old_addr, old_addr + PUD_SIZE); >> if (new_ptl != old_ptl) >> spin_unlock(new_ptl); > > That "(pmd_t *)pud_page_vaddr(pud)" looks a bit odd and doesn't match > the pattern. > > Should we perhaps have a wrapper for it? Maybe called "pud_pgtable()", > the same way we have pmd_pgtable()? IIUC the reason why we do have pmd_pgtable() is that pgtable_t type is arch dependent. On some architecture it is pte_t * and on the other struct page *. The reason being highmem and level 4 page table can be located in highmem. The above is not true with the level 3 table and hence we didn't add an extra type to point to level 3 table. We could add pud_pgtable(), but then it will essentially be a typecast as I did above? Even if we want to do that, that should be done as a separate patch than this series? > > And that wrapper would be good to have a comment or two about what it > actually is. The same way that pmd_pgtable() should but doesn't ;( > > Linus
