Re: [PATCH] mm/vmscan: fix hwpoisoned large folio handling in shrink_folio_list

[Jinjiang Tu <tujinjiang@huawei.com>]

在 2025/6/11 16:35, David Hildenbrand 写道:
> On 11.06.25 10:29, Jinjiang Tu wrote:
>>
>> 在 2025/6/11 15:59, David Hildenbrand 写道:
>>> On 11.06.25 09:46, Jinjiang Tu wrote:
>>>> In shrink_folio_list(), the hwpoisoned folio may be large folio, which
>>>> can't be handled by unmap_poisoned_folio().
>>>>
>>>> Since UCE is rare in real world, and race with reclaimation is more
>>>> rare,
>>>> just skipping the hwpoisoned large folio is enough. memory_failure()
>>>> will
>>>> handle it if the UCE is triggered again.
>>>>
>>>> Fixes: 1b0449544c64 ("mm/vmscan: don't try to reclaim hwpoison folio")
>>>
>>> Please also add
>>>
>>> Closes:
>>>
>>> with a link to the report
>> Thanks, I will add it.
>>>
>>>> Reported-by: syzbot+3b220254df55d8ca8a61@syzkaller.appspotmail.com
>>>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>>>> ---
>>>>    mm/vmscan.c | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>> /home/tujinjiang/hulk-repo/hulk/mm/mempolicy.c
>>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>>> index b6f4db6c240f..3a4e8d7419ae 100644
>>>> --- a/mm/vmscan.c
>>>> +++ b/mm/vmscan.c
>>>> @@ -1131,6 +1131,14 @@ static unsigned int shrink_folio_list(struct
>>>> list_head *folio_list,
>>>>                goto keep;
>>>>              if (folio_contain_hwpoisoned_page(folio)) {
>>>> +            /*
>>>> +             * unmap_poisoned_folio() can't handle large
>>>> +             * folio, just skip it. memory_failure() will
>>>> +             * handle it if the UCE is triggered again.
>>>> +             */
>>>> +            if (folio_test_large(folio))
>>>> +                goto keep_locked;
>>>> +
>>>>                unmap_poisoned_folio(folio, folio_pfn(folio), false);
>>>>                folio_unlock(folio);
>>>>                folio_put(folio);
>>>
>>> Why not handle that in unmap_poisoned_folio() to make that limitation
>>> clear and avoid?
>> I tried to put the check in unmap_poisoned_folio(), but it still exists
>> other issues.
>
>
>
>> The calltrace in v6.6 kernel:
>>
>> Unable to handle kernel paging request at virtual address 
>> fbd5200000000024
>> KASAN: maybe wild-memory-access in range
>> [0xdead000000000120-0xdead000000000127]
>> pc : __list_add_valid_or_report+0x50/0x158 lib/list_debug.c:32
>> lr : __list_add_valid include/linux/list.h:88 [inline]
>> lr : __list_add include/linux/list.h:150 [inline]
>> lr : list_add_tail include/linux/list.h:183 [inline]
>> lr : lru_add_page_tail.constprop.0+0x4ac/0x640 mm/huge_memory.c:3187
>> Call trace:
>>    __list_add_valid_or_report+0x50/0x158 lib/list_debug.c:32
>>    __list_add_valid include/linux/list.h:88 [inline]
>>    __list_add include/linux/list.h:150 [inline]
>>    list_add_tail include/linux/list.h:183 [inline]
>>    lru_add_page_tail.constprop.0+0x4ac/0x640 mm/huge_memory.c:3187
>>    __split_huge_page_tail.isra.0+0x344/0x508 mm/huge_memory.c:3286
>>    __split_huge_page+0x244/0x1270 mm/huge_memory.c:3317
>>    split_huge_page_to_list_to_order+0x1038/0x1620 mm/huge_memory.c:3625
>>    split_folio_to_list_to_order include/linux/huge_mm.h:638 [inline]
>>    split_folio_to_order include/linux/huge_mm.h:643 [inline]
>>    deferred_split_scan+0x5f8/0xb70 mm/huge_memory.c:3778
>>    do_shrink_slab+0x2a0/0x828 mm/vmscan.c:927
>>    shrink_slab_memcg+0x2c0/0x558 mm/vmscan.c:996
>>    shrink_slab+0x228/0x250 mm/vmscan.c:1075
>>    shrink_node_memcgs+0x34c/0x6a0 mm/vmscan.c:6630
>>    shrink_node+0x21c/0x1378 mm/vmscan.c:6664
>>    shrink_zones.constprop.0+0x24c/0xab0 mm/vmscan.c:6906
>>    do_try_to_free_pages+0x150/0x880 mm/vmscan.c:6968
>>
>>
>> The folio is deleted from lru and the folio->lru can't be accessed. If
>> the folio is splitted later,
>> lru_add_split_folio() assumes the folio is on lru.
>
> Not sure if something like the following would be appropriate:
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index b91a33fb6c694..fdd58c8ba5254 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1566,6 +1566,9 @@ int unmap_poisoned_folio(struct folio *folio, 
> unsigned long pfn, bool must_kill)
>         enum ttu_flags ttu = TTU_IGNORE_MLOCK | TTU_SYNC | TTU_HWPOISON;
>         struct address_space *mapping;
>
> +       if (folio_test_large && !folio_test_hugetlb(folio))
> +               return -EBUSY;
> +
>         if (folio_test_swapcache(folio)) {
>                 pr_err("%#lx: keeping poisoned page in swap cache\n", 
> pfn);
>                 ttu &= ~TTU_HWPOISON;
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f8dfd2864bbf4..6a3426bc9e9d7 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1138,7 +1138,8 @@ static unsigned int shrink_folio_list(struct 
> list_head *folio_list,
>                         goto keep;
>
>                 if (folio_contain_hwpoisoned_page(folio)) {
> -                       unmap_poisoned_folio(folio, folio_pfn(folio), 
> false);
> +                       if (unmap_poisoned_folio(folio, 
> folio_pfn(folio), false)){
> +                               list_add(&folio->lru, &ret_folios);
>                         folio_unlock(folio);
>                         folio_put(folio);
>                         continue;

The expected behaviour is keeping the folio on lru if 
unmap_poisoned_folio fails?

If so, we should:

+                       if (unmap_poisoned_folio(folio, 
folio_pfn(folio), false)){
+                               goto keep_locked;

otherwise, folio_put() is called twice to put ref grabbed from isolation.