Re: [PATCH v4 01/12] mm/rmap: convert page -> folio for hwpoison checks

[David Hildenbrand (Arm)]

On 5/26/26 08:36, Dev Jain wrote:
> If any page of a folio is poisoned, try_to_unmap() is called with the
> intent to remove any mappings pointing to any part of the folio.
> 
> Therefore, an hwpoison check on a single page does not make sense
> (although everything works out because the memory-failure path sets
> hwpoison on the head page too). Convert these checks to check whether
> the folio has at least one hwpoisoned page.

I think the reasoning is different and should better be described:

1) For hugetlb, we will always get subpage == head page (single PTE -> first PFN).

try_memory_failure_hugetlb()->try_memory_failure_hugetlb()->hugetlb_update_hwpoison()
will do the folio_test_set_hwpoison().

folio_test_set_hwpoison() is the same as setting it on the head page.

For hugetlb, subpage==headpage

That's why PageHWPoison(subpage) used to work.


2) try_to_unmap() cannot handle large folios (besides hugetlb folios).

unmap_poisoned_folio() spells that out. That's why subpage==headpage right now.


That's why PageHWPoison(subpage) used to work.

> 
> While at it, convert VM_BUG_* to VM_WARN_*.
> 
> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
> ---
>  mm/rmap.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/rmap.c b/mm/rmap.c
> index fb3c351f8c458..430c91c8fe2ae 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -2116,10 +2116,11 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  			bool anon = folio_test_anon(folio);
>  
>  			/*
> -			 * The try_to_unmap() is only passed a hugetlb page
> -			 * in the case where the hugetlb page is poisoned.
> +			 * The try_to_unmap() is only passed a hugetlb folio
> +			 * in the case where the hugetlb folio contains a
> +			 * poisoned page.
>  			 */
> -			VM_BUG_ON_PAGE(!PageHWPoison(subpage), subpage);
> +			VM_WARN_ON_FOLIO(!folio_contain_hwpoisoned_page(folio), folio);

I think we could also just use folio_test_hwpoison() here.

>  			/*
>  			 * huge_pmd_unshare may unmap an entire PMD page.
>  			 * There is no way of knowing exactly which PMDs may
> @@ -2198,7 +2199,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
>  		/* Update high watermark before we lower rss */
>  		update_hiwater_rss(mm);
>  
> -		if (PageHWPoison(subpage) && (flags & TTU_HWPOISON)) {
> +		if (folio_contain_hwpoisoned_page(folio) && (flags & TTU_HWPOISON)) {
>  			pteval = swp_entry_to_pte(make_hwpoison_entry(subpage));
>  			if (folio_test_hugetlb(folio)) {
>  				hugetlb_count_sub(folio_nr_pages(folio), mm);

And here. Which might be cleaner, because folio_contain_hwpoisoned_page() is so
far never called on hugetlb folios.

Because folio_set_has_hwpoisoned() is only ever used on THPs, not hugetlb. It
would currently work, but the expectation so far is that these primitives are
for small folios+thp, not hugetlb.

You could add a comment here, that with TTU_HWPOISON, we only ever expect small
folios or hugetlb folios for now.

-- 
Cheers,

David