Re: [syzbot] [mm?] WARNING in memory_failure

[jane . chu]

On 9/29/2025 10:29 AM, jane.chu@xxxxxxxxxx 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;
                }


thanks,
-jane