Re: [PATCH 2/2] mm/hugetlb: refactor subpage recording

From: Matthew Wilcox
Date: Tue Jan 26 2021 - 23:04:29 EST


On Tue, Jan 26, 2021 at 08:07:30PM -0400, Jason Gunthorpe wrote:
> I'm looking at Matt's folio patches and see:
>
> +static inline struct folio *next_folio(struct folio *folio)
> +{
> + return folio + folio_nr_pages(folio);
> +}

This is a replacement for places that would do 'page++'. eg it's
used by the bio iterator where we already checked that the phys addr
and the struct page are contiguous.

> And checking page_trans_huge_mapcount():
>
> for (i = 0; i < thp_nr_pages(page); i++) {
> mapcount = atomic_read(&page[i]._mapcount) + 1;

I think we are guaranteed this for transparent huge pages. At least
for now. Zi Yan may have some thoughts for his work on 1GB transhuge
pages ...

> And we have the same logic in hmm_vma_walk_pud():
>
> if (pud_huge(pud) && pud_devmap(pud)) {
> pfn = pud_pfn(pud) + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
> for (i = 0; i < npages; ++i, ++pfn)
> hmm_pfns[i] = pfn | cpu_flags;
>
> So, if page[n] does not access the tail pages of a compound we have
> many more people who are surprised by this than just GUP.
>
> Where are these special rules for hugetlb compound tails documented?
> Why does it need to be like this?
>
> Isn't it saner to forbid a compound and its tails from being
> non-linear in the page array? That limits when compounds can be
> created, but seems more likely to happen than a full mm audit to find
> all the places that assume linearity.
>
> Jason