On 08/30/22 09:52, Mike Kravetz wrote:
> On 08/30/22 10:11, David Hildenbrand wrote:
> > On 30.08.22 01:40, Mike Kravetz wrote:
> > > During discussions of this series [1], it was suggested that hugetlb
> > > handling code in follow_page_mask could be simplified. At the beginning
> >
> > Feel free to use a Suggested-by if you consider it appropriate.
> >
> > > of follow_page_mask, there currently is a call to follow_huge_addr which
> > > 'may' handle hugetlb pages. ia64 is the only architecture which provides
> > > a follow_huge_addr routine that does not return error. Instead, at each
> > > level of the page table a check is made for a hugetlb entry. If a hugetlb
> > > entry is found, a call to a routine associated with that entry is made.
> > >
> > > Currently, there are two checks for hugetlb entries at each page table
> > > level. The first check is of the form:
> > > if (p?d_huge())
> > > page = follow_huge_p?d();
> > > the second check is of the form:
> > > if (is_hugepd())
> > > page = follow_huge_pd().
> >
> > BTW, what about all this hugepd stuff in mm/pagewalk.c?
> >
> > Isn't this all dead code as we're essentially routing all hugetlb VMAs
> > via walk_hugetlb_range? [yes, all that hugepd stuff in generic code that
> > overcomplicates stuff has been annoying me for a long time]
>
> I am 'happy' to look at cleaning up that code next. Perhaps I will just
> create a cleanup series.
>
Technically, that code is not dead IIUC. The call to walk_hugetlb_range in
__walk_page_range is as follows:
if (vma && is_vm_hugetlb_page(vma)) {
if (ops->hugetlb_entry)
err = walk_hugetlb_range(start, end, walk);
} else
err = walk_pgd_range(start, end, walk);
We also have the interface walk_page_range_novma() that will call
__walk_page_range without a value for vma. So, in that case we would
end up calling walk_pgd_range, etc. walk_pgd_range and related routines
do have those checks such as:
if (is_hugepd(__hugepd(pmd_val(*pmd))))
err = walk_hugepd_range((hugepd_t *)pmd, addr, next, walk, PMD_SHIFT);
So, it looks like in this case we would process 'hugepd' entries but not
'normal' hugetlb entries. That does not seem right.
Christophe Leroy added this code with commit e17eae2b8399 "mm: pagewalk: fix
walk for hugepage tables". This was part of the series "Convert powerpc to
GENERIC_PTDUMP". And, the ptdump code uses the walk_page_range_novma
interface. So, this code is certainly not dead.
Adding Christophe on Cc:
Christophe do you know if is_hugepd is true for all hugetlb entries, not
just hugepd?
On systems without hugepd entries, I guess ptdump skips all hugetlb entries.
Sigh!
--
Mike Kravetz
