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

[James Houghton <jthoughton@google.com>]

On Wed, Oct 21, 2026 at 2:33 AM Muchun Song <muchun.song@linux.dev> wrote:
> > On Oct 3, 2026, at 08:21, James Houghton <jthoughton@google.com> wrote:
> > 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. :) I think it's slightly easier to read without the second
folio_test_hugetlb(), like this:

	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)) {
			if (hugetlb_vmemmap_restore_folio(h, folio)) {
				spin_lock_irq(&hugetlb_lock);
				add_hugetlb_folio(h, folio, true);
				spin_unlock_irq(&hugetlb_lock);
				continue;
			}
			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;
	}

Applied this in the v3.