From: Mani Tofigh memory_failure() checks under hugetlb_lock that the page isn't part of a hugetlb folio and sets PG_hwpoison only after dropping the lock. The page can become a tail page of a new hugetlb folio in the meantime. With hugetlb_free_vmemmap=on its struct page then lives in the tail page that all such folios in the zone share, so the flag lands on tail pages of every one of them: Task A (memory_failure) Task B (nr_hugepages write) get_huge_page_for_hwpoison() /* P not hugetlb */ alloc_fresh_hugetlb_folio() /* P becomes a tail page */ hugetlb_vmemmap_optimize_folio() /* P's struct page now in the shared tail page */ TestSetPageHWPoison(P) The error on P itself is ignored, while healthy pages of other folios stay marked and are never reused once their folio is freed. After freeing the pool, /proc/kpageflags shows them as hwpoisoned. This is the allocation side of the race that commit 32c877191e02 ("hugetlb: do not clear hugetlb dtor until allocating vmemmap") fixed for freeing. Set the hugetlb flag of a new folio under hugetlb_lock and set PG_hwpoison of a non-hugetlb page inside the locked check, so that the vmemmap can't be optimized in between. The retry path clears it under the same lock. With the fix, no healthy page gets marked anymore. This bug was identified and confirmed with the help of a deterministic hypervisor currently under development. The C reproducer below however hits it within about 35s. Fixes: f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with each HugeTLB page") Cc: stable@vger.kernel.org Signed-off-by: Mani Tofigh --- Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/hugetlbro.c --- mm/hugetlb.c | 3 +++ mm/memory-failure.c | 37 ++++++++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index ff9dbfbc7a96..eab0d9c5758b 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1770,7 +1770,10 @@ static void account_new_hugetlb_folio(struct hstate *h, struct folio *folio) void init_new_hugetlb_folio(struct folio *folio) { + /* Pairs with get_huge_page_for_hwpoison(), see there. */ + spin_lock_irq(&hugetlb_lock); __folio_set_hugetlb(folio); + spin_unlock_irq(&hugetlb_lock); INIT_LIST_HEAD(&folio->lru); hugetlb_set_folio_subpool(folio, NULL); set_hugetlb_cgroup(folio, NULL); diff --git a/mm/memory-failure.c b/mm/memory-failure.c index b705f9f97f42..087b385aa060 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1976,6 +1976,7 @@ static unsigned long __folio_free_raw_hwp(struct folio *folio, bool move_flag) #define MF_HUGETLB_FOLIO_PRE_POISONED 3 /* folio already poisoned */ #define MF_HUGETLB_PAGE_PRE_POISONED 4 /* exact page already poisoned */ #define MF_HUGETLB_RETRY 5 /* hugepage is busy, retry */ +#define MF_HUGETLB_NON_HUGEPAGE_POISONED 6 /* not a hugepage, already poisoned */ /* * Set hugetlb folio as hwpoisoned, update folio private raw hwpoison list * to keep track of the poisoned pages. @@ -2060,7 +2061,11 @@ static int get_huge_page_for_hwpoison(unsigned long pfn, int flags, spin_lock_irq(&hugetlb_lock); folio = page_folio(page); if (!folio_test_hugetlb(folio)) { - ret = MF_HUGETLB_NON_HUGEPAGE; + /* Mark it before it can become part of a new hugetlb folio. */ + if (TestSetPageHWPoison(page)) + ret = MF_HUGETLB_NON_HUGEPAGE_POISONED; + else + ret = MF_HUGETLB_NON_HUGEPAGE; goto out_unlock; } else if (flags & MF_COUNT_INCREASED) { ret = MF_HUGETLB_IN_USED; @@ -2111,7 +2116,8 @@ static int get_huge_page_for_hwpoison(unsigned long pfn, int flags, * PageHWPoison) should be done in single hugetlb_lock range. * Returns: * 0 - recovered - * -ENOENT - no hugetlb page + * -ENOENT - no hugetlb page, now marked hwpoison + * -EEXIST - no hugetlb page, already marked hwpoison * -EBUSY - not recovered * -EOPNOTSUPP - hwpoison_filter'ed * -EHWPOISON - folio or exact page already poisoned @@ -2130,6 +2136,8 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags) switch (res) { case MF_HUGETLB_NON_HUGEPAGE: /* fallback to normal page handling */ return -ENOENT; + case MF_HUGETLB_NON_HUGEPAGE_POISONED: + return -EEXIST; case MF_HUGETLB_RETRY: if (!(flags & MF_NO_RETRY)) { flags |= MF_NO_RETRY; @@ -2189,10 +2197,24 @@ static int try_memory_failure_hugetlb(unsigned long pfn, int flags) return identify_page_state(pfn, p, page_flags); } +/* Keep the flag if the page has become hugetlb in the meantime. */ +static void clear_non_hugetlb_hwpoison(struct page *page) +{ + spin_lock_irq(&hugetlb_lock); + if (!folio_test_hugetlb(page_folio(page))) + ClearPageHWPoison(page); + spin_unlock_irq(&hugetlb_lock); +} + #else static inline int try_memory_failure_hugetlb(unsigned long pfn, int flags) { - return -ENOENT; + return TestSetPageHWPoison(pfn_to_page(pfn)) ? -EEXIST : -ENOENT; +} + +static inline void clear_non_hugetlb_hwpoison(struct page *page) +{ + ClearPageHWPoison(page); } static inline unsigned long folio_free_raw_hwp(struct folio *folio, bool flag) @@ -2470,12 +2492,13 @@ int memory_failure(unsigned long pfn, int flags) try_again: res = try_memory_failure_hugetlb(pfn, flags); /* - * -ENOENT means the page we found is not hugetlb, so proceed with normal page handling + * -ENOENT and -EEXIST mean the page is not hugetlb and is now marked + * hwpoison, so proceed with normal page handling. */ - if (res != -ENOENT) + if (res != -ENOENT && res != -EEXIST) goto unlock_mutex; - if (TestSetPageHWPoison(p)) { + if (res == -EEXIST) { res = -EHWPOISON; if (flags & MF_ACTION_REQUIRED) res = kill_accessing_process(current, pfn, flags); @@ -2506,7 +2529,7 @@ int memory_failure(unsigned long pfn, int flags) } else { /* We lost the race, try again */ if (retry) { - ClearPageHWPoison(p); + clear_non_hugetlb_hwpoison(p); retry = false; goto try_again; } --- base-commit: 5e6c45aaaf93343c7f8a1cde9797080238af5c1b change-id: 20261030-hwpoison-hugetlb-ro-5f8881a60af9 Best regards, -- Mani Tofigh