Re: [PATCH v3 1/5] mm: memory_hotplug: remove head variable in do_migrate_range()

[Matthew Wilcox <willy@infradead.org>]

On Tue, Aug 27, 2024 at 07:47:24PM +0800, Kefeng Wang wrote:
> Directly use a folio for HugeTLB and THP when calculate the next pfn, then
> remove unused head variable.

I just noticed this got merged.  You're going to hit BUG_ON with it.

> -		if (PageHuge(page)) {
> -			pfn = page_to_pfn(head) + compound_nr(head) - 1;
> -			isolate_hugetlb(folio, &source);
> -			continue;
> -		} else if (PageTransHuge(page))
> -			pfn = page_to_pfn(head) + thp_nr_pages(page) - 1;
> +		/*
> +		 * No reference or lock is held on the folio, so it might
> +		 * be modified concurrently (e.g. split).  As such,
> +		 * folio_nr_pages() may read garbage.  This is fine as the outer
> +		 * loop will revisit the split folio later.
> +		 */
> +		if (folio_test_large(folio)) {

But it's not fine.  Look at the implementation of folio_test_large():

static inline bool folio_test_large(const struct folio *folio)
{
        return folio_test_head(folio);
}

That's going to be provided by:

#define FOLIO_TEST_FLAG(name, page)                                     \
static __always_inline bool folio_test_##name(const struct folio *folio) \
{ return test_bit(PG_##name, const_folio_flags(folio, page)); }

and here's the BUG:

static const unsigned long *const_folio_flags(const struct folio *folio,
                unsigned n)
{
        const struct page *page = &folio->page;

        VM_BUG_ON_PGFLAGS(PageTail(page), page);
        VM_BUG_ON_PGFLAGS(n > 0 && !test_bit(PG_head, &page->flags), page);
        return &page[n].flags;
}

(this page can be transformed from a head page to a tail page because,
as the comment notes, we don't hold a reference.

Please back this out.