Re: [PATCH 4/4] mm/memory-failure.c: fix potential VM_BUG_ON_PAGE in split_huge_page_to_list

[HORIGUCHI NAOYA(堀口 直也)]

On Mon, Feb 28, 2022 at 10:02:45PM +0800, Miaohe Lin wrote:
> The huge zero page could reach here and if we ever try to split it, the
> VM_BUG_ON_PAGE will be triggered in split_huge_page_to_list(). Also the
> non-lru compound movable pages could be taken for transhuge pages. Skip
> these pages by checking PageLRU because huge zero page isn't lru page as
> non-lru compound movable pages.

It seems that memory_failure() also fails at get_any_page() with "hwpoison:
unhandlable page" message.

  [16478.203474] page:00000000b6acdbd1 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1810b4
  [16478.206612] flags: 0x57ffffc0801000(reserved|hwpoison|node=1|zone=2|lastcpupid=0x1fffff)
  [16478.209411] raw: 0057ffffc0801000 fffff11bc6042d08 fffff11bc6042d08 0000000000000000
  [16478.211921] raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
  [16478.214473] page dumped because: hwpoison: unhandlable page
  [16478.216386] Memory failure: 0x1810b4: recovery action for unknown page: Ignored

We can't handle errors on huge (or normal) zero page, so the current
behavior seems to me more suitable than "unsplit thp".

Or if you have some producer to reach the following path with huge zero
page, could you share it?

Thanks,
Naoya Horiguchi

> 
> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> ---
>  mm/memory-failure.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 23bfd809dc8c..ac6492e36978 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1792,6 +1792,20 @@ int memory_failure(unsigned long pfn, int flags)
>  	}
>  
>  	if (PageTransHuge(hpage)) {
> +		/*
> +		 * The non-lru compound movable pages could be taken for
> +		 * transhuge pages. Also huge zero page could reach here
> +		 * and if we ever try to split it, the VM_BUG_ON_PAGE will
> +		 * be triggered in split_huge_page_to_list(). Skip these
> +		 * pages by checking PageLRU because huge zero page isn't
> +		 * lru page as non-lru compound movable pages.
> +		 */
> +		if (!PageLRU(hpage)) {
> +			put_page(p);
> +			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_IGNORED);
> +			res = -EBUSY;
> +			goto unlock_mutex;
> +		}
>  		/*
>  		 * The flag must be set after the refcount is bumped
>  		 * otherwise it may race with THP split.
> -- 
> 2.23.0