Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "David Hildenbrand (Arm)" <david@kernel.org>
To: mawupeng <mawupeng1@huawei.com>,
	muchun.song@linux.dev, osalvador@suse.de,
	akpm@linux-foundation.org, baolin.wang@linux.alibaba.com
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/hugetlb: fix missing migratable flag on same-node hugetlb migration
Date: Wed, 26 Aug 2026 09:23:37 +0200	[thread overview]
Message-ID: <24f8108d-e7e7-47d6-8994-f50625daa4f0@kernel.org> (raw)
In-Reply-To: <ce6be959-58df-4138-bb35-01a1785a91d3@huawei.com>

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@huawei.com>
>>> ---
>>>  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@kernel.org>

-- 
Cheers,

David


      reply	other threads:[~2026-08-26  7:23 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07 11:02 [PATCH] mm/hugetlb: fix missing migratable flag on same-node hugetlb migration Wupeng Ma
2026-07-07 20:21 ` Andrew Morton
2026-07-08  1:14   ` mawupeng
2026-08-25 10:27 ` David Hildenbrand (Arm)
2026-08-26  1:46   ` mawupeng
2026-08-26  7:23     ` David Hildenbrand (Arm) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=24f8108d-e7e7-47d6-8994-f50625daa4f0@kernel.org \
    --to=david@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mawupeng1@huawei.com \
    --cc=muchun.song@linux.dev \
    --cc=osalvador@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox