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

[email protected]
Newsgroups org.kernel.vger.linux-fsdevel
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 
> 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. Rename 
> is_raw_hwpoison_page_in_hugepage() to hugetlb_page_hwpoison()
> 
> 
> 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.
> 
> 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.
> 
> 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(-)
> 
> 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 *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 (hugetlb_page_hwpoison(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 (hugetlb_page_hwpoison(folio, page))
>   			break;
>   
>   	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
>   
> -/*
> - * 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);
>   
>   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_cgroup.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);
>   
>   #ifdef CONFIG_HUGETLB_PAGE
>   
> +/* 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 folio.
> + * 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;
>   };
>   
> -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 = page_folio(page);
> -	bool ret = false;
> +	const struct hwp_page *p;
> +	unsigned long flags;
>   
>   	if (!folio_test_has_hwpoisoned(folio))
>   		return false;
>   
> -	if (!folio_test_hugetlb(folio))
> -		return PageHWPoison(page);
> +	spin_lock_irqsave(&hwp_page_lock, flags);
>   
>   	/*
> -	 * 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;
> +	}
>   
> -	mutex_lock(&mf_mutex);
> -
> -	raw_hwp_head = raw_hwp_list_head(folio);
> -	llist_for_each_entry(p, raw_hwp_head->first, node) {
> -		if (page == p->page) {
> -			ret = true;
> +	for (p = folio->hugetlb_hwpoison; p; p = p->next) {
> +		if (page == p->page)
>   			break;
> -		}
>   	}
> +	spin_unlock_irqrestore(&hwp_page_lock, flags);
>   
> -	mutex_unlock(&mf_mutex);
> -
> -	return ret;
> +	return p != NULL;
>   }
>   
>   static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag)
>   {
> -	struct llist_node *head;
> -	struct raw_hwp_page *p, *next;
> +	struct hwp_page *p, *next;
>   	unsigned long count = 0;
>   
> -	head = llist_del_all(raw_hwp_list_head(folio));
> -	llist_for_each_entry_safe(p, next, head, node) {
> +	next = folio->hugetlb_hwpoison;
> +	folio->hugetlb_hwpoison = NULL;
> +
> +	while (next) {
> +		p = next;
> +		next = 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 *page)
>   {
> -	struct llist_head *head;
> -	struct raw_hwp_page *raw_hwp;
> -	struct raw_hwp_page *p;
> +	struct hwp_page *p;
> +	unsigned long flags;
>   	int ret = hugetlb_set_poison(folio);
>   
>   	/*
> @@ -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 = raw_hwp_list_head(folio);
> -	llist_for_each_entry(p, head->first, node) {
> +
> +	spin_lock_irqsave(&hwp_page_lock, flags);
> +	for (p = folio->hugetlb_hwpoison; p; p = p->next) {
>   		if (p->page == page)
> -			return MF_HUGETLB_PAGE_PRE_POISONED;
> +			break;
>   	}
>   
> -	raw_hwp = kmalloc_obj(struct raw_hwp_page, GFP_ATOMIC);
> -	if (raw_hwp) {
> -		raw_hwp->page = page;
> -		llist_add(&raw_hwp->node, head);
> +	if (p) {
> +		spin_unlock_irqrestore(&hwp_page_lock, flags);
> +		return MF_HUGETLB_PAGE_PRE_POISONED;
> +	}
> +
> +	p = kmalloc_obj(*p, GFP_ATOMIC);
> +	if (p) {
> +		p->page = page;
> +		p->next = folio->hugetlb_hwpoison;
> +		folio->hugetlb_hwpoison = 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;
>   }
>   
>   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 folio *folio, bool move_flag)
>   	if (folio_test_hugetlb_raw_hwp_unreliable(folio))
>   		return 0;
>   
> -	return __folio_free_raw_hwp(folio, move_flag);
> +	spin_lock_irqsave(&hwp_page_lock, flags);
> +	count = __folio_free_raw_hwp(folio, move_flag);
> +	spin_unlock_irqrestore(&hwp_page_lock, flags);
> +
> +	return count;
>   }
>   
>   void folio_clear_hugetlb_hwpoison(struct folio *folio)
> -- 
> 2.47.3
> 

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

thanks,
-jane
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.