The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <ljs@kernel.org>
To: Rik van Riel <riel@surriel.com>
Cc: linux-kernel@vger.kernel.org, kernel-team@meta.com,
	linux-mm@kvack.org,  akpm@linux-foundation.org, david@kernel.org,
	ziy@nvidia.com, baolin.wang@linux.alibaba.com,
	 liam@infradead.org, npache@redhat.com, ryan.roberts@arm.com,
	dev.jain@arm.com,  baohua@kernel.org, lance.yang@linux.dev,
	yang@os.amperecomputing.com
Subject: Re: [PATCH] mm/huge_memory: set PG_has_hwpoisoned only after new folio head is established
Date: Wed, 1 Jul 2026 15:52:39 +0100	[thread overview]
Message-ID: <akUoqvmGtSMgD0QT@lucifer> (raw)
In-Reply-To: <20260701134622.3152896-1-riel@surriel.com>

On Wed, Jul 01, 2026 at 09:46:22AM -0400, Rik van Riel wrote:
> __split_folio_to_order() copies the hwpoison state onto each new
> sub-folio while splitting a folio to a non-zero order.  It did so via
>
> 	if (handle_hwpoison && page_range_has_hwpoisoned(new_head, new_nr_pages))
> 		folio_set_has_hwpoisoned(new_folio);
>
> *before* clear_compound_head(new_head)/prep_compound_page(new_head, ...)
> turn @new_head from a tail page into a proper folio head.
>
> PG_has_hwpoisoned is a FOLIO_SECOND_PAGE flag, so folio_set_has_hwpoisoned()
> resolves to folio_flags(folio, 1).  With the new compound_info-based
> page-flags layout, folio_flags() asserts the page is not a tail:
>
> 	VM_BUG_ON_PGFLAGS(page->compound_info & 1, page);
> 	VM_BUG_ON_PGFLAGS(n > 0 && !test_bit(PG_head, &page->flags.f), page);
>
> At the original call site @new_head still has the tail marker
> (compound_info bit 0 set, PG_head clear), so on CONFIG_DEBUG_VM kernels
> this hits:
>
>   kernel BUG at include/linux/page-flags.h:354
>   folio_flags+0x82
>   folio_set_has_hwpoisoned
>   __split_folio_to_order
>   __split_unmapped_folio
>   __folio_split
>   truncate_inode_partial_folio  (shmem hole-punch / MADV_REMOVE)
>
> Reproduced by syzkaller: hwpoison-inject a few subpages of a large shmem
> folio, then MADV_REMOVE (fallocate punch hole) on the same range, which
> splits the partial folio to a non-zero order.
>
> Move the folio_set_has_hwpoisoned() call to after
> clear_compound_head()/prep_compound_page(), where @new_folio is a real
> order-new_order head folio (handle_hwpoison implies new_order != 0, so a
> second page always exists).  The flag still lands on the same struct page
> (page[1] of the new folio); only the ordering relative to compound-head
> setup changes, satisfying the FOLIO_SECOND_PAGE precondition.
>
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Assisted-by: Claude:claude-opus-4-8
> Fixes: fa5a06170036 ("mm/huge_memory: preserve PG_has_hwpoisoned if a folio is split to >0 order")
> ---
>  mm/huge_memory.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2bccb0a53a0a..ee7ecb3b45c6 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3587,10 +3587,6 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
>  				 (1L << PG_dropbehind) |
>  				 LRU_GEN_MASK | LRU_REFS_MASK));
>
> -		if (handle_hwpoison &&
> -		    page_range_has_hwpoisoned(new_head, new_nr_pages))
> -			folio_set_has_hwpoisoned(new_folio);
> -
>  		new_folio->mapping = folio->mapping;
>  		new_folio->index = folio->index + i;
>
> @@ -3612,6 +3608,18 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
>  			folio_set_large_rmappable(new_folio);
>  		}
>
> +		/*
> +		 * PG_has_hwpoisoned is a FOLIO_SECOND_PAGE flag, so it can only
> +		 * be set once @new_folio is a real (head) folio.  Defer setting
> +		 * it until after clear_compound_head()/prep_compound_page() have
> +		 * turned @new_head from a tail page into a proper folio head;
> +		 * otherwise folio_flags() trips on (page->compound_info & 1).
> +		 * handle_hwpoison implies new_order != 0.
> +		 */

This reads like an LLM comment...! A ton of noise and unnecessary detail.

How about:

		/*
		 * PG_has_hwpoisoned is on the 2nd page, so set it after
		 * compound head prepped.
		 */

?

> +		if (handle_hwpoison &&
> +		    page_range_has_hwpoisoned(new_head, new_nr_pages))
> +			folio_set_has_hwpoisoned(new_folio);
> +
>  		if (folio_test_young(folio))
>  			folio_set_young(new_folio);
>  		if (folio_test_idle(folio))
> --
> 2.53.0-Meta
>

Thanks, Lorenzo

  parent reply	other threads:[~2026-07-01 14:52 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 13:46 [PATCH] mm/huge_memory: set PG_has_hwpoisoned only after new folio head is established Rik van Riel
2026-07-01 14:34 ` Zi Yan
2026-07-01 14:52 ` Lorenzo Stoakes [this message]
2026-07-01 16:29   ` Rik van Riel
2026-07-01 15:24 ` Lance Yang
2026-07-01 16:33 ` David Hildenbrand (Arm)
2026-07-01 17:24   ` Rik van Riel
2026-07-02  2:29     ` Lance Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akUoqvmGtSMgD0QT@lucifer \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=kernel-team@meta.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npache@redhat.com \
    --cc=riel@surriel.com \
    --cc=ryan.roberts@arm.com \
    --cc=yang@os.amperecomputing.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox