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
prev parent 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