[PATCH] sched/numa: Fix scan period for remote private faults

[PATCH] sched/numa: Fix scan period for remote private faults

[Hongling Zeng]

The early return condition in update_task_scan_period() incorrectly
slows down NUMA scanning for workloads with pure remote private memory
accesses.

Current condition:
    if (local + shared == 0 || p->numa_faults_locality[2])

For a workload accessing only private memory on remote nodes:
- shared = 0 (no shared accesses)
- local = 0 (all accesses are remote)
- Result: condition is TRUE, scan period doubles (slower)

This is wrong because for remote private memory, we should continue
to the ratio calculation which can speed up scanning to migrate the
memory to the local node.

The fix checks if there are actual faults (local + remote > 0) before
slowing down the scan rate. If there are faults, we should continue
to the ratio calculation logic to make an informed decision.

Also update the comments for ps_ratio and lr_ratio checks which appear
to be swapped - ps_ratio checks private/shared ratio not local accesses,
and lr_ratio checks local/remote ratio not shared accesses.

Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 kernel/sched/fair.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 37001c63452e..1fafaeb8d645 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3505,9 +3505,13 @@ static void update_task_scan_period(struct task_struct *p,
 	 * completely idle or all activity is in areas that are not of interest
 	 * to automatic numa balancing. Related to that, if there were failed
 	 * migration then it implies we are migrating too quickly or the local
-	 * node is overloaded. In either case, scan slower
+	 * node is overloaded. In either case, scan slower.
+	 *
+	 * Slow down if there are no actual memory faults (local + remote == 0),
+	 * or if previous migrations failed. Otherwise, use the locality ratios
+	 * to decide whether the scan rate should be adjusted.
 	 */
-	if (local + shared == 0 || p->numa_faults_locality[2]) {
+	if (local + remote == 0 || p->numa_faults_locality[2]) {
 		p->numa_scan_period = min(p->numa_scan_period_max,
 			p->numa_scan_period << 1);
 
@@ -3529,8 +3533,8 @@ static void update_task_scan_period(struct task_struct *p,
 
 	if (ps_ratio >= NUMA_PERIOD_THRESHOLD) {
 		/*
-		 * Most memory accesses are local. There is no need to
-		 * do fast NUMA scanning, since memory is already local.
+		 * Most memory accesses are private. Slow down NUMA scanning
+		 * since there is little shared memory to rebalance.
 		 */
 		int slot = ps_ratio - NUMA_PERIOD_THRESHOLD;
 		if (!slot)
@@ -3538,8 +3542,9 @@ static void update_task_scan_period(struct task_struct *p,
 		diff = slot * period_slot;
 	} else if (lr_ratio >= NUMA_PERIOD_THRESHOLD) {
 		/*
-		 * Most memory accesses are shared with other tasks.
-		 * There is no point in continuing fast NUMA scanning,
+		 * Most memory accesses are local. There is no need to
+		 * do fast NUMA scanning, since memory is already local.
+		 * Also, shared memory may be moved by other tasks anyway,
 		 * since other tasks may just move the memory elsewhere.
 		 */
 		int slot = lr_ratio - NUMA_PERIOD_THRESHOLD;
-- 
2.25.1

Re: [PATCH] sched/numa: Fix scan period for remote private faults

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

On Tue, Aug 04, 2026 at 11:07:31AM +0800, Hongling Zeng wrote:
> This is wrong because for remote private memory, we should continue
> to the ratio calculation which can speed up scanning to migrate the
> memory to the local node.

I don't think the ratio calculation actually speeds scanning up in that
case, though. For the pure remote-private accesses you describe
(shared == 0, private > 0):

	ps_ratio = private * NUMA_PERIOD_SLOTS / (private + shared)
	         = private * 10 / (private + 0)
	         = 10

which is >= NUMA_PERIOD_THRESHOLD (7), so it takes the first branch:

	int slot = ps_ratio - NUMA_PERIOD_THRESHOLD;   /* 3 */
	diff = slot * period_slot;                     /* > 0 */

and numa_scan_period is *increased* (scan slower), not decreased. The
speed-up (else) branch is only reached when both ps_ratio < 7 and
lr_ratio < 7, which pure-private accesses (ps_ratio == 10) never satisfy.

So dropping the early return here doesn't speed scanning up; it just
grows the period by ~3 slots instead of doubling it. That might still be
a reasonable change, but the justification as written describes an effect
that doesn't seem to happen. Could you double-check, and share some
before/after numbers on a remote-private workload? A scan-rate change
like this really wants data behind it.

Two smaller things:

  - The comment rewrites (ps_ratio -> "private", lr_ratio -> "local") look
    like a reasonable cleanup on their own, but folding them into a
    behavioural change makes the patch harder to review -- perhaps split
    them out. (The lr_ratio branch also keeps the "shared ... moved by
    other tasks" sentence, which no longer fits a local-dominant branch.)

  - This is the same early return that other in-flight patches touch (the
    numa_faults_locality reset thread, where Peter suggested sharing the
    tail with the normal path). It may be worth coordinating so the
    changes don't collide.

Thanks,
Zhan Xusheng

Re: [PATCH] sched/numa: Fix scan period for remote private faults

[Hongling Zeng]

在 2026年08月04日 11:44, Zhan Xusheng 写道:
> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
>
> On Tue, Aug 04, 2026 at 11:07:31AM +0800, Hongling Zeng wrote:
>> This is wrong because for remote private memory, we should continue
>> to the ratio calculation which can speed up scanning to migrate the
>> memory to the local node.
> I don't think the ratio calculation actually speeds scanning up in that
> case, though. For the pure remote-private accesses you describe
> (shared == 0, private > 0):
>
> 	ps_ratio = private * NUMA_PERIOD_SLOTS / (private + shared)
> 	         = private * 10 / (private + 0)
> 	         = 10
>
> which is >= NUMA_PERIOD_THRESHOLD (7), so it takes the first branch:
>
> 	int slot = ps_ratio - NUMA_PERIOD_THRESHOLD;   /* 3 */
> 	diff = slot * period_slot;                     /* > 0 */
>
> and numa_scan_period is *increased* (scan slower), not decreased. The
> speed-up (else) branch is only reached when both ps_ratio < 7 and
> lr_ratio < 7, which pure-private accesses (ps_ratio == 10) never satisfy.
>
> So dropping the early return here doesn't speed scanning up; it just
> grows the period by ~3 slots instead of doubling it. That might still be
> a reasonable change, but the justification as written describes an effect
> that doesn't seem to happen. Could you double-check, and share some
> before/after numbers on a remote-private workload? A scan-rate change
> like this really wants data behind it.
Thank you for the detailed review. Your analysis is completely correct.
   You're right. The original commit message incorrectly claimed this would
   "speed up" scanning. The actual effect is changing from unconditional
   doubling to a ratio-based adjustment (which still slows scanning, but 
less
   aggressively).

   I've updated the patch based on your feedback:
[PATCH v2] sched/numa: avoid doubling scan period for remote private faults

>
> Two smaller things:
>
>    - The comment rewrites (ps_ratio -> "private", lr_ratio -> "local") look
>      like a reasonable cleanup on their own, but folding them into a
>      behavioural change makes the patch harder to review -- perhaps split
>      them out. (The lr_ratio branch also keeps the "shared ... moved by
>      other tasks" sentence, which no longer fits a local-dominant branch.)
  This is a good point. For now I've kept them together since both changes
   address the same underlying issue (misleading comments about what the 
ratios
   represent). If this version is acceptable, I can submit a separate 
cleanup
   patch in the future if needed.

>
>    - This is the same early return that other in-flight patches touch (the
>      numa_faults_locality reset thread, where Peter suggested sharing the
>      tail with the normal path). It may be worth coordinating so the
>      changes don't collide
>
> Thanks,
> Zhan Xusheng
   I'll check for coordination with other in-flight patches before the next
   submission.

   Thank you again for catching the fundamental issue with the original
   justification and for the detailed suggestions on how to improve it.

   Best regards,
   Hongling