[PATCH v2] sched/cache: Remove the old cache group footprint on exec

[PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Jemmy Wong]

Exec replaces the task's cache group before resetting its NUMA fault
statistics, but only the exit path subtracts the task's contribution
from the old group's footprint.

Although de_thread() removes other members of the executing task's
thread group, tasks sharing the mm through CLONE_VM without CLONE_THREAD
(e.g., vfork()) can retain the old mm and cache group. The executing
task's contribution then remains without further updates or decay,
potentially suppressing cache-aware aggregation through the LLC
capacity check.

A vfork child rarely accumulates NUMA faults before exec because NUMA
scanning starts late. The issue mainly concerns longer-lived CLONE_VM
tasks that accumulate NUMA faults and later exec. This issue was found
by code inspection, not a workload reproducer.

Factor the existing footprint subtraction into a helper and use it when
leaving a cache group on both exec and exit. Subtract before dropping the
old group reference, preserving the existing underflow protection.
Document that exec must subtract the contribution before
task_numa_free() resets p->total_numa_faults.

Fixes: b636fef85bda ("sched/cache: Introduce task_struct->sched_cache_grp to fix UAF")
Signed-off-by: Jemmy Wong <jemmywong512@gmail.com>
Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>

---

v1: https://lore.kernel.org/all/20261009151215.62878-1-jemmywong512@gmail.com/

Changes in v2:
- Mention vfork() and that a vfork child rarely accumulates NUMA faults
  before exec. The leak mainly matters for longer-lived CLONE_VM tasks.
  Note that this was found by code inspection.
- Add a Fixes: tag.
- Comment that footprint subtraction must stay ahead of task_numa_free()
  in bprm_execve(), and that nothing enforces that order.
- Add Reviewed-by: Tim Chen.

 kernel/sched/fair.c | 55 +++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..719a3fc3651f 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1771,6 +1771,25 @@ void sched_cache_fork_cleanup(struct task_struct *p)
 	RCU_INIT_POINTER(p->sched_cache_grp, NULL);
 }

+static void sched_cache_footprint_sub(struct sched_cache_group *grp,
+				      struct task_struct *p)
+{
+#ifdef CONFIG_NUMA_BALANCING
+	/*
+	 * Remove this task's contribution when it leaves the group, either
+	 * through exit or exec. Other thread groups sharing the old mm can
+	 * keep the group alive after exec resets this task's NUMA statistics.
+	 * Unlocked for performance; clamp to avoid underflow.
+	 */
+	if (grp && p->total_numa_faults) {
+		unsigned long fp = READ_ONCE(grp->footprint);
+		unsigned long sub = min(fp, p->total_numa_faults);
+
+		WRITE_ONCE(grp->footprint, fp - sub);
+	}
+#endif
+}
+
 void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
 {
 	struct sched_cache_group *old;
@@ -1778,8 +1797,13 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
 	/*
 	 * Acquire the new reference before publishing the pointer, then drop
 	 * the old one. @p is current and the only writer of its own pointer.
+	 *
+	 * Footprint subtraction must stay ahead of task_numa_free() in
+	 * bprm_execve(), which clears p->total_numa_faults. Nothing here
+	 * enforces that order.
 	 */
 	old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
+	sched_cache_footprint_sub(old, p);
 	sched_cache_group_put(old);
 }

@@ -1787,19 +1811,7 @@ void sched_cache_exit_mm(struct task_struct *p)
 {
 	struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);

-#ifdef CONFIG_NUMA_BALANCING
-	/*
-	 * Subtract this task's footprint from the group before dropping the
-	 * reference, so the group footprint converges as its threads exit.
-	 * Unlocked for performance; clamp to avoid underflow.
-	 */
-	if (grp && p->total_numa_faults) {
-		unsigned long fp = READ_ONCE(grp->footprint);
-		unsigned long sub = min(fp, p->total_numa_faults);
-
-		WRITE_ONCE(grp->footprint, fp - sub);
-	}
-#endif
+	sched_cache_footprint_sub(grp, p);
 	sched_cache_group_put(grp);
 }

@@ -3975,16 +3987,15 @@ static void task_numa_placement(struct task_struct *p)
 			 * sharing this mm. Acceptable since footprint is a
 			 * heuristic and occasional lost updates are tolerable.
 			 *
-			 * If a task exits, its corresponding footprint must
-			 * be subtracted from p->sched_cache_grp->footprint,
-			 * otherwise the footprint will not converge: the
-			 * exiting thread's footprint remains unchanged/undecayed.
-			 * See exit_mm().
+			 * If a task leaves its cache group through exit or exec,
+			 * its contribution must be subtracted from the old group's
+			 * footprint. Otherwise, that contribution remains
+			 * unchanged/undecayed while other tasks keep the group alive.
+			 * See sched_cache_footprint_sub().
 			 *
-			 * Lost updates and unsynchronized subtraction
-			 * in exit_mm() can cause footprint + diff to
-			 * go negative. Clamp to zero to prevent the
-			 * unsigned footprint from wrapping.
+			 * Lost updates and unsynchronized subtraction on exit or
+			 * exec can cause footprint + diff to go negative. Clamp
+			 * to zero to prevent the unsigned footprint from wrapping.
 			 */
 			scoped_guard(rcu) {
 				grp = rcu_dereference(p->sched_cache_grp);
--
2.54.0 (Apple Git-157)

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Chen Yu]

On Sat, Oct 10, 2026 at 07:16:12PM +0800, Jemmy Wong wrote:

[ ... ]

>  void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
>  {
>  	struct sched_cache_group *old;
> @@ -1778,8 +1797,13 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
>  	/*
>  	 * Acquire the new reference before publishing the pointer, then drop
>  	 * the old one. @p is current and the only writer of its own pointer.
> +	 *
> +	 * Footprint subtraction must stay ahead of task_numa_free() in
> +	 * bprm_execve(), which clears p->total_numa_faults. Nothing here
> +	 * enforces that order.
>  	 */
>  	old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
> +	sched_cache_footprint_sub(old, p);


Yes, it fixes the stale footprint issue for a vfork(). While looking at this fix I
realize that maybe even with this patch applied, we might still can not fix the losing
footprint, due to an existing race condition:

  CPU0: execve()                    CPU1: CLONE_VM task
  ------------------------------    -------------------------------
                                    task_numa_placement()
                                      read footprint
  sched_cache_exec_mmap()
    read footprint
    write footprint - own faults    <-- the fix
                                      write footprint + own diff
                                      <-- overwrites the subtraction

So the footprint might still be lost because CPU1 overwrites it, and
there seems to be no lock protected against the race(I did not find one).

An enhanced way is to turn the raw footprint write into cmpxchg.
It reduces the race, and since numa balancing scan interval 
numa_scan_period is not small, the overhead of using
atomic write is low, I'm thinking of something below(only compile tested):

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..4123f0535cd9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1771,6 +1771,11 @@ void sched_cache_fork_cleanup(struct task_struct *p)
 	RCU_INIT_POINTER(p->sched_cache_grp, NULL);
 }
 
+#ifdef CONFIG_NUMA_BALANCING
+static void sched_cache_footprint_update(struct sched_cache_group *grp,
+					 unsigned long delta, bool add);
+#endif
+
 void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
 {
 	struct sched_cache_group *old;
@@ -1780,9 +1785,28 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
 	 * the old one. @p is current and the only writer of its own pointer.
 	 */
 	old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
+#ifdef CONFIG_NUMA_BALANCING
+	if (old && p->total_numa_faults)
+		sched_cache_footprint_update(old, p->total_numa_faults, false);
+#endif
 	sched_cache_group_put(old);
 }
 
+#ifdef CONFIG_NUMA_BALANCING
+static void sched_cache_footprint_update(struct sched_cache_group *grp,
+					 unsigned long delta, bool add)
+{
+	unsigned long old = READ_ONCE(grp->footprint), new;
+
+	do {
+		if (add)
+			new = old + delta;
+		else
+			new = old - min(old, delta);
+	} while (!try_cmpxchg(&grp->footprint, &old, new));
+}
+#endif
+
 void sched_cache_exit_mm(struct task_struct *p)
 {
 	struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);
@@ -1793,12 +1817,8 @@ void sched_cache_exit_mm(struct task_struct *p)
 	 * reference, so the group footprint converges as its threads exit.
 	 * Unlocked for performance; clamp to avoid underflow.
 	 */
-	if (grp && p->total_numa_faults) {
-		unsigned long fp = READ_ONCE(grp->footprint);
-		unsigned long sub = min(fp, p->total_numa_faults);
-
-		WRITE_ONCE(grp->footprint, fp - sub);
-	}
+	if (grp && p->total_numa_faults)
+		sched_cache_footprint_update(grp, p->total_numa_faults, false);
 #endif
 	sched_cache_group_put(grp);
 }
@@ -3890,7 +3910,7 @@ static void task_numa_placement(struct task_struct *p)
 	unsigned long total_faults;
 	u64 runtime, period;
 	spinlock_t *group_lock = NULL;
-	long __maybe_unused new_fp;
+	long __maybe_unused fp_diff = 0;
 	struct numa_group *ng;
 
 	/*
@@ -3986,14 +4006,8 @@ static void task_numa_placement(struct task_struct *p)
 			 * go negative. Clamp to zero to prevent the
 			 * unsigned footprint from wrapping.
 			 */
-			scoped_guard(rcu) {
-				grp = rcu_dereference(p->sched_cache_grp);
-
-				if (grp) {
-					new_fp = (long)READ_ONCE(grp->footprint) + diff;
-					WRITE_ONCE(grp->footprint, max(new_fp, 0L));
-				}
-			}
+			/* Update grp->footprint once, to limit the c2c cost. */
+			fp_diff += diff;
 #endif
 		}
 
@@ -4008,6 +4022,18 @@ static void task_numa_placement(struct task_struct *p)
 		}
 	}
 
+#ifdef CONFIG_SCHED_CACHE
+	if (fp_diff) {
+		scoped_guard(rcu) {
+			grp = rcu_dereference(p->sched_cache_grp);
+
+			if (grp)
+				sched_cache_footprint_update(grp, abs(fp_diff),
+							     fp_diff > 0);
+		}
+	}
+#endif
+
 	/* Cannot migrate task to CPU-less node */
 	max_nid = numa_nearest_node(max_nid, N_CPU);
 
-- 
2.25.1

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Jemmy Wong]

Hi Chenyu,

Thanks for the review and for spotting the race.

> On Oct 10, 2026, at 11:04 PM, Chen Yu <yu.c.chen@intel.com> wrote:
> 
> On Sat, Oct 10, 2026 at 07:16:12PM +0800, Jemmy Wong wrote:
> 
> [ ... ]
> 
>> void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
>> {
>> struct sched_cache_group *old;
>> @@ -1778,8 +1797,13 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
>> /*
>> * Acquire the new reference before publishing the pointer, then drop
>> * the old one. @p is current and the only writer of its own pointer.
>> + *
>> + * Footprint subtraction must stay ahead of task_numa_free() in
>> + * bprm_execve(), which clears p->total_numa_faults. Nothing here
>> + * enforces that order.
>> */
>> old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
>> + sched_cache_footprint_sub(old, p);
> 
> 
> Yes, it fixes the stale footprint issue for a vfork(). While looking at this fix I
> realize that maybe even with this patch applied, we might still can not fix the losing
> footprint, due to an existing race condition:
> 
>  CPU0: execve()                    CPU1: CLONE_VM task
>  ------------------------------    -------------------------------
>                                    task_numa_placement()
>                                      read footprint
>  sched_cache_exec_mmap()
>    read footprint
>    write footprint - own faults    <-- the fix
>                                      write footprint + own diff
>                                      <-- overwrites the subtraction
> 
> So the footprint might still be lost because CPU1 overwrites it, and
> there seems to be no lock protected against the race(I did not find one).

Right, and there is no lock. grp->footprint is only ever written by plain
READ_ONCE()/WRITE_ONCE() in task_numa_placement(), exit and (with v2) exec.

> An enhanced way is to turn the raw footprint write into cmpxchg.
> It reduces the race, and since numa balancing scan interval 
> numa_scan_period is not small, the overhead of using
> atomic write is low, I'm thinking of something below(only compile tested):

This looks right to me, including applying the placement diff once.
I've taken it into my local tree with a few tweaks, explained
inline below. Feel free to push back on any of them.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 57360f5cdde4..4123f0535cd9 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1771,6 +1771,11 @@ void sched_cache_fork_cleanup(struct task_struct *p)
> RCU_INIT_POINTER(p->sched_cache_grp, NULL);
> }
> 
> +#ifdef CONFIG_NUMA_BALANCING
> +static void sched_cache_footprint_update(struct sched_cache_group *grp,
> + unsigned long delta, bool add);
> +#endif
> +
> void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
> {
> struct sched_cache_group *old;
> @@ -1780,9 +1785,28 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
> * the old one. @p is current and the only writer of its own pointer.
> */
> old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
> +#ifdef CONFIG_NUMA_BALANCING
> + if (old && p->total_numa_faults)
> + sched_cache_footprint_update(old, p->total_numa_faults, false);
> +#endif
> sched_cache_group_put(old);
> }

I folded it in without the forward declaration by ordering the helpers ahead
of the callers. The condition lives in sched_cache_footprint_drop(), with an
empty stub when CONFIG_NUMA_BALANCING is off, so exec and exit share one call:

	old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
	sched_cache_footprint_drop(old, p);
	sched_cache_group_put(old);

Moving the check into the helper costs no call: I confirmed in the
generated code that sched_cache_footprint_drop() is inlined into both
exec and exit, so the early-out test still sits in the caller.

The "must stay ahead of task_numa_free()" note moved onto that helper.
Nothing enforces the order; sched_cache_exec_mmap() just remains before
task_numa_free(current, false) in bprm_execve().

> +#ifdef CONFIG_NUMA_BALANCING
> +static void sched_cache_footprint_update(struct sched_cache_group *grp,
> + unsigned long delta, bool add)
> +{
> + unsigned long old = READ_ONCE(grp->footprint), new;
> +
> + do {
> + if (add)
> + new = old + delta;
> + else
> + new = old - min(old, delta);
> + } while (!try_cmpxchg(&grp->footprint, &old, new));
> +}
> +#endif

#ifdef CONFIG_NUMA_BALANCING
static void sched_cache_footprint_update(struct sched_cache_group *grp, long diff)
{
	unsigned long old = READ_ONCE(grp->footprint), new;

	do {
		if (diff < 0)
			new = old - min(old, -(unsigned long)diff);
		else
			new = old + diff;
	} while (!try_cmpxchg_relaxed(&grp->footprint, &old, new));
}

static void sched_cache_footprint_drop(struct sched_cache_group *grp,
				     struct task_struct *p)
{
	if (grp && p->total_numa_faults)
		sched_cache_footprint_update(grp, -(long)p->total_numa_faults);
}
#else
static inline void sched_cache_footprint_drop(struct sched_cache_group *grp,
					    struct task_struct *p) { }
#endif

Two changes in that helper:
- try_cmpxchg_relaxed() rather than try_cmpxchg().
  try_cmpxchg() is fully ordered on success; on arm64 with LSE that
  is casal. Its release half keeps the CAS from being observed before
  any earlier access from this CPU, and its acquire half keeps later
  accesses from being observed before the CAS. In exec_mmap() those
  earlier accesses include e.g. the tsk->mm and tsk->active_mm stores
  just above, which need no ordering against grp->footprint.

- Signed @diff rather than (delta, bool add), so the sign is not split
  across an abs()/">0" pair at the call site. Subtraction still clamps,
  with the negate done in unsigned arithmetic to keep the clamp in one
  type.

One question about the clamp: shouldn't atomic updates balance each
task's placement diffs against its exit/exec subtraction? Assuming no
overflow, old < delta would suggest an accounting bug that clamping
hides. Is there a legitimate path where the contributions do not balance?

If not, we could use atomic_long_t and atomic_long_add(diff,
&grp->footprint) instead of the CAS loop, provided the footprint fits
in a signed long. atomic_long_add() is already unordered; readers would
use atomic_long_read().

> void sched_cache_exit_mm(struct task_struct *p)
> {
> struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);
> @@ -1793,12 +1817,8 @@ void sched_cache_exit_mm(struct task_struct *p)
> * reference, so the group footprint converges as its threads exit.
> * Unlocked for performance; clamp to avoid underflow.
> */
> - if (grp && p->total_numa_faults) {
> - unsigned long fp = READ_ONCE(grp->footprint);
> - unsigned long sub = min(fp, p->total_numa_faults);
> -
> - WRITE_ONCE(grp->footprint, fp - sub);
> - }
> + if (grp && p->total_numa_faults)
> + sched_cache_footprint_update(grp, p->total_numa_faults, false);
> #endif
> sched_cache_group_put(grp);
> }
> @@ -3890,7 +3910,7 @@ static void task_numa_placement(struct task_struct *p)
> unsigned long total_faults;
> u64 runtime, period;
> spinlock_t *group_lock = NULL;
> - long __maybe_unused new_fp;
> + long __maybe_unused fp_diff = 0;
> struct numa_group *ng;
> 
> /*
> @@ -3986,14 +4006,8 @@ static void task_numa_placement(struct task_struct *p)
> * go negative. Clamp to zero to prevent the
> * unsigned footprint from wrapping.
> */
> - scoped_guard(rcu) {
> - grp = rcu_dereference(p->sched_cache_grp);
> -
> - if (grp) {
> - new_fp = (long)READ_ONCE(grp->footprint) + diff;
> - WRITE_ONCE(grp->footprint, max(new_fp, 0L));
> - }
> - }
> + /* Update grp->footprint once, to limit the c2c cost. */
> + fp_diff += diff;
> #endif
> }
> 
> @@ -4008,6 +4022,18 @@ static void task_numa_placement(struct task_struct *p)
> }
> }
> 
> +#ifdef CONFIG_SCHED_CACHE
> + if (fp_diff) {
> + scoped_guard(rcu) {
> + grp = rcu_dereference(p->sched_cache_grp);
> +
> + if (grp)
> + sched_cache_footprint_update(grp, abs(fp_diff),
> +     fp_diff > 0);
> + }

grp is declared inside the guard, and the signed diff is passed through:

	if (fp_diff) {
		scoped_guard(rcu) {
			struct sched_cache_group *grp;

			grp = rcu_dereference(p->sched_cache_grp);
			if (grp)
				sched_cache_footprint_update(grp, fp_diff);
		}
	}

> + }
> +#endif
> +
> /* Cannot migrate task to CPU-less node */
> max_nid = numa_nearest_node(max_nid, N_CPU);
> 
> -- 
> 2.25.1

Thanks,
Jemmy

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Jemmy Wong]

Hi Chenyu,

I've updated the patch to use atomic_long_t for grp->footprint, with
atomic_long_add() for placement deltas and atomic_long_sub() when a
task leaves the group through exec or exit.

The reason for removing the clamp is that the footprint aggregates the
same contributions tracked by p->total_numa_faults. NUMA placement
already applies each signed memory-fault delta to the per-node counters
and the task total without clamping. A negative delta only removes half
of an existing bucket, so normal decay cannot make the task's
contribution negative.

With atomic updates preventing lost deltas, and each departing task's
contribution subtracted exactly once, the group footprint should remain
nonnegative too, assuming no overflow. Clamping would hide an accounting
mismatch rather than correct it. The non-returning atomic add/sub
operations also need no ordering against unrelated accesses.

I've kept your suggestion to update the footprint once per placement
pass. Instead of maintaining a separate delta accumulator, the patch
snapshots p->total_numa_faults before the loop and applies its net change
afterward. This reuses the existing NUMA accounting and limits shared
cache-line traffic.

There is one exception to the balance argument: if exec fails after
switching groups but before task_numa_free() resets the statistics, exit
can subtract the old total from the new group, where it was never
accounted. That can make the new footprint negative. This group belongs
only to the execing task, which is past the point of no return and will
exit, so the mismatch does not persist in a surviving shared group.
This version leaves the existing NUMA reset ordering unchanged.

Does using atomic_long_t without the clamp look reasonable to you,
including handling that failed-exec case as described?



diff --git a/include/linux/sched.h b/include/linux/sched.h
index d35ae49a991f..4a02a47c988e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2412,7 +2412,8 @@ struct sched_cache_group {
 	unsigned long epoch;
 	u64 nr_running_avg;
 	unsigned long next_scan;
-	unsigned long footprint;
+	/* Sum of the member tasks' accounted footprints, in pages. */
+	atomic_long_t footprint;
 	int cpu;
 	refcount_t refcnt;
 	struct rcu_head rcu;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..c9aa59148abd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1483,6 +1483,7 @@ static bool exceed_llc_capacity(struct sched_cache_group *grp, int cpu)
 #ifdef CONFIG_NUMA_BALANCING
 	unsigned long llc, footprint;
 	struct sched_domain *sd;
+	long fp;
 	int scale;

 	guard(rcu)();
@@ -1497,7 +1498,9 @@ static bool exceed_llc_capacity(struct sched_cache_group *grp, int cpu)
 		 * excluded.
 		 */
 		llc = sd->llc_bytes;
-		footprint = READ_ONCE(grp->footprint);
+		fp = atomic_long_read(&grp->footprint);
+		WARN_ON_ONCE(fp < 0);
+		footprint = max(fp, 0L);

 		/*
 		 * Scale the LLC size by 256*llc_aggr_tolerance
@@ -1677,7 +1680,7 @@ int mm_init_sched(struct mm_struct *mm,
 	grp->cpu = -1;
 	grp->next_scan = jiffies;
 	grp->nr_running_avg = 0;
-	grp->footprint = 0;
+	atomic_long_set(&grp->footprint, 0);
 	refcount_set(&grp->refcnt, 1);
 	/*
 	 * The update to grp->pcpu_sched should not be reordered
@@ -1719,12 +1722,24 @@ DEFINE_FREE(sched_cache_group_put, struct sched_cache_group *,
 #define rcu_deref_sched_cache_grp(tsk) \
 	rcu_dereference_check((tsk)->sched_cache_grp, (tsk) == current)

+static inline void sched_cache_footprint_update(struct task_struct *p, long delta)
+{
+	struct sched_cache_group *grp = rcu_deref_sched_cache_grp(p);
+
+	if (grp && delta)
+		atomic_long_add(delta, &grp->footprint);
+}
+
 static struct sched_cache_group *sched_cache_replace_grp(struct task_struct *p,
 							 struct sched_cache_group *new)
 {
 	struct sched_cache_group *old;

 	old = rcu_deref_sched_cache_grp(p);
+#ifdef CONFIG_NUMA_BALANCING
+	if (old && old != new)
+		atomic_long_sub(p->total_numa_faults, &old->footprint);
+#endif
 	rcu_assign_pointer(p->sched_cache_grp, new);

 	return old;
@@ -1787,19 +1802,6 @@ void sched_cache_exit_mm(struct task_struct *p)
 {
 	struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);

-#ifdef CONFIG_NUMA_BALANCING
-	/*
-	 * Subtract this task's footprint from the group before dropping the
-	 * reference, so the group footprint converges as its threads exit.
-	 * Unlocked for performance; clamp to avoid underflow.
-	 */
-	if (grp && p->total_numa_faults) {
-		unsigned long fp = READ_ONCE(grp->footprint);
-		unsigned long sub = min(fp, p->total_numa_faults);
-
-		WRITE_ONCE(grp->footprint, fp - sub);
-	}
-#endif
 	sched_cache_group_put(grp);
 }

@@ -2189,6 +2191,9 @@ void init_sched_mm(struct task_struct *p)
 static inline void account_mm_sched(struct rq *rq, struct task_struct *p,
 				    s64 delta_exec) { }

+static inline void sched_cache_footprint_update(struct task_struct *p,
+						long delta) { }
+
 void init_sched_mm(struct task_struct *p) { }

 static void task_tick_cache(struct rq *rq, struct task_struct *p) { }
@@ -3883,14 +3888,13 @@ static int preferred_group_nid(struct task_struct *p, int nid)
 static void task_numa_placement(struct task_struct *p)
 	__context_unsafe(/* conditional locking */)
 {
-	struct sched_cache_group __maybe_unused *grp;
 	int seq, nid, max_nid = NUMA_NO_NODE;
 	unsigned long max_faults = 0;
 	unsigned long fault_types[2] = { 0, 0 };
+	unsigned long old_faults = p->total_numa_faults;
 	unsigned long total_faults;
 	u64 runtime, period;
 	spinlock_t *group_lock = NULL;
-	long __maybe_unused new_fp;
 	struct numa_group *ng;

 	/*
@@ -3965,36 +3969,6 @@ static void task_numa_placement(struct task_struct *p)
 				ng->total_faults += diff;
 				group_faults += ng->faults[mem_idx];
 			}
-#ifdef CONFIG_SCHED_CACHE
-			/*
-			 * Per task p->numa_faults[mem_idx] converges,
-			 * so the accumulation of each task's faults
-			 * converges too - Given the number of threads,
-			 * it cannot overflow an unsigned long.
-			 * Racy with concurrent updates from other threads
-			 * sharing this mm. Acceptable since footprint is a
-			 * heuristic and occasional lost updates are tolerable.
-			 *
-			 * If a task exits, its corresponding footprint must
-			 * be subtracted from p->sched_cache_grp->footprint,
-			 * otherwise the footprint will not converge: the
-			 * exiting thread's footprint remains unchanged/undecayed.
-			 * See exit_mm().
-			 *
-			 * Lost updates and unsynchronized subtraction
-			 * in exit_mm() can cause footprint + diff to
-			 * go negative. Clamp to zero to prevent the
-			 * unsigned footprint from wrapping.
-			 */
-			scoped_guard(rcu) {
-				grp = rcu_dereference(p->sched_cache_grp);
-
-				if (grp) {
-					new_fp = (long)READ_ONCE(grp->footprint) + diff;
-					WRITE_ONCE(grp->footprint, max(new_fp, 0L));
-				}
-			}
-#endif
 		}

 		if (!ng) {
@@ -4017,6 +3991,8 @@ static void task_numa_placement(struct task_struct *p)
 		max_nid = preferred_group_nid(p, max_nid);
 	}

+	sched_cache_footprint_update(p, p->total_numa_faults - old_faults);
+
 	if (max_faults) {
 		/* Set the new preferred node */
 		if (max_nid != p->numa_preferred_nid)
--
2.25.1


> On Oct 10, 2026, at 11:04 PM, Chen Yu <yu.c.chen@intel.com> wrote:
> 
> On Sat, Oct 10, 2026 at 07:16:12PM +0800, Jemmy Wong wrote:
> 
> [ ... ]
> 
>> void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
>> {
>> struct sched_cache_group *old;
>> @@ -1778,8 +1797,13 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
>> /*
>> * Acquire the new reference before publishing the pointer, then drop
>> * the old one. @p is current and the only writer of its own pointer.
>> + *
>> + * Footprint subtraction must stay ahead of task_numa_free() in
>> + * bprm_execve(), which clears p->total_numa_faults. Nothing here
>> + * enforces that order.
>> */
>> old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
>> + sched_cache_footprint_sub(old, p);
> 
> 
> Yes, it fixes the stale footprint issue for a vfork(). While looking at this fix I
> realize that maybe even with this patch applied, we might still can not fix the losing
> footprint, due to an existing race condition:
> 
>  CPU0: execve()                    CPU1: CLONE_VM task
>  ------------------------------    -------------------------------
>                                    task_numa_placement()
>                                      read footprint
>  sched_cache_exec_mmap()
>    read footprint
>    write footprint - own faults    <-- the fix
>                                      write footprint + own diff
>                                      <-- overwrites the subtraction
> 
> So the footprint might still be lost because CPU1 overwrites it, and
> there seems to be no lock protected against the race(I did not find one).
> 
> An enhanced way is to turn the raw footprint write into cmpxchg.
> It reduces the race, and since numa balancing scan interval 
> numa_scan_period is not small, the overhead of using
> atomic write is low, I'm thinking of something below(only compile tested):
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 57360f5cdde4..4123f0535cd9 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1771,6 +1771,11 @@ void sched_cache_fork_cleanup(struct task_struct *p)
> RCU_INIT_POINTER(p->sched_cache_grp, NULL);
> }
> 
> +#ifdef CONFIG_NUMA_BALANCING
> +static void sched_cache_footprint_update(struct sched_cache_group *grp,
> + unsigned long delta, bool add);
> +#endif
> +
> void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
> {
> struct sched_cache_group *old;
> @@ -1780,9 +1785,28 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
> * the old one. @p is current and the only writer of its own pointer.
> */
> old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
> +#ifdef CONFIG_NUMA_BALANCING
> + if (old && p->total_numa_faults)
> + sched_cache_footprint_update(old, p->total_numa_faults, false);
> +#endif
> sched_cache_group_put(old);
> }
> 
> +#ifdef CONFIG_NUMA_BALANCING
> +static void sched_cache_footprint_update(struct sched_cache_group *grp,
> + unsigned long delta, bool add)
> +{
> + unsigned long old = READ_ONCE(grp->footprint), new;
> +
> + do {
> + if (add)
> + new = old + delta;
> + else
> + new = old - min(old, delta);
> + } while (!try_cmpxchg(&grp->footprint, &old, new));
> +}
> +#endif
> +
> void sched_cache_exit_mm(struct task_struct *p)
> {
> struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);
> @@ -1793,12 +1817,8 @@ void sched_cache_exit_mm(struct task_struct *p)
> * reference, so the group footprint converges as its threads exit.
> * Unlocked for performance; clamp to avoid underflow.
> */
> - if (grp && p->total_numa_faults) {
> - unsigned long fp = READ_ONCE(grp->footprint);
> - unsigned long sub = min(fp, p->total_numa_faults);
> -
> - WRITE_ONCE(grp->footprint, fp - sub);
> - }
> + if (grp && p->total_numa_faults)
> + sched_cache_footprint_update(grp, p->total_numa_faults, false);
> #endif
> sched_cache_group_put(grp);
> }
> @@ -3890,7 +3910,7 @@ static void task_numa_placement(struct task_struct *p)
> unsigned long total_faults;
> u64 runtime, period;
> spinlock_t *group_lock = NULL;
> - long __maybe_unused new_fp;
> + long __maybe_unused fp_diff = 0;
> struct numa_group *ng;
> 
> /*
> @@ -3986,14 +4006,8 @@ static void task_numa_placement(struct task_struct *p)
> * go negative. Clamp to zero to prevent the
> * unsigned footprint from wrapping.
> */
> - scoped_guard(rcu) {
> - grp = rcu_dereference(p->sched_cache_grp);
> -
> - if (grp) {
> - new_fp = (long)READ_ONCE(grp->footprint) + diff;
> - WRITE_ONCE(grp->footprint, max(new_fp, 0L));
> - }
> - }
> + /* Update grp->footprint once, to limit the c2c cost. */
> + fp_diff += diff;
> #endif
> }
> 
> @@ -4008,6 +4022,18 @@ static void task_numa_placement(struct task_struct *p)
> }
> }
> 
> +#ifdef CONFIG_SCHED_CACHE
> + if (fp_diff) {
> + scoped_guard(rcu) {
> + grp = rcu_dereference(p->sched_cache_grp);
> +
> + if (grp)
> + sched_cache_footprint_update(grp, abs(fp_diff),
> +     fp_diff > 0);
> + }
> + }
> +#endif
> +
> /* Cannot migrate task to CPU-less node */
> max_nid = numa_nearest_node(max_nid, N_CPU);
> 
> -- 
> 2.25.1
> 
> 
> 

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Chen Yu]

Hi Jemmy,

On Wed, Oct 14, 2026 at 05:17:30PM +0800, Jemmy Wong wrote:
> 
> Hi Chenyu,
> 
> I've updated the patch to use atomic_long_t for grp->footprint, with
> atomic_long_add() for placement deltas and atomic_long_sub() when a
> task leaves the group through exec or exit.
> 
> The reason for removing the clamp is that the footprint aggregates the
> same contributions tracked by p->total_numa_faults. NUMA placement
> already applies each signed memory-fault delta to the per-node counters
> and the task total without clamping. A negative delta only removes half
> of an existing bucket, so normal decay cannot make the task's
> contribution negative.
> 
> With atomic updates preventing lost deltas, and each departing task's
> contribution subtracted exactly once, the group footprint should remain
> nonnegative too, assuming no overflow. Clamping would hide an accounting
> mismatch rather than correct it. The non-returning atomic add/sub
> operations also need no ordering against unrelated accesses.
> 
> I've kept your suggestion to update the footprint once per placement
> pass. Instead of maintaining a separate delta accumulator, the patch
> snapshots p->total_numa_faults before the loop and applies its net change
> afterward. This reuses the existing NUMA accounting and limits shared
> cache-line traffic.
> 
> There is one exception to the balance argument: if exec fails after
> switching groups but before task_numa_free() resets the statistics, exit
> can subtract the old total from the new group, where it was never
> accounted. That can make the new footprint negative. This group belongs
> only to the execing task, which is past the point of no return and will
> exit, so the mismatch does not persist in a surviving shared group.
> This version leaves the existing NUMA reset ordering unchanged.
> 
> Does using atomic_long_t without the clamp look reasonable to you,

Yes, your proposal to use atomic_long_add() is more straighforward.

> including handling that failed-exec case as described?
>

The failed-exec case is not a big deal IMO as that over-substract value will
soon disappear after the task exit.

> 
> +static inline void sched_cache_footprint_update(struct task_struct *p, long delta)
> +{
> +	struct sched_cache_group *grp = rcu_deref_sched_cache_grp(p);
> +
> +	if (grp && delta)
> +		atomic_long_add(delta, &grp->footprint);
> +}
> +
>  static struct sched_cache_group *sched_cache_replace_grp(struct task_struct *p,
>  							 struct sched_cache_group *new)
>  {
>  	struct sched_cache_group *old;
> 
>  	old = rcu_deref_sched_cache_grp(p);
> +#ifdef CONFIG_NUMA_BALANCING
> +	if (old && old != new)
> +		atomic_long_sub(p->total_numa_faults, &old->footprint);
> +#endif

The minor issue is that the goal of sched_cache_replace_grp() was to serve as a
helper for dealing with sched_cache_group pointer assignment, so we don't want to
interfere with its members' values, such as footprint. If you can reuse
sched_cache_footprint_update() in sched_cache_exit_mm() and sched_cache_exec_mmap(),
that would make the code cleaner for future maintenance. Do you have any reason
to keep the current form?

Could you please send a formal patch? I'll launch a test with your patch applied.
The only concern is that we convert the raw READ/WRITE_ONCE into atomic write,
which might impact performance.

thanks,
Chenyu

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Jemmy Wong]

> On Oct 14, 2026, at 9:03 PM, Chen Yu <chen.yu@linux.dev> wrote:

[…]

> The minor issue is that the goal of sched_cache_replace_grp() was to serve as a
> helper for dealing with sched_cache_group pointer assignment, so we don't want to
> interfere with its members' values, such as footprint. If you can reuse
> sched_cache_footprint_update() in sched_cache_exit_mm() and sched_cache_exec_mmap(),
> that would make the code cleaner for future maintenance. Do you have any reason
> to keep the current form?
> 
> Could you please send a formal patch? I'll launch a test with your patch applied.
> The only concern is that we convert the raw READ/WRITE_ONCE into atomic write,
> which might impact performance.
> 
> thanks,
> Chenyu

Hi Chenyu,

I originally put the subtraction in sched_cache_replace_grp()
so both exec and exit share it (and the CONFIG_NUMA_BALANCING
guard stays in one place).

I tried moving it into the two callers as you suggested
(patch attached). That required several small helpers to hide
the config guards and reuse the old group pointer, which
fragmented the code more than expected for this small bit of
accounting.

I agree a pure pointer-assignment helper should not touch the
footprint. I now think it is better to change its responsibility:
rename it to sched_cache_switch_grp(): have it subtract the
task's contribution from the old group, publish the new pointer,
and return the old group for the caller to put. This keeps the
leave-group accounting and NUMA guard centralized.

Would you be comfortable with that approach? I'll send a formal
v3 once we settle this. Thanks for offering to test, especially
the performance impact of the atomic updates.

Thanks,
Jemmy


diff --git a/include/linux/sched.h b/include/linux/sched.h
index d35ae49a991f..4a02a47c988e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2412,7 +2412,8 @@ struct sched_cache_group {
unsigned long epoch;
u64 nr_running_avg;
unsigned long next_scan;
- unsigned long footprint;
+ /* Sum of the member tasks' accounted footprints, in pages. */
+ atomic_long_t footprint;
int cpu;
refcount_t refcnt;
struct rcu_head rcu;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..4d635bb28f18 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1483,6 +1483,7 @@ static bool exceed_llc_capacity(struct sched_cache_group *grp, int cpu)
#ifdef CONFIG_NUMA_BALANCING
unsigned long llc, footprint;
struct sched_domain *sd;
+ long fp;
int scale;

guard(rcu)();
@@ -1497,7 +1498,9 @@ static bool exceed_llc_capacity(struct sched_cache_group *grp, int cpu)
* excluded.
*/
llc = sd->llc_bytes;
- footprint = READ_ONCE(grp->footprint);
+ fp = atomic_long_read(&grp->footprint);
+ WARN_ON_ONCE(fp < 0);
+ footprint = max(fp, 0L);

/*
* Scale the LLC size by 256*llc_aggr_tolerance
@@ -1677,7 +1680,7 @@ int mm_init_sched(struct mm_struct *mm,
grp->cpu = -1;
grp->next_scan = jiffies;
grp->nr_running_avg = 0;
- grp->footprint = 0;
+ atomic_long_set(&grp->footprint, 0);
refcount_set(&grp->refcnt, 1);
/*
* The update to grp->pcpu_sched should not be reordered
@@ -1719,6 +1722,27 @@ DEFINE_FREE(sched_cache_group_put, struct sched_cache_group *,
#define rcu_deref_sched_cache_grp(tsk) \
rcu_dereference_check((tsk)->sched_cache_grp, (tsk) == current)

+static inline void sched_cache_footprint_update(struct sched_cache_group *grp,
+ long delta)
+{
+ if (grp && delta)
+ atomic_long_add(delta, &grp->footprint);
+}
+
+static inline void sched_cache_footprint_update_task(struct task_struct *p,
+ long delta)
+{
+ sched_cache_footprint_update(rcu_deref_sched_cache_grp(p), delta);
+}
+
+static inline void sched_cache_footprint_drop(struct sched_cache_group *grp,
+ struct task_struct *p)
+{
+#ifdef CONFIG_NUMA_BALANCING
+ sched_cache_footprint_update(grp, -(long)p->total_numa_faults);
+#endif
+}
+
static struct sched_cache_group *sched_cache_replace_grp(struct task_struct *p,
struct sched_cache_group *new)
{
@@ -1780,6 +1804,7 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
* the old one. @p is current and the only writer of its own pointer.
*/
old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
+ sched_cache_footprint_drop(old, p);
sched_cache_group_put(old);
}

@@ -1787,19 +1812,7 @@ void sched_cache_exit_mm(struct task_struct *p)
{
struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);

-#ifdef CONFIG_NUMA_BALANCING
- /*
- * Subtract this task's footprint from the group before dropping the
- * reference, so the group footprint converges as its threads exit.
- * Unlocked for performance; clamp to avoid underflow.
- */
- if (grp && p->total_numa_faults) {
- unsigned long fp = READ_ONCE(grp->footprint);
- unsigned long sub = min(fp, p->total_numa_faults);
-
- WRITE_ONCE(grp->footprint, fp - sub);
- }
-#endif
+ sched_cache_footprint_drop(grp, p);
sched_cache_group_put(grp);
}

@@ -2189,6 +2202,9 @@ void init_sched_mm(struct task_struct *p)
static inline void account_mm_sched(struct rq *rq, struct task_struct *p,
s64 delta_exec) { }

+static inline void sched_cache_footprint_update_task(struct task_struct *p,
+ long delta) { }
+
void init_sched_mm(struct task_struct *p) { }

static void task_tick_cache(struct rq *rq, struct task_struct *p) { }
@@ -3883,14 +3899,13 @@ static int preferred_group_nid(struct task_struct *p, int nid)
static void task_numa_placement(struct task_struct *p)
__context_unsafe(/* conditional locking */)
{
- struct sched_cache_group __maybe_unused *grp;
int seq, nid, max_nid = NUMA_NO_NODE;
unsigned long max_faults = 0;
unsigned long fault_types[2] = { 0, 0 };
+ unsigned long old_faults = p->total_numa_faults;
unsigned long total_faults;
u64 runtime, period;
spinlock_t *group_lock = NULL;
- long __maybe_unused new_fp;
struct numa_group *ng;

/*
@@ -3965,36 +3980,6 @@ static void task_numa_placement(struct task_struct *p)
ng->total_faults += diff;
group_faults += ng->faults[mem_idx];
}
-#ifdef CONFIG_SCHED_CACHE
- /*
- * Per task p->numa_faults[mem_idx] converges,
- * so the accumulation of each task's faults
- * converges too - Given the number of threads,
- * it cannot overflow an unsigned long.
- * Racy with concurrent updates from other threads
- * sharing this mm. Acceptable since footprint is a
- * heuristic and occasional lost updates are tolerable.
- *
- * If a task exits, its corresponding footprint must
- * be subtracted from p->sched_cache_grp->footprint,
- * otherwise the footprint will not converge: the
- * exiting thread's footprint remains unchanged/undecayed.
- * See exit_mm().
- *
- * Lost updates and unsynchronized subtraction
- * in exit_mm() can cause footprint + diff to
- * go negative. Clamp to zero to prevent the
- * unsigned footprint from wrapping.
- */
- scoped_guard(rcu) {
- grp = rcu_dereference(p->sched_cache_grp);
-
- if (grp) {
- new_fp = (long)READ_ONCE(grp->footprint) + diff;
- WRITE_ONCE(grp->footprint, max(new_fp, 0L));
- }
- }
-#endif
}

if (!ng) {
@@ -4017,6 +4002,8 @@ static void task_numa_placement(struct task_struct *p)
max_nid = preferred_group_nid(p, max_nid);
}

+ sched_cache_footprint_update_task(p, p->total_numa_faults - old_faults);
+
if (max_faults) {
/* Set the new preferred node */
if (max_nid != p->numa_preferred_nid)
--
2.54.0 (Apple Git-157)

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Chen Yu]

On Thu, Oct 15, 2026 at 01:25:52AM +0800, Jemmy Wong wrote:
[…]

> I agree a pure pointer-assignment helper should not touch the
> footprint. I now think it is better to change its responsibility:
> rename it to sched_cache_switch_grp(): have it subtract the
> task's contribution from the old group, publish the new pointer,
> and return the old group for the caller to put. This keeps the
> leave-group accounting and NUMA guard centralized.
> 

Fair enough. I support this renaming proposal. Please feel free to
send a formal version.

thanks,
Chenyu