Re: [PATCH v3] mm/memory-failure: set PG_hwpoison of a non-hugetlb page under hugetlb_lock

[Mani Tofigh <mt4022@columbia.edu>]

On 11/9/26 2:23 AM, Miaohe Lin wrote:
> I have no idea why the error on P itself is ignored, could you explain that?


My bad, I should have explained that.  By then P is part of a hugetlb
folio that's still free or being set up, so get_hwpoison_page() can't
take it and we just end with "Ignored".  Nothing marks the folio, so
the bad page goes back to the pool and gets handed out later.

If the folio is already in use it's even worse... it gets split as a
THP and hangs.  (That's a different race though, I'll send a separate 
fix for that soon.)
> I think we should clear the PageHWPoison flag we just set since the retry path expects
> we have cleared the PageHWPoison flag in the first round.


Agreed, as long as the page still isn't hugetlb.  Once it is, HVO may
have remapped its struct page, so the clear either hits the shared tail
page (read-only on e.g. 6.6) or gets lost while HVO copies the first
vmemmap page.

So how about clearing it under hugetlb_lock only if the page still isn't
hugetlb, and otherwise skipping the retry like the second round does?

> I suspect this won’t solve the problem. The below race might still exist:
> 
>    Task A (memory_failure)              Task B (nr_hugepages write)
>    get_huge_page_for_hwpoison()         alloc_fresh_hugetlb_folio()
>      /* P not hugetlb */                  /* P becomes a tail page */
>    clear_non_hugetlb_hwpoison
>    goto retry
>    get_huge_page_for_hwpoison()
>      /* P not hugetlb */
> 					 __folio_set_hugetlb
>                                           hugetlb_vmemmap_optimize_folio()
>                                             /* P's struct page now in
>                                                the shared tail page */
>    TestSetPageHWPoison(P)


I think that one's covered.  With the patch, the "not hugetlb" check and
TestSetPageHWPoison() are under the same hugetlb_lock, and
__folio_set_hugetlb() moves under hugetlb_lock too, before HVO.  So A
either sees P as hugetlb, or the flag lands in P's OWN struct page
before HVO gets to it.

Also, with Muchun's vmemmap_remap_rwsem [1] HVO waits for
get_huge_page_for_hwpoison() anyway, so if his series goes in first the
hugetlb.c part can go.

[1] 
https://lore.kernel.org/all/20261111093747.50048-2-songmuchun@bytedance.com/

Mani