Re: [PATCH v8 09/15] mm: Remove locking mf_mutex in is_raw_hwpoison_page_in_hugepage()

[email protected] Mon, 3 Aug 2026 23:56:57 -0700
Newsgroups gmane.linux.file-systems,gmane.linux.kernel.mm
Message-ID <[email protected]>

On 7/31/2026 1:07 PM, Matthew Wilcox (Oracle) wrote:
> Sleeping in this kind of predicate is unexpected. Add a new spinlock to=20
> protect access to the list, and turn it into a normal singly linked list=
=20
> now that it doesn't need to be a lockless list. Rename=20
> is_raw_hwpoison_page_in_hugepage() to hugetlb_page_hwpoison()
>=20
>=20
> Sleeping in this kind of predicate is unexpected.  Add a new spinlock
> to protect access to the list, and turn it into a normal singly linked
> list now that it doesn't need to be a lockless list.
>=20
> Rename is_raw_hwpoison_page_in_hugepage() to hugetlb_page_hwpoison()
> and make it take the folio (since the callers naturally have the folio).
> Also remove the handling of non-hugetlb folios and make the arguments
> const.
>=20
> Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
> ---
>   fs/hugetlbfs/inode.c     |   4 +-
>   include/linux/hugetlb.h  |   5 +-
>   include/linux/mm_types.h |   4 +-
>   mm/memory-failure.c      | 101 ++++++++++++++++++++++-----------------
>   4 files changed, 61 insertions(+), 53 deletions(-)
>=20
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index fbac554886c3..dbca3f713bbf 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -198,7 +198,7 @@ static size_t adjust_range_hwpoison(struct folio *fol=
io, size_t offset,
>   	struct page *page =3D folio_page(folio, offset / PAGE_SIZE);
>   	size_t safe_bytes;
>  =20
> -	if (is_raw_hwpoison_page_in_hugepage(page))
> +	if (hugetlb_page_hwpoison(folio, page))
>   		return 0;
>   	/* Safe to read the remaining bytes in this page. */
>   	safe_bytes =3D PAGE_SIZE - (offset % PAGE_SIZE);
> @@ -206,7 +206,7 @@ static size_t adjust_range_hwpoison(struct folio *fol=
io, size_t offset,
>  =20
>   	/* Check each remaining page as long as we are not done yet. */
>   	for (; safe_bytes < bytes; safe_bytes +=3D PAGE_SIZE, page++)
> -		if (is_raw_hwpoison_page_in_hugepage(page))
> +		if (hugetlb_page_hwpoison(folio, page))
>   			break;
>  =20
>   	return min(safe_bytes, bytes);
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index e200c93088bc..ec604cee8d22 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -1088,10 +1088,7 @@ void hugetlb_register_node(struct node *node);
>   void hugetlb_unregister_node(struct node *node);
>   #endif
>  =20
> -/*
> - * Check if a given raw @page in a hugepage is HWPOISON.
> - */
> -bool is_raw_hwpoison_page_in_hugepage(struct page *page);
> +bool hugetlb_page_hwpoison(const struct folio *folio, const struct page =
*page);
>  =20
>   static inline unsigned long huge_page_mask_align(struct file *file)
>   {
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index b18c2b2e7d2c..86a2d0fee557 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -387,7 +387,7 @@ typedef unsigned short mm_id_t;
>    * @_hugetlb_subpool: Do not use directly, use accessor in hugetlb.h.
>    * @_hugetlb_cgroup: Do not use directly, use accessor in hugetlb_cgrou=
p.h.
>    * @_hugetlb_cgroup_rsvd: Do not use directly, use accessor in hugetlb_=
cgroup.h.
> - * @_hugetlb_hwpoison: Do not use directly, call raw_hwp_list_head().
> + * @hugetlb_hwpoison: List of pages with hwpoison.
>    * @_deferred_list: Folios to be split under memory pressure.
>    * @_unused_slab_obj_exts: Placeholder to match obj_exts in struct slab.
>    *
> @@ -499,7 +499,7 @@ struct folio {
>   			void *_hugetlb_subpool;
>   			void *_hugetlb_cgroup;
>   			void *_hugetlb_cgroup_rsvd;
> -			void *_hugetlb_hwpoison;
> +			struct hwp_page *hugetlb_hwpoison;
>   	/* private: the union with struct page is transitional */
>   		};
>   		struct page __page_3;
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 714e1b398f2c..437a2be6c58c 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1808,63 +1808,60 @@ EXPORT_SYMBOL_GPL(mf_dax_kill_procs);
>  =20
>   #ifdef CONFIG_HUGETLB_PAGE
>  =20
> +/* Protects all lists of hwp_pages */
> +static DEFINE_SPINLOCK(hwp_page_lock);
> +
>   /*
> - * Struct raw_hwp_page represents information about "raw error page",
> - * constructing singly linked list from ->_hugetlb_hwpoison field of fol=
io.
> + * hwp_page represents information about "error page",
> + * constructing singly linked list from folio->hugetlb_hwpoison field.
>    */
> -struct raw_hwp_page {
> -	struct llist_node node;
> +struct hwp_page {
> +	struct hwp_page *next;
>   	struct page *page;
>   };
>  =20
> -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)
> +/*
> + * Check if a given @page in a hugetlb folio is HWPOISON.
> + */
> +bool hugetlb_page_hwpoison(const struct folio *folio, const struct page =
*page)
>   {
> -	struct llist_head *raw_hwp_head;
> -	struct raw_hwp_page *p;
> -	struct folio *folio =3D page_folio(page);
> -	bool ret =3D false;
> +	const struct hwp_page *p;
> +	unsigned long flags;
>  =20
>   	if (!folio_test_has_hwpoisoned(folio))
>   		return false;
>  =20
> -	if (!folio_test_hugetlb(folio))
> -		return PageHWPoison(page);
> +	spin_lock_irqsave(&hwp_page_lock, flags);
>  =20
>   	/*
> -	 * When RawHwpUnreliable is set, kernel lost track of which subpages
> -	 * are HWPOISON. So return as if ALL subpages are HWPOISONed.
> +	 * When RawHwpUnreliable is set, kernel lost track of which pages
> +	 * are HWPOISON. So return as if ALL pages are HWPOISONed.
>   	 */
> -	if (folio_test_hugetlb_raw_hwp_unreliable(folio))
> +	if (folio_test_hugetlb_raw_hwp_unreliable(folio)) {
> +		spin_unlock_irqrestore(&hwp_page_lock, flags);
>   		return true;
> +	}
>  =20
> -	mutex_lock(&mf_mutex);
> -
> -	raw_hwp_head =3D raw_hwp_list_head(folio);
> -	llist_for_each_entry(p, raw_hwp_head->first, node) {
> -		if (page =3D=3D p->page) {
> -			ret =3D true;
> +	for (p =3D folio->hugetlb_hwpoison; p; p =3D p->next) {
> +		if (page =3D=3D p->page)
>   			break;
> -		}
>   	}
> +	spin_unlock_irqrestore(&hwp_page_lock, flags);
>  =20
> -	mutex_unlock(&mf_mutex);
> -
> -	return ret;
> +	return p !=3D NULL;
>   }
>  =20
>   static unsigned long __folio_free_raw_hwp(struct folio *folio, bool mov=
e_flag)
>   {
> -	struct llist_node *head;
> -	struct raw_hwp_page *p, *next;
> +	struct hwp_page *p, *next;
>   	unsigned long count =3D 0;
>  =20
> -	head =3D llist_del_all(raw_hwp_list_head(folio));
> -	llist_for_each_entry_safe(p, next, head, node) {
> +	next =3D folio->hugetlb_hwpoison;
> +	folio->hugetlb_hwpoison =3D NULL;
> +
> +	while (next) {
> +		p =3D next;
> +		next =3D p->next;
>   		if (move_flag)
>   			SetPageHWPoison(p->page);
>   		else
> @@ -1905,9 +1902,8 @@ static inline int hugetlb_clear_poison(struct folio=
 *folio)
>    */
>   static int hugetlb_update_hwpoison(struct folio *folio, struct page *pa=
ge)
>   {
> -	struct llist_head *head;
> -	struct raw_hwp_page *raw_hwp;
> -	struct raw_hwp_page *p;
> +	struct hwp_page *p;
> +	unsigned long flags;
>   	int ret =3D hugetlb_set_poison(folio);
>  =20
>   	/*
> @@ -1917,16 +1913,23 @@ static int hugetlb_update_hwpoison(struct folio *=
folio, struct page *page)
>   	 */
>   	if (folio_test_hugetlb_raw_hwp_unreliable(folio))
>   		return MF_HUGETLB_FOLIO_PRE_POISONED;
> -	head =3D raw_hwp_list_head(folio);
> -	llist_for_each_entry(p, head->first, node) {
> +
> +	spin_lock_irqsave(&hwp_page_lock, flags);
> +	for (p =3D folio->hugetlb_hwpoison; p; p =3D p->next) {
>   		if (p->page =3D=3D page)
> -			return MF_HUGETLB_PAGE_PRE_POISONED;
> +			break;
>   	}
>  =20
> -	raw_hwp =3D kmalloc_obj(struct raw_hwp_page, GFP_ATOMIC);
> -	if (raw_hwp) {
> -		raw_hwp->page =3D page;
> -		llist_add(&raw_hwp->node, head);
> +	if (p) {
> +		spin_unlock_irqrestore(&hwp_page_lock, flags);
> +		return MF_HUGETLB_PAGE_PRE_POISONED;
> +	}
> +
> +	p =3D kmalloc_obj(*p, GFP_ATOMIC);
> +	if (p) {
> +		p->page =3D page;
> +		p->next =3D folio->hugetlb_hwpoison;
> +		folio->hugetlb_hwpoison =3D p;
>   	} else {
>   		/*
>   		 * Failed to save raw error info.  We no longer trace all
> @@ -1935,16 +1938,20 @@ static int hugetlb_update_hwpoison(struct folio *=
folio, struct page *page)
>   		 */
>   		folio_set_hugetlb_raw_hwp_unreliable(folio);
>   		/*
> -		 * Once hugetlb_raw_hwp_unreliable is set, raw_hwp_page is not
> +		 * Once hugetlb_raw_hwp_unreliable is set, hwp_page is not
>   		 * used any more, so free it.
>   		 */
>   		__folio_free_raw_hwp(folio, false);
>   	}
> +	spin_unlock_irqrestore(&hwp_page_lock, flags);
>   	return ret;
>   }
>  =20
>   static unsigned long folio_free_raw_hwp(struct folio *folio, bool move_=
flag)
>   {
> +	unsigned long count;
> +	unsigned long flags;
> +
>   	/*
>   	 * hugetlb_vmemmap_optimized hugepages can't be freed because struct
>   	 * pages for tail pages are required but they don't exist.
> @@ -1959,7 +1966,11 @@ static unsigned long folio_free_raw_hwp(struct fol=
io *folio, bool move_flag)
>   	if (folio_test_hugetlb_raw_hwp_unreliable(folio))
>   		return 0;
>  =20
> -	return __folio_free_raw_hwp(folio, move_flag);
> +	spin_lock_irqsave(&hwp_page_lock, flags);
> +	count =3D __folio_free_raw_hwp(folio, move_flag);
> +	spin_unlock_irqrestore(&hwp_page_lock, flags);
> +
> +	return count;
>   }
>  =20
>   void folio_clear_hugetlb_hwpoison(struct folio *folio)
> --=20
> 2.47.3
>=20

Reviewed-by: Jane Chu <[email protected]>

thanks,
-jane