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

[David Hildenbrand (Arm)]

On 8/26/26 03:46, mawupeng wrote:
> 
> 
> 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. 

I am clearly in need of some vacation (next week!!! :) ).

> 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.

Nah, let's keep it like it is, thanks!

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David