Re: [PATCH RFC] mm: skip gigantic pages in isolate_single_pageblock() when mem offline

[Matthew Wilcox <willy@infradead.org>]

On Tue, Aug 13, 2024 at 08:52:26PM +0800, Kefeng Wang wrote:
> The gigantic page size may larger than memory block size, so memory
> offline always fails in this case after commit b2c9e2fbba32 ("mm: make
> alloc_contig_range work at pageblock granularity"),
> 
> offline_pages
>   start_isolate_page_range
>     start_isolate_page_range(isolate_before=true)
>       isolate [isolate_start, isolate_start + pageblock_nr_pages)
>     start_isolate_page_range(isolate_before=false)
>       isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock
>        	__alloc_contig_migrate_range
>           isolate_migratepages_range
>             isolate_migratepages_block
>               isolate_or_dissolve_huge_page
>                 if (hstate_is_gigantic(h))
>                     return -ENOMEM;
> 
> [   15.815756] memory offlining [mem 0x3c0000000-0x3c7ffffff] failed due to failure to isolate range
> 
> Fix it by skipping the __alloc_contig_migrate_range() if met gigantic
> pages when memory offline, which return back to the original logic to
> handle the gigantic pages.

This seems like the wrong way to fix this.  The logic in the next
PageHuge() section seems like it's specifically supposed to handle
gigantic pages.  So you've just made that dead code, but instead of
removing it, you've left it there to confuse everyone?

I admit to not understanding this code terribly well.

> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  mm/page_isolation.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 042937d5abe4..25db4040e70a 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -400,6 +400,16 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
>  				continue;
>  			}
>  
> +			if ((flags & MEMORY_OFFLINE) && PageHuge(page)) {
> +				struct hstate *h;
> +
> +				h = size_to_hstate(nr_pages << PAGE_SHIFT);
> +				if (hstate_is_gigantic(h)) {
> +					pfn = head_pfn + nr_pages;
> +					continue;
> +				}
> +			}
> +
>  #if defined CONFIG_COMPACTION || defined CONFIG_CMA
>  			if (PageHuge(page)) {
>  				int page_mt = get_pageblock_migratetype(page);
> -- 
> 2.27.0
> 
>