Re: [PATCH] mm/hugetlb: fix missing migratable flag on same-node hugetlb migration

[mawupeng]

On 周二 2026-8-25 18:27, David Hildenbrand (Arm) wrote:
> On 7/7/26 13:02, Wupeng Ma wrote:
>> Commit ba23f58de896 ("mm/migrate: don't call
>> folio_putback_active_hugetlb() on dst hugetlb folio") moved setting of
>> the migratable flag and active-list placement from
>> folio_putback_active_hugetlb(dst) into move_hugetlb_state(), so that
>> the freshly allocated destination folio is handled where allocation is
>> known to have succeeded.
>>
>> Unfortunately, the new code was appended after the existing
>> temporary-folio block in move_hugetlb_state(), which contains an early
>> return added earlier by commit 5af1ab1d24e08 ("mm/hugetlb: optimize
>> the surplus state transfer code in move_hugetlb_state()"):
>>
>>   if (folio_test_hugetlb_temporary(new_folio)) {
>>       ...
>>       if (new_nid == old_nid)
>>           return;                       <-- skips the new code
>>       ...
>>   }
>>
>>   /* added by ba23f58 */
>>   folio_set_hugetlb_migratable(new_folio);
>>   list_move_tail(&new_folio->lru, ...&h->hugepage_activelist);
>>
>> When the destination folio is temporary (i.e. the hugetlb pool was
>> exhausted and the migration callback fell back to
>> alloc_migrate_hugetlb_folio()) and the migration does not cross a
>> node -- the common case, and always true on a single-NUMA system --
>> move_hugetlb_state() returns before setting the migratable flag or
>> adding the new folio to the active list. The destination folio is
>> then installed in the page table but cannot be isolated afterwards,
>> since folio_isolate_hugetlb() rejects folios without the migratable
>> flag; a subsequent soft-offline, hard-offline or memory-hotplug
>> offline of that folio fails with -EBUSY.
>>
>> This was reproduced on a single-NUMA arm64 VM: a second
>> MADV_SOFT_OFFLINE on an already-migrated hugetlb page returned EBUSY
>> and logged "hugepage isolation failed".
>>
>> Keep the surplus adjustment, which is the only part that depends on
>> the node crossing, guarded by `if (new_nid != old_nid)', while making
>> the migratable flag and active-list placement unconditional. This
>> preserves the cleanup intent of ba23f58 and closes the early-return
>> hole.
>>
>> Fixes: ba23f58de896 ("mm/migrate: don't call folio_putback_active_hugetlb() on dst hugetlb folio")
> 
> Agreed, let's CC stable.

Thanks.

> 
>> Signed-off-by: Wupeng Ma <mawupeng1@xxxxxxxxxx>
>> ---
>>  mm/hugetlb.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 571212b80835..cafadfdb63c0 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -7211,14 +7211,14 @@ void move_hugetlb_state(struct folio *old_folio, struct folio *new_folio, int re
>>  		 * There is no need to transfer the per-node surplus state
>>  		 * when we do not cross the node.
>>  		 */
>> -		if (new_nid == old_nid)
>> -			return;
>> -		spin_lock_irq(&hugetlb_lock);
>> -		if (h->surplus_huge_pages_node[old_nid]) {
>> -			h->surplus_huge_pages_node[old_nid]--;
>> -			h->surplus_huge_pages_node[new_nid]++;
>> +		if (new_nid != old_nid) {
>> +			spin_lock_irq(&hugetlb_lock);
>> +			if (h->surplus_huge_pages_node[old_nid]) {
>> +				h->surplus_huge_pages_node[old_nid]--;
>> +				h->surplus_huge_pages_node[new_nid]++;
>> +			}
>> +			spin_unlock_irq(&hugetlb_lock);
>>  		}
> 
> The return was really rather hidden, thanks!
> 
> Can't we instead just turn the "return;" into a "continue;" ?

There’s no loop in move_hugetlb_state(), so continue won’t work there. But I understand your intention. 

To be honest, I’m not fully satisfied with the nested if either, but I haven’t found a cleaner way.
goto, or extracting the surplus move into a helper, feels like over-engineering.

>