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

[Muchun Song]

> On Oct 22, 2026, at 05:27, James Houghton <jthoughton@xxxxxxxxxx> wrote:
> 
> On Wed, Oct 21, 2026 at 2:33 AM Muchun Song <muchun.song@xxxxxxxxx> wrote:
>>> On Oct 3, 2026, at 08:21, James Houghton <jthoughton@xxxxxxxxxx> 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:

More clear, LGTM.

Thanks,
Muchun

> 
> 	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_ire(&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.