Re: [PATCH 1/2] mm: Change pud_page_vaddr to return pmd_t *

[Linus Torvalds <torvalds@linux-foundation.org>]

On Mon, Jun 14, 2021 at 4:30 AM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> 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