Re: [PATCH v2 01/20] hugetlb: Don't restore vmemmap of non-HVOed folios on bulk restore error

[Muchun Song]

> On Oct 3, 2026, at 08:21, James Houghton <jthoughton@xxxxxxxxxx> wrote:
> 
> When hugetlb_vmemmap_restore_folios() fails, it stops processing the
> list, so folio_list may still contain folios that were never vmemmap
> optimized, e.g. because HVO failed when they were allocated. Their
> hugetlb flag has already been cleared by remove_hugetlb_folio().
> 
> If no folios could be restored, bulk_vmemmap_restore_error() then calls
> hugetlb_vmemmap_restore_folio() on every folio left on folio_list,
> including those, which trips the !folio_test_hugetlb() check in
> __hugetlb_vmemmap_restore_folio():
> 
>  page dumped because: VM_WARN_ON_ONCE_FOLIO(!folio_test_hugetlb(folio))
>  WARNING: mm/hugetlb_vmemmap.c:503 at __hugetlb_vmemmap_restore_folio+0x270/0x300
>  ...
>   __hugetlb_vmemmap_restore_folio+0x270/0x300 (P)
>   hugetlb_vmemmap_restore_folio+0x1c/0x30
>   update_and_free_pages_bulk+0x2c0/0x368
>   __nr_hugepages_store_common+0x2e8/0x3f0
> 
> Otherwise this is benign: the restore is a no-op returning success for
> non-optimized folios, which are then freed as intended.

Good catch.

> 
> Skip the restore for folios that are not vmemmap optimized; they can be
> freed right away.
> 
> This is very unlikely to happen without fault injection, as it requires
> both HVO and a subsequent vmemmap restore to fail.
> 
> Fixes: cfb8c75099db ("hugetlb: perform vmemmap restoration on a list of pages")
> Assisted-by: LLM
> Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>
> ---
> mm/hugetlb.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index a1b51251103b..4dac7ed1df57 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1613,9 +1613,14 @@ static void bulk_vmemmap_restore_error(struct hstate *h,
> 		* page is made a surplus page and removed from the list.
> 		* If are able to restore vmemmap and free one hugetlb page, we
> 		* quit processing the list to retry the bulk operation.
> + 		*
> + 		* Folios whose vmemmap was never optimized (e.g. because HVO
> + 		* failed when they were allocated) can be freed right away;
> + 		* remove_hugetlb_folio() has already cleared their hugetlb flag.

Since you mentioned the hugetlb flag is cleared. So it's unnecessary to clear it again.

> 		*/
> 		list_for_each_entry_safe(folio, t_folio, folio_list, lru)
> - 			if (hugetlb_vmemmap_restore_folio(h, folio)) {
> + 			if (folio_test_hugetlb_vmemmap_optimized(folio) &&
> + 			    hugetlb_vmemmap_restore_folio(h, folio)) {
> 				list_del(&folio->lru);
> 				spin_lock_irq(&hugetlb_lock);
> 				add_hugetlb_folio(h, folio, true);

Something like following:

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 6a011b627986..1ab3e61f4704 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1616,21 +1616,31 @@ static void bulk_vmemmap_restore_error(struct hstate *h,
                 * If are able to restore vmemmap and free one hugetlb page, we
                 * quit processing the list to retry the bulk operation.
                 */
-               list_for_each_entry_safe(folio, t_folio, folio_list, lru)
-                       if (hugetlb_vmemmap_restore_folio(h, folio)) {
-                               list_del(&folio->lru);
+               list_for_each_entry_safe(folio, t_folio, folio_list, lru) {
+                       list_del(&folio->lru);
+
+                       /*
+                        * remove_hugetlb_folio() already cleared the hugetlb flag
+                        * for folios whose vmemmap was not optimized, so they can
+                        * be freed without restoring vmemmap.
+                        */
+                       if (folio_test_hugetlb(folio) &&
+                           hugetlb_vmemmap_restore_folio(h, folio)) {
                                spin_lock_irq(&hugetlb_lock);
                                add_hugetlb_folio(h, folio, true);
                                spin_unlock_irq(&hugetlb_lock);
-                       } else {
-                               list_del(&folio->lru);
+                               continue;
+                       }
+
+                       if (folio_test_hugetlb(folio)) {
                                spin_lock_irq(&hugetlb_lock);
                                __folio_clear_hugetlb(folio);
                                spin_unlock_irq(&hugetlb_lock);
-                               update_and_free_hugetlb_folio(h, folio, false);
-                               cond_resched();
-                               break;
                        }
+                       update_and_free_hugetlb_folio(h, folio, false);
+                       cond_resched();
+                       break;
+               }
        }
 }

Thanks,
Muchun

> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
>