Re: [PATCH v9 11/15] filemap: Add hwpoison handling to filemap_read()

On Wed, Aug 05, 2026 at 10:05:51PM +0100, Matthew Wilcox (Oracle) wrote:
> From: Jane Chu <[email protected]>
> 
> Add hwpoison handling to filemap_read() such that .read_iter() could
> make best effort copying data out of clean pages without risking
> MCE in case page cache contains HWpoison.
> 
> Signed-off-by: Jane Chu <[email protected]>
> Co-developed-by: Matthew Wilcox <[email protected]>
> Signed-off-by: Matthew Wilcox <[email protected]>
> ---
>  include/linux/hugetlb.h    |  2 --
>  include/linux/page-flags.h | 11 +++++++++++
>  mm/filemap.c               | 33 +++++++++++++++++++++++++++++++--
>  3 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index ec604cee8d22..639c0a772856 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1088,8 +1088,6 @@ void hugetlb_register_node(struct node *node);
>  void hugetlb_unregister_node(struct node *node);
>  #endif
>  
> -bool hugetlb_page_hwpoison(const struct folio *folio, const struct page *page);
> -
>  static inline unsigned long huge_page_mask_align(struct file *file)
>  {
>  	return PAGE_MASK & ~huge_page_mask(hstate_file(file));
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 5d01e5b28d0f..f75d66c42509 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -1095,6 +1095,7 @@ static inline bool PageHuge(const struct page *page)
>  	return folio_test_hugetlb(page_folio(page));
>  }
>  
> +bool hugetlb_page_hwpoison(const struct folio *folio, const struct page *page);
>  bool hugetlb_unref_page_hwpoison(const struct page *page);
>  
>  /*
> @@ -1116,6 +1117,16 @@ static inline bool is_page_hwpoison(const struct page *page)
>  	return PageHWPoison(page);
>  }
>  
> +static inline bool is_ref_page_hwpoison(const struct folio *folio,
> +		const struct page *page)
> +{
> +	if (PageHWPoison(page))
> +		return true;
> +	if (folio_test_hugetlb(folio))
> +		return hugetlb_page_hwpoison(folio, page);
> +	return false;
> +}
> +
>  static inline bool folio_has_hwpoisoned_page(const struct folio *folio)
>  {
>  	return PageHWPoison(&folio->page) ||
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 58eb9d240643..26a5f18121f9 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2483,6 +2483,8 @@ static void filemap_get_read_batch(struct address_space *mapping,
>  
>  		if (!folio_batch_add(fbatch, folio))
>  			break;
> +		if (folio_has_hwpoisoned_page(folio))
> +			break;
>  		if (!folio_test_uptodate(folio))
>  			break;
>  		if (folio_test_readahead(folio))
> @@ -2749,6 +2751,29 @@ static inline bool pos_same_folio(loff_t pos1, loff_t pos2, struct folio *folio)
>  	return (pos1 >> shift == pos2 >> shift);
>  }
>  
> +static size_t adjust_range_hwpoison(const struct folio *folio, size_t offset,
> +		size_t bytes)
> +{
> +	const struct page *page = folio_page(folio, offset / PAGE_SIZE);
> +	size_t safe_bytes;
						    offset >> PAGE_SHIFT would probably be more idiomatic
> +
> +	if (!folio_has_hwpoisoned_page(folio))
> +		return bytes;
> +	if (is_ref_page_hwpoison(folio, page))
> +		return 0;
> +
> +	/* Safe to read the remaining bytes in this page. */
> +	safe_bytes = PAGE_SIZE - (offset % PAGE_SIZE);
			similarly, offset_in_page(offset)

(yes, no codegen difference in both cases, but there could be a difference
if the ARM dynamic page size stuff ever moves forward)

> +	page++;
> +
> +	/* Check each remaining page as long as we are not done yet. */
> +	for (; safe_bytes < bytes; safe_bytes += PAGE_SIZE, page++)
> +		if (is_ref_page_hwpoison(folio, page))
> +			break;
> +
> +	return min(safe_bytes, bytes);
> +}

Wouldn't this whole logic fit better in copy_folio_to_iter? shmem for
instance also handrolls its own hwpoison read_iter logic with
per-page-copies, etc; not amazing.

> +
>  static void filemap_end_dropbehind_read(struct folio *folio)
>  {
>  	if (!folio_test_dropbehind(folio))
> @@ -2862,14 +2887,18 @@ ssize_t filemap_read(struct kiocb *iocb, struct iov_iter *iter,
>  			if (writably_mapped)
>  				flush_dcache_folio(folio);
>  
> -			copied = copy_folio_to_iter(folio, offset, bytes, iter);
> +			copied = adjust_range_hwpoison(folio, offset, bytes);
> +			if (copied < bytes)
> +				error = -EIO;

I don't love reusing copied here, but I guess there isn't much better
(in order to detect hwpoison-derived truncation)

> +			copied = copy_folio_to_iter(folio, offset, copied, iter);
>  
>  			already_read += copied;
>  			iocb->ki_pos += copied;
>  			last_pos = iocb->ki_pos;
>  
>  			if (copied < bytes) {
> -				error = -EFAULT;
> +				if (!error)
> +					error = -EFAULT;

Logic looks good though.

-- 
Pedro