[v1] memory tiering: Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled

[PATCH] memory tiering: Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled

Posted by Donet Tom 2 weeks ago

In the current implementation, if NUMA_BALANCING_MEMORY_TIERING is
disabled and the pages are on the lower tier, the pages may still be
promoted.

This happens because task_numa_work() updates the last_cpupid field to
record the last access time only when NUMA_BALANCING_MEMORY_TIERING is
enabled and the folio is on the lower tier. If
NUMA_BALANCING_MEMORY_TIERING is disabled, the last_cpupid field
retains a valid last CPU id.

In should_numa_migrate_memory(), the decision checks whether
NUMA_BALANCING_MEMORY_TIERING is disabled, the folio is on the lower
tier, and last_cpupid is invalid. However, since last_cpupid remains
valid when NUMA_BALANCING_MEMORY_TIERING is disabled, the condition
evaluates to false and migration is allowed.

This patch prevents promotion when NUMA_BALANCING_MEMORY_TIERING is
disabled and the folio is on the lower tier.

Also, when NUMA_BALANCING_MEMORY_TIERING is enabled, last_cpupid is always
invalid. Therefore, the !cpupid_valid(last_cpupid) check in
task_numa_fault() is redundant. Removed the unnecessary check and simplify
the condition.

Behavior before this change:
============================
  - If NUMA_BALANCING_NORMAL is enabled, migration occurs between
    nodes within the same memory tier, and promotion from lower
    tier to higher tier may also happen.

  - If NUMA_BALANCING_MEMORY_TIERING is enabled, promotion from
    lower tier to higher tier nodes is allowed.

Behavior after this change:
===========================
  - If NUMA_BALANCING_NORMAL is enabled, migration will occur only
    between nodes within the same memory tier.

  - If NUMA_BALANCING_MEMORY_TIERING is enabled, promotion from lower
    tier to higher tier nodes will be allowed.

  - If both NUMA_BALANCING_MEMORY_TIERING and NUMA_BALANCING_NORMAL are
    enabled, both migration (same tier) and promotion (cross tier) are
    allowed.

Fixes: 33024536bafd ("memory tiering: hot page selection with hint page fault latency")
Signed-off-by: Donet Tom <donettom@linux.ibm.com>
---
 kernel/sched/fair.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bf948db905ed..39e860fce85a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1990,6 +1990,13 @@ bool should_numa_migrate_memory(struct task_struct *p, struct folio *folio,
 	 */
 	if (!node_state(dst_nid, N_MEMORY))
 		return false;
+	/*
+	 * Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled
+	 * and the pages are on the lower tier.
+	 */
+	if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&
+	    !node_is_toptier(src_nid))
+		return false;
 
 	/*
 	 * The pages in slow memory node should be migrated according
@@ -2024,10 +2031,6 @@ bool should_numa_migrate_memory(struct task_struct *p, struct folio *folio,
 	this_cpupid = cpu_pid_to_cpupid(dst_cpu, current->pid);
 	last_cpupid = folio_xchg_last_cpupid(folio, this_cpupid);
 
-	if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&
-	    !node_is_toptier(src_nid) && !cpupid_valid(last_cpupid))
-		return false;
-
 	/*
 	 * Allow first faults or private faults to migrate immediately early in
 	 * the lifetime of a task. The magic number 4 is based on waiting for
@@ -3242,8 +3245,7 @@ void task_numa_fault(int last_cpupid, int mem_node, int pages, int flags)
 	 * node for memory tiering mode.
 	 */
 	if (!node_is_toptier(mem_node) &&
-	    (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING ||
-	     !cpupid_valid(last_cpupid)))
+	    (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING))
 		return;
 
 	/* Allocate buffer to track faults on a per-node basis */
-- 
2.52.0

Re: [PATCH] memory tiering: Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled

Posted by Andrew Morton 2 weeks ago

On Fri, 20 Mar 2026 14:52:51 +0530 Donet Tom <donettom@linux.ibm.com> wrote:

> In the current implementation, if NUMA_BALANCING_MEMORY_TIERING is
> disabled and the pages are on the lower tier, the pages may still be
> promoted.
> 
> This happens because task_numa_work() updates the last_cpupid field to
> record the last access time only when NUMA_BALANCING_MEMORY_TIERING is
> enabled and the folio is on the lower tier. If
> NUMA_BALANCING_MEMORY_TIERING is disabled, the last_cpupid field
> retains a valid last CPU id.
> 
> In should_numa_migrate_memory(), the decision checks whether
> NUMA_BALANCING_MEMORY_TIERING is disabled, the folio is on the lower
> tier, and last_cpupid is invalid. However, since last_cpupid remains
> valid when NUMA_BALANCING_MEMORY_TIERING is disabled, the condition
> evaluates to false and migration is allowed.
> 
> This patch prevents promotion when NUMA_BALANCING_MEMORY_TIERING is
> disabled and the folio is on the lower tier.

Thanks.  The AI reviewbot asked some questions:
	https://sashiko.dev/#/patchset/20260320092251.1290207-1-donettom@linux.ibm.com

Re: [PATCH] memory tiering: Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled

Posted by Donet Tom 1 week, 6 days ago

On 3/20/26 9:50 PM, Andrew Morton wrote:
> On Fri, 20 Mar 2026 14:52:51 +0530 Donet Tom <donettom@linux.ibm.com> wrote:
>
>> In the current implementation, if NUMA_BALANCING_MEMORY_TIERING is
>> disabled and the pages are on the lower tier, the pages may still be
>> promoted.
>>
>> This happens because task_numa_work() updates the last_cpupid field to
>> record the last access time only when NUMA_BALANCING_MEMORY_TIERING is
>> enabled and the folio is on the lower tier. If
>> NUMA_BALANCING_MEMORY_TIERING is disabled, the last_cpupid field
>> retains a valid last CPU id.
>>
>> In should_numa_migrate_memory(), the decision checks whether
>> NUMA_BALANCING_MEMORY_TIERING is disabled, the folio is on the lower
>> tier, and last_cpupid is invalid. However, since last_cpupid remains
>> valid when NUMA_BALANCING_MEMORY_TIERING is disabled, the condition
>> evaluates to false and migration is allowed.
>>
>> This patch prevents promotion when NUMA_BALANCING_MEMORY_TIERING is
>> disabled and the folio is on the lower tier.
> Thanks.  The AI reviewbot asked some questions:
> 	https://sashiko.dev/#/patchset/20260320092251.1290207-1-donettom@linux.ibm.com
>
AI review questions

> commit b0847daa3e0064d33d2232cba1d7607919cae7db
> Author: Donet Tom <donettom@linux.ibm.com>
> memory tiering: Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled
> In the current implementation, if NUMA_BALANCING_MEMORY_TIERING is
> disabled and the pages are on the lower tier, the pages may still be
> promoted.
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -2060,6 +2060,13 @@ bool should_numa_migrate_memory(struct task_struct *p, struct folio *folio,
> >  	 */
> >  	if (!node_state(dst_nid, N_MEMORY))
> >  		return false;
> > +	/*
> > +	 * Do not allow promotion if NUMA_BALANCING_MEMORY_TIERING is disabled
> > +	 * and the pages are on the lower tier.
> > +	 */
> > +	if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&
> > +	    !node_is_toptier(src_nid))
> > +		return false;
> Does this block all migrations originating from a lower-tier node when tiering
>
> is disabled, including valid same-tier migration between two 
> lower-tier nodes?
>
This blocks migration (promotion) from a lower tier to a higher tier 
when tiering is disabled. However, migration within the same tier will 
still be performed if NUMA_BALANCING_NORMAL is enabled.

> Conversely, since this only checks !node_is_toptier(src_nid), does it still
> allow cross-tier migration from top-tier to lower-tier nodes when tiering is
> disabled?

Migration (demotion) from a higher tier to a lower tier is not handled 
by NUMA balancing, so it will work irrespective of this setting.

> Also, because this early return is placed before folio_xchg_last_cpupid() is
> called, does it prevent lower-tier folios from overwriting their stale
> _last_cpupid timestamp with a valid CPU ID? This could permanently freeze NUMA
> access tracking for these folios and inflate private fault statistics.


No, the timestamp is only stored when NUMA_BALANCING_MEMORY_TIERING is 
enabled.


> [ ... ]
> > @@ -2094,10 +2101,6 @@ bool should_numa_migrate_memory(struct task_struct *p, struct folio *folio,
> >  	this_cpupid = cpu_pid_to_cpupid(dst_cpu, current->pid);
> >  	last_cpupid = folio_xchg_last_cpupid(folio, this_cpupid);
> >  
> > -	if (!(sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&
> > -	    !node_is_toptier(src_nid) && !cpupid_valid(last_cpupid))
> > -		return false;
> > -
> This isn't a bug, but since the only usages of cpupid_valid() were removed
> here and in task_numa_fault(), should the definition of cpupid_valid() and its
> accompanying comments also be removed to avoid leaving dead code behind?


Yes, We can remove that. I will send a V2 with that change.


> [ ... ]
> > @@ -3312,8 +3315,7 @@ void task_numa_fault(int last_cpupid, int mem_node, int pages, int flags)
> >  	 * node for memory tiering mode.
> >  	 */
> >  	if (!node_is_toptier(mem_node) &&
> > -	    (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING ||
> > -	     !cpupid_valid(last_cpupid)))
> > +	    (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING))
> >  		return;
> If memory tiering is disabled at runtime, existing folios on slow nodes may
> retain an access time stored in _last_cpupid instead of a valid CPU ID.
> By removing the !cpupid_valid(last_cpupid) check, could this timestamp be
> treated as a valid CPU ID and passed into task_numa_group()?
> Inside task_numa_group(), cpupid_to_cpu(cpupid) could then extract an
> out-of-bounds CPU index, which might lead to an out-of-bounds read when
> accessing cpu_rq(cpu)->curr and cause a kernel panic.

Yes. This is also a valid point. Let me fix this and send a v2.