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