[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <00650ad9-367c-421c-9bfd-8701613ead85@oracle.com>
Date: Mon, 29 Sep 2025 13:52:26 -0700
From: jane.chu@...cle.com
To: Zi Yan <ziy@...dia.com>
Cc: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>,
David Hildenbrand <david@...hat.com>,
Luis Chamberlain <mcgrof@...nel.org>,
syzbot <syzbot+e6367ea2fdab6ed46056@...kaller.appspotmail.com>,
akpm@...ux-foundation.org, linmiaohe@...wei.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
nao.horiguchi@...il.com, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [mm?] WARNING in memory_failure
On 9/29/2025 1:15 PM, Zi Yan wrote:
> On 29 Sep 2025, at 14:23, jane.chu@...cle.com wrote:
>
>> On 9/29/2025 10:49 AM, jane.chu@...cle.com wrote:
>>>
>>> On 9/29/2025 10:29 AM, jane.chu@...cle.com wrote:
>>>>
>>>> On 9/29/2025 4:08 AM, Pankaj Raghav (Samsung) wrote:
>>>>>>
>>>>>> I want to change all the split functions in huge_mm.h and provide
>>>>>> mapping_min_folio_order() to try_folio_split() in truncate_inode_partial_folio().
>>>>>>
>>>>>> Something like below:
>>>>>>
>>>>>> 1. no split function will change the given order;
>>>>>> 2. __folio_split() will no longer give VM_WARN_ONCE when provided new_order
>>>>>> is smaller than mapping_min_folio_order().
>>>>>>
>>>>>> In this way, for an LBS folio that cannot be split to order 0, split
>>>>>> functions will return -EINVAL to tell caller that the folio cannot
>>>>>> be split. The caller is supposed to handle the split failure.
>>>>>
>>>>> IIUC, we will remove warn on once but just return -EINVAL in __folio_split()
>>>>> function if new_order < min_order like this:
>>>>> ...
>>>>> min_order = mapping_min_folio_order(folio->mapping);
>>>>> if (new_order < min_order) {
>>>>> - VM_WARN_ONCE(1, "Cannot split mapped folio below min- order: %u",
>>>>> - min_order);
>>>>> ret = -EINVAL;
>>>>> goto out;
>>>>> }
>>>>> ...
>>>>
>>>> Then the user process will get a SIGBUS indicting the entire huge page at higher order -
>>>> folio_set_has_hwpoisoned(folio);
>>>> if (try_to_split_thp_page(p, false) < 0) {
>>>> res = -EHWPOISON;
>>>> kill_procs_now(p, pfn, flags, folio);
>>>> put_page(p);
>>>> action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
>>>> goto unlock_mutex;
>>>> }
>>>> VM_BUG_ON_PAGE(!page_count(p), p);
>>>> folio = page_folio(p);
>>>>
>>>> the huge page is not usable any way, kind of similar to the hugetlb page situation: since the page cannot be splitted, the entire page is marked unusable.
>>>>
>>>> How about keep the current huge page split code as is, but change the M- F code to recognize that in a successful splitting case, the poisoned page might just be in a lower folio order, and thus, deliver the SIGBUS ?
>>>>
>>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>>> index a24806bb8e82..342c81edcdd9 100644
>>>> --- a/mm/memory-failure.c
>>>> +++ b/mm/memory-failure.c
>>>> @@ -2291,7 +2291,9 @@ int memory_failure(unsigned long pfn, int flags)
>>>> * page is a valid handlable page.
>>>> */
>>>> folio_set_has_hwpoisoned(folio);
>>>> - if (try_to_split_thp_page(p, false) < 0) {
>>>> + ret = try_to_split_thp_page(p, false);
>>>> + folio = page_folio(p);
>>>> + if (ret < 0 || folio_test_large(folio)) {
>>>> res = -EHWPOISON;
>>>> kill_procs_now(p, pfn, flags, folio);
>>>> put_page(p);
>>>> @@ -2299,7 +2301,6 @@ int memory_failure(unsigned long pfn, int flags)
>>>> goto unlock_mutex;
>>>> }
>>>> VM_BUG_ON_PAGE(!page_count(p), p);
>>>> - folio = page_folio(p);
>>>> }
>>>>
>>>> thanks,
>>>> -jane
>>>
>>> Maybe this is better, in case there are other reason for split_huge_page() to return -EINVAL.
>>>
>>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>>> index a24806bb8e82..2bfa05acae65 100644
>>> --- a/mm/memory-failure.c
>>> +++ b/mm/memory-failure.c
>>> @@ -1659,9 +1659,10 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>>> static int try_to_split_thp_page(struct page *page, bool release)
>>> {
>>> int ret;
>>> + int new_order = min_order_for_split(page_folio(page));
>>>
>>> lock_page(page);
>>> - ret = split_huge_page(page);
>>> + ret = split_huge_page_to_list_to_order(page, NULL, new_order);
>>> unlock_page(page);
>>>
>>> if (ret && release)
>>> @@ -2277,6 +2278,7 @@ int memory_failure(unsigned long pfn, int flags)
>>> folio_unlock(folio);
>>>
>>> if (folio_test_large(folio)) {
>>> + int ret;
>>> /*
>>> * The flag must be set after the refcount is bumped
>>> * otherwise it may race with THP split.
>>> @@ -2291,7 +2293,9 @@ int memory_failure(unsigned long pfn, int flags)
>>> * page is a valid handlable page.
>>> */
>>> folio_set_has_hwpoisoned(folio);
>>> - if (try_to_split_thp_page(p, false) < 0) {
>>> + ret = try_to_split_thp_page(p, false);
>>> + folio = page_folio(p);
>>> + if (ret < 0 || folio_test_large(folio)) {
>>> res = -EHWPOISON;
>>> kill_procs_now(p, pfn, flags, folio);
>>> put_page(p);
>>> @@ -2299,7 +2303,6 @@ int memory_failure(unsigned long pfn, int flags)
>>> goto unlock_mutex;
>>> }
>>> VM_BUG_ON_PAGE(!page_count(p), p);
>>> - folio = page_folio(p);
>>> }
>>>
>>> /*
>>> @@ -2618,7 +2621,8 @@ static int soft_offline_in_use_page(struct page *page)
>>> };
>>>
>>> if (!huge && folio_test_large(folio)) {
>>> - if (try_to_split_thp_page(page, true)) {
>>> + if ((try_to_split_thp_page(page, true)) ||
>>> + folio_test_large(page_folio(page))) {
>>> pr_info("%#lx: thp split failed\n", pfn);
>>> return -EBUSY;
>>> }
>>
>
> What you are proposing here is basically split_huge_page_to_min_order().
> I can add that as a second patch.
>
>> In soft offline, better to check if (min_order_for_split > 0), no need to split, just return for now ...
>
> OK. I can do that too.
>
> Thank you for the input.
>
That'll be great! Thank you!
-jane
>
> Best Regards,
> Yan, Zi
Powered by blists - more mailing lists