Re: [PATCH v4] mm/hwpoison: fix race between hugetlb free/demotion and memory_failure_hugetlb()

["HORIGUCHI NAOYA(堀口 直也)" <naoya.horiguchi@nec.com>]

On Thu, Mar 17, 2022 at 05:28:13PM +0800, Miaohe Lin wrote:
> On 2022/3/17 6:51, Mike Kravetz wrote:
> > On 3/16/22 05:07, Naoya Horiguchi wrote:
> >> From: Miaohe Lin <linmiaohe@huawei.com>
...
> > 
> > In my testing, there is a change in behavior that may not be intended.
> > 
> > My test strategy is:
> > - allocate two hugetlb pages
> > - create a mapping which reserves those two pages, but does not fault them in
> >   - as a result, the pages are on the free list but can not be freed
> > - inject error on a subpage of one of the huge pages
> >   - echo 0xYYY > /sys/kernel/debug/hwpoison/corrupt-pfn
> > - memory error code will call dissolve_free_huge_page
> >   - dissolve_free_huge_page returns -EBUSY because
> >     h->free_huge_pages - h->resv_huge_pages == 0
> > - We never end up setting Poison on the page with error or head page
> > - Huge page sitting on free list with error in subpage and not marked
> > - huge page with error could be given to an application or returned to buddy
> > 
> > Prior to this change, Poison would be set on the head page
> > 
> 
> Many thanks for pointing this out. IIUC, this change in behavior should be a bit
> unintended. We're trying to avoid setting PageHWPoison flag on the wrong page so
> we have to set the PageHWPoison flag after passing prechecks as commit log said.
> But there is room for improvement, e.g. when page changed to single page or another
> compound-size page after we grab the page refcnt, we could also set PageHWPoison
> before bailing out ? There might be something more we can do?

Yha, we could have more improvement around it.  I think that we can add
SetPageHWPoison near action_result(MF_MSG_DIFFERENT_PAGE_SIZE), but on which
page?  Maybe setting PageHWPoison on the raw page (not head page of new
compound page) is better because the new compound pages should not be hugetlb.
What about action_result(MF_MSG_UNKNOWN)?  Maybe we should do the same.
IOW, setting PageHWPoison on the head page is justified only when the
page is surely a hugepage.

Thanks,
Naoya Horiguchi