On 12/23/25 02:21, Jane Chu wrote:
When a newly poisoned subpage ends up in an already poisoned hugetlb folio, 'num_poisoned_pages' is incremented, but the per node ->mf_stats is not. Fix the inconsistency by designating action_result() to update them both.
While at it, define __get_huge_page_for_hwpoison() return values in terms of symbol names for better readibility. Also rename folio_set_hugetlb_hwpoison() to hugetlb_update_hwpoison() since the function does more than the conventional bit setting and the fact three possible return values are expected.
Fixes: 18f41fa616ee4 ("mm: memory-failure: bump memory failure stats to pglist_data") Cc: stable@vger.kernel.org Signed-off-by: Jane Chu jane.chu@oracle.com
v2 -> v3: No change. v1 -> v2: adapted David and Liam's comment, define __get_huge_page_for_hwpoison() return values in terms of symbol names instead of naked integers for better readibility. #define instead of enum is used since the function has footprint outside MF, just try to limit the MF specifics local. also renamed folio_set_hugetlb_hwpoison() to hugetlb_update_hwpoison() since the function does more than the conventional bit setting and the fact three possible return values are expected.
mm/memory-failure.c | 56 ++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 23 deletions(-)
diff --git a/mm/memory-failure.c b/mm/memory-failure.c index fbc5a01260c8..8b47e8a1b12d 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1883,12 +1883,18 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag) return count; } -static int folio_set_hugetlb_hwpoison(struct folio *folio, struct page *page) +#define MF_HUGETLB_ALREADY_POISONED 3 /* already poisoned */ +#define MF_HUGETLB_ACC_EXISTING_POISON 4 /* accessed existing poisoned page */
What happened to the idea of using an enum?
+/*
- Set hugetlb folio as hwpoisoned, update folio private raw hwpoison list
- to keep track of the poisoned pages.
- */
+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;
- int ret = folio_test_set_hwpoison(folio) ? -EHWPOISON : 0;
- int ret = folio_test_set_hwpoison(folio) ? MF_HUGETLB_ALREADY_POISONED : 0;
/* * Once the hwpoison hugepage has lost reliable raw error info, @@ -1896,20 +1902,18 @@ static int folio_set_hugetlb_hwpoison(struct folio *folio, struct page *page) * so skip to add additional raw error info. */ if (folio_test_hugetlb_raw_hwp_unreliable(folio))
return -EHWPOISON;
return MF_HUGETLB_ALREADY_POISONED;- head = raw_hwp_list_head(folio); llist_for_each_entry(p, head->first, node) { if (p->page == page)
return -EHWPOISON;
return MF_HUGETLB_ACC_EXISTING_POISON;raw_hwp = kmalloc(sizeof(struct raw_hwp_page), GFP_ATOMIC); if (raw_hwp) { raw_hwp->page = page; llist_add(&raw_hwp->node, head);
/* the first error event will be counted in action_result(). */if (ret)num_poisoned_pages_inc(page_to_pfn(page));
- Failed to save raw error info. We no longer trace all
@@ -1955,32 +1959,30 @@ void folio_clear_hugetlb_hwpoison(struct folio *folio) folio_free_raw_hwp(folio, true); } +#define MF_HUGETLB_FREED 0 /* freed hugepage */ +#define MF_HUGETLB_IN_USED 1 /* in-use hugepage */ +#define MF_NOT_HUGETLB 2 /* not a hugepage */
If you're already dealing with negative error codes, "MF_NOT_HUGETLB" nicely translated to -EINVAL.
But I wonder if it would be cleaner to just define all values in an enum and return that enum instead of an int from the functions.
enum md_hugetlb_status { MF_HUGETLB_INVALID, /* not a hugetlb folio */ MF_HUGETLB_BUSY, /* busy, retry later */ MF_HUGETLB_FREED, /* hugetlb folio was freed */ MF_HUGETLB_IN_USED, /* ??? no idea what that really means */ MF_HUGETLB_FOLIO_PRE_POISONED, /* folio already poisoned, per-page information unclear */ MF_HUGETLB_PAGE_PRE_POISONED, /* exact page already poisoned */ }