Re: [PATCH v2 01/11] mm/memory-failure: make is_raw_hwpoison_page_in_hugepage() general purpose

["David Hildenbrand (Arm)" <david@kernel.org>]

On 6/17/26 19:25, Jane Chu wrote:
> Make is_raw_hwpoison_page_in_hugepage() general for checking whether
> a given raw page within any kind of folio is HW poisoned. Thus,
> replace folio_test_hwpoison() with folio_contain_hwpoisoned_page().
> Also rename to is_raw_hwpoison_page_in_folio().
> 
> Signed-off-by: Jane Chu <jane.chu@oracle.com>
> ---
>  fs/hugetlbfs/inode.c    |  4 ++--
>  include/linux/hugetlb.h |  4 ++--
>  mm/memory-failure.c     | 12 ++++++++++--
>  3 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 78d61bf2bd9b..66520f7c53c6 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -198,7 +198,7 @@ static size_t adjust_range_hwpoison(struct folio *folio, size_t offset,
>  	struct page *page = folio_page(folio, offset / PAGE_SIZE);
>  	size_t safe_bytes;
>  
> -	if (is_raw_hwpoison_page_in_hugepage(page))
> +	if (is_raw_hwpoison_page_in_folio(page))
>  		return 0;
>  	/* Safe to read the remaining bytes in this page. */
>  	safe_bytes = PAGE_SIZE - (offset % PAGE_SIZE);
> @@ -206,7 +206,7 @@ static size_t adjust_range_hwpoison(struct folio *folio, size_t offset,
>  
>  	/* Check each remaining page as long as we are not done yet. */
>  	for (; safe_bytes < bytes; safe_bytes += PAGE_SIZE, page++)
> -		if (is_raw_hwpoison_page_in_hugepage(page))
> +		if (is_raw_hwpoison_page_in_folio(page))
>  			break;
>  
>  	return min(safe_bytes, bytes);
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 5957bc25efa8..a9846f043712 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1079,9 +1079,9 @@ void hugetlb_unregister_node(struct node *node);
>  #endif
>  
>  /*
> - * Check if a given raw @page in a hugepage is HWPOISON.
> + * Check if a given raw @page is HWPOISON in a folio of any kind
>   */
> -bool is_raw_hwpoison_page_in_hugepage(struct page *page);
> +bool is_raw_hwpoison_page_in_folio(struct page *page);
>  
>  static inline unsigned long huge_page_mask_align(struct file *file)
>  {
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index ee42d4361309..40129e0b8213 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1834,14 +1834,21 @@ static inline struct llist_head *raw_hwp_list_head(struct folio *folio)
>  	return (struct llist_head *)&folio->_hugetlb_hwpoison;
>  }
>  
> -bool is_raw_hwpoison_page_in_hugepage(struct page *page)
> +/**
> + * is_raw_hwpoison_page_in_folio - answers the question whether a given
> + * page is indeed hwpoisoned.
> + * @page: given page, maybe base page, part of a large folio or hugetlb.
> + *
> + * Return: true if @page is the raw hwpoisoned page; else, false.
> + */

Why do we need the "in_folio" part at all?

> +bool is_raw_hwpoison_page_in_folio(struct page *page)
>  {
>  	struct llist_head *raw_hwp_head;
>  	struct raw_hwp_page *p;
>  	struct folio *folio = page_folio(page);
>  	bool ret = false;
>  
> -	if (!folio_test_hwpoison(folio))
> +	if (!folio_contain_hwpoisoned_page(folio))

I wonder if we should just not call that function for hugetlb, it doesn't make
sense as hugetlb doesn't set _has_hwpoisoned.

But then, I wonder if we really need folio_contain_hwpoisoned_page() at all?

Why not a simple:

if (!folio_test_hugetlb(folio))
	return PageHWPoison(page);

And now I am confused which scenario you are worried about (it's warm here ...)
can you explain which scenario you want to change?

>  		return false;
>  
>  	if (!folio_test_hugetlb(folio))
> @@ -1868,6 +1875,7 @@ bool is_raw_hwpoison_page_in_hugepage(struct page *page)
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(is_raw_hwpoison_page_in_folio);

You should spell out why you export that function in the patch description.

-- 
Cheers,

David