On Mon, Jun 14, 2021 at 4:30 AM Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxx> wrote: > > while we are doing this, should we rename pud_page_vaddr to pud_pgtable > and p4d_page_vaddr to p4d_pgtable? I actually find the name > pud_page_vaddr confusing (is it the vaddr of pud page kind of confusion) I agree. The "vaddr" thing is only confusing - I think it was there exactly _because_ the return type was so odd, and because of historical implementation. IOW it was probably mostly due to the implementation issue of "we use __va() to generate the virtual address from a physical address that exists the page tables", and then that internal implementation thing because part of the naming because there was no conceptually good name for it. Now that it literally returns a pointer to a pmd_t, I think the "vaddr" is just a pointless and silly name - *of*course* it's a virtual address, it's a pointer! And it would be much more logical to say that it returns the (pmd-level) pgtable pointer of the pud entry, so yeah, "pud_pgtable()" seems conceptually fairly sensible to me. It's probably a good idea to do the "rename and change type" in one single patch. Not just because it changes the exact same lines, but because the whole "pick a new name when changing semantics" is generally a good idea in that it also makes compiler errors more obvious - type changes can otherwise be hidden by explicit casts etc. Linus