Re: [PATCH] mm/damon: prevent migration fallback to non-target nodes

[SJ Park <sj@kernel.org>]

+ Honggyu for origin intent of not adding __GFP_THISNODE

On Wed, 8 Jul 2026 10:55:05 -0700 Jiahui Zhang <jiahuitry@outlook.com> wrote:

> Hi SJ,
> 
> Thank you for the review.
> 
> On Wed, Jul 08, 2026 at 07:11:33AM -0700, SJ Park wrote:
> > So this patch is introducing a small behavioral change.  May I ask
> > why you think the new behavior is better?  Have you find some 
> > negative user impacts from the current behavior?
> 
> Consider a two-node tiered system, where node 0 is the fast tier and 
> node 1 is a CPU-less slow tier, such as CXL memory.  Suppose node 0 
> is nearly full and the user wants to promote hot regions (which reside
> on node 1) to node 0 with a command like:
> 
>   sudo damo start --ops vaddr --target_pid ${workload_pid} \
>       --damos_action migrate_hot 0 \
>       --damos_access_rate 70% max
> 
> Without the __GFP_THISNODE flag, when the memory allocator finds that
> node 0 is nearly full, it can fall back to node 1 without waking up 
> kswapd.

Have you considered or tested running demotion-purpose DAMOS scheme together?

> Then the pages allocated for migrate_pages() are still on node 1, 
> and the regions that are expected to be promoted to node 0 are only moved
>  to different physical pages on node 1.
> 
> As a result, the NUMA node placement does not change, but the following
> command still observes successful migrations:
> 
>   sudo bpftrace -e '
>   tracepoint:migrate:mm_migrate_pages /args->reason == 9/
>   {
>       @succeeded_pages = sum(args->succeeded);
>   }
>   interval:s:1
>   {
>       time("%H:%M:%S ");
>       print(@succeeded_pages);
>       clear(@succeeded_pages);
>   }'
> 
> I've also checked other page migration paths, such as alloc_demote_folio(),
>  alloc_misplaced_dst_folio() and do_move_pages_to_node(), all of which set
> the __GFP_THISNODE flag.
> 
> I am not sure whether the original design intended to provide only
> best-effort semantics,

To my understanding, it was initially trying to reflect what
demote_folio_list() does.  Because it doesn't have ___GFP_THISNODE, this
function also ended up not having it without a very serious reason.

My mental model of DAMOS_MIGRATE_{HOT,COLD} nowadays is like move_pages().
And I find do_move_pages_to_node(), which is used by move_pages() is also using
the flag.

So I think adding the flag is ok.  Cc-ed Honggyu for a case he has different
opinions, though.

> but I found this behavior very confusing in my
> experiment.

Thank you for sharing this pain point.

I agree this can be very confusing for inveestigations.  Not just confusing but
makes investigations of this case nearly impossible, iiuc.  DAMOS provides its
own stats for succeeded and failed operations, but that will again not be
helpful since migrate_pages would returned success in this case.

So, yes, let's add the flag unless someone else has a different opinions.

> 
> > We as the kernel community disallow [1] anonymous patch submissions.  Is zjh
> > your preferred identity?  Also, it is not a strict rule, but we as the mm
> > community prefer using a formal name if that's ok.
> 
> Sorry I did not notice this.  If you think the GFP flag should be added, I will
> send a v2 of the patch and correct the Signed-off-by issue.

Yes, please give others time to comment on (say, one day?) and proceed to v2 if
nobody disagrees to.

On the version, could you please add the clarification of the pain points you
encountered, and how this change made it disappeared?

Also, let's change the subject prefix from mm/damon: to mm/damon/ops-common:


Thanks,
SJ

[...]