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

[Jemmy Wong <jemmywong512@gmail.com>]

> 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)