* [PATCH v2 0/1] sched: Improve cache locality of RSEQ concurrency IDs
@ 2024-09-09 21:15 Mathieu Desnoyers
2024-09-09 21:15 ` [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads Mathieu Desnoyers
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-09-09 21:15 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: linux-kernel, Mathieu Desnoyers, Valentin Schneider, Mel Gorman,
Steven Rostedt, Vincent Guittot, Dietmar Eggemann, Ben Segall,
Yury Norov, Rasmus Villemoes
Intermittent workloads behaving in bursts spaced by more than 100ms
on each CPU exhibit bad cache locality and degraded performance compared
to purely per-cpu data indexing, because concurrency IDs are allocated
over various CPUs and cores, therefore losing cache locality of the
associated data.
This series addresses this shortcoming. I observed speedups up to 16.7x
compared to plain mm_cid indexing in benchmarks.
It applies on top of v6.10.6.
This deprecates the prior "sched: NUMA-aware per-memory-map concurrency
IDs" patch series with a simpler and more general approach.
Feedback is welcome!
Thanks,
Mathieu
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Mathieu Desnoyers (1):
sched: Improve cache locality of RSEQ concurrency IDs for intermittent
workloads
fs/exec.c | 2 +-
include/linux/mm_types.h | 72 +++++++++++++++++++++++++++++++++++-----
kernel/fork.c | 2 +-
kernel/sched/core.c | 7 ++--
kernel/sched/sched.h | 47 ++++++++++++++++++--------
5 files changed, 103 insertions(+), 27 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-09 21:15 [PATCH v2 0/1] sched: Improve cache locality of RSEQ concurrency IDs Mathieu Desnoyers
@ 2024-09-09 21:15 ` Mathieu Desnoyers
2024-09-12 16:38 ` Marco Elver
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-09-09 21:15 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: linux-kernel, Mathieu Desnoyers, Valentin Schneider, Mel Gorman,
Steven Rostedt, Vincent Guittot, Dietmar Eggemann, Ben Segall,
Yury Norov, Rasmus Villemoes, Dmitry Vyukov, Marco Elver
commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
a reference to the concurrency id allocated for each CPU. This reference
expires shortly after a 100ms delay.
These per-CPU references keep the per-mm-cid data cache-local in
situations where threads are running at least once on each CPU within
each 100ms window, thus keeping the per-cpu reference alive.
However, intermittent workloads behaving in bursts spaced by more than
100ms on each CPU exhibit bad cache locality and degraded performance
compared to purely per-cpu data indexing, because concurrency IDs are
allocated over various CPUs and cores, therefore losing cache locality
of the associated data.
Introduce the following changes to improve per-mm-cid cache locality:
- Add a "recent_cid" field to the per-mm/cpu mm_cid structure to keep
track of which mm_cid value was last used, and use it as a hint to
attempt re-allocating the same concurrency ID the next time this
mm/cpu needs to allocate a concurrency ID,
- Add a per-mm CPUs allowed mask, which keeps track of the union of
CPUs allowed for all threads belonging to this mm. This cpumask is
only set during the lifetime of the mm, never cleared, so it
represents the union of all the CPUs allowed since the beginning of
the mm lifetime. (note that the mm_cpumask() is really arch-specific
and tailored to the TLB flush needs, and is thus _not_ a viable
approach for this)
- Add a per-mm nr_cpus_allowed to keep track of the weight of the
per-mm CPUs allowed mask (for fast access),
- Add a per-mm nr_cids_used to keep track of the highest concurrency
ID allocated for the mm. This is used for expanding the concurrency ID
allocation within the upper bound defined by:
min(mm->nr_cpus_allowed, mm->mm_users)
When the next unused CID value reaches this threshold, stop trying
to expand the cid allocation and use the first available cid value
instead.
Spreading allocation to use all the cid values within the range
[ 0, min(mm->nr_cpus_allowed, mm->mm_users) - 1 ]
improves cache locality while preserving mm_cid compactness within the
expected user limits.
- In __mm_cid_try_get, only return cid values within the range
[ 0, mm->nr_cpus_allowed ] rather than [ 0, nr_cpu_ids ]. This
prevents allocating cids above the number of allowed cpus in
rare scenarios where cid allocation races with a concurrent
remote-clear of the per-mm/cpu cid. This improvement is made
possible by the addition of the per-mm CPUs allowed mask.
- In sched_mm_cid_migrate_to, use mm->nr_cpus_allowed rather than
t->nr_cpus_allowed. This criterion was really meant to compare
the number of mm->mm_users to the number of CPUs allowed for the
entire mm. Therefore, the prior comparison worked fine when all
threads shared the same CPUs allowed mask, but not so much in
scenarios where those threads have different masks (e.g. each
thread pinned to a single CPU). This improvement is made
possible by the addition of the per-mm CPUs allowed mask.
* Benchmarks
Each thread increments 16kB worth of 8-bit integers in bursts, with
a configurable delay between each thread's execution. Each thread run
one after the other (no threads run concurrently). The order of
thread execution in the sequence is random. The thread execution
sequence begins again after all threads have executed. The 16kB areas
are allocated with rseq_mempool and indexed by either cpu_id, mm_cid
(not cache-local), or cache-local mm_cid. Each thread is pinned to its
own core.
Testing configurations:
8-core/1-L3: Use 8 cores within a single L3
24-core/24-L3: Use 24 cores, 1 core per L3
192-core/24-L3: Use 192 cores (all cores in the system)
384-thread/24-L3: Use 384 HW threads (all HW threads in the system)
Intermittent workload delays between threads: 200ms, 10ms.
Hardware:
CPU(s): 384
On-line CPU(s) list: 0-383
Vendor ID: AuthenticAMD
Model name: AMD EPYC 9654 96-Core Processor
Thread(s) per core: 2
Core(s) per socket: 96
Socket(s): 2
Caches (sum of all):
L1d: 6 MiB (192 instances)
L1i: 6 MiB (192 instances)
L2: 192 MiB (192 instances)
L3: 768 MiB (24 instances)
Each result is an average of 5 test runs. The cache-local speedup
is calculated as: (cache-local mm_cid) / (mm_cid).
Intermittent workload delay: 200ms
per-cpu mm_cid cache-local mm_cid cache-local speedup
(ns) (ns) (ns)
8-core/1-L3 1374 19289 1336 14.4x
24-core/24-L3 2423 26721 1594 16.7x
192-core/24-L3 2291 15826 2153 7.3x
384-thread/24-L3 1874 13234 1907 6.9x
Intermittent workload delay: 10ms
per-cpu mm_cid cache-local mm_cid cache-local speedup
(ns) (ns) (ns)
8-core/1-L3 662 756 686 1.1x
24-core/24-L3 1378 3648 1035 3.5x
192-core/24-L3 1439 10833 1482 7.3x
384-thread/24-L3 1503 10570 1556 6.8x
[ This deprecates the prior "sched: NUMA-aware per-memory-map concurrency IDs"
patch series with a simpler and more general approach. ]
Link: https://lore.kernel.org/lkml/20240823185946.418340-1-mathieu.desnoyers@efficios.com/
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Changes since v0:
- Use for_each_cpu_andnot.
- Drop "cpumask: Implement cpumask_next_andnot" from the series, which
is not needed anymore.
- Use the shorter form
"nr_set += !cpumask_test_and_set_cpu(cpu, mm_allowed);"
as suggested by Yury Norov.
- Document that ordering between mm_cpus_allowed mask and
nr_cpus_allowed is unnecessary, because the cid algorithm
only cares about nr_cpus_allowed.
Changes since v1:
- Only invoke atomic_add if value to add is nonzero.
- Use cpumask_or and cpumask_weight with locking, as suggested by Yury
Norov.
---
fs/exec.c | 2 +-
include/linux/mm_types.h | 72 +++++++++++++++++++++++++++++++++++-----
kernel/fork.c | 2 +-
kernel/sched/core.c | 7 ++--
kernel/sched/sched.h | 47 ++++++++++++++++++--------
5 files changed, 103 insertions(+), 27 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 0c17e59e3767..7e73b0fc1305 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1039,7 +1039,7 @@ static int exec_mmap(struct mm_struct *mm)
active_mm = tsk->active_mm;
tsk->active_mm = mm;
tsk->mm = mm;
- mm_init_cid(mm);
+ mm_init_cid(mm, tsk);
/*
* This prevents preemption while active_mm is being loaded and
* it and mm are being updated, which could cause problems for
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index af3a0256fa93..10d35933d0e8 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -755,6 +755,7 @@ struct vm_area_struct {
struct mm_cid {
u64 time;
int cid;
+ int recent_cid;
};
#endif
@@ -825,6 +826,27 @@ struct mm_struct {
* When the next mm_cid scan is due (in jiffies).
*/
unsigned long mm_cid_next_scan;
+ /**
+ * @nr_cpus_allowed: Number of CPUs allowed for mm.
+ *
+ * Number of CPUs allowed in the union of all mm's
+ * threads allowed CPUs.
+ */
+ atomic_t nr_cpus_allowed;
+ /**
+ * @nr_cids_used: Number of used concurrency IDs.
+ *
+ * Track the highest concurrency ID allocated for the
+ * mm: nr_cids_used - 1.
+ */
+ atomic_t nr_cids_used;
+ /**
+ * @cpus_allowed_lock: Lock protecting mm cpus_allowed.
+ *
+ * Provide mutual exclusion for mm cpus_allowed and
+ * mm nr_cpus_allowed updates.
+ */
+ spinlock_t cpus_allowed_lock;
#endif
#ifdef CONFIG_MMU
atomic_long_t pgtables_bytes; /* size of all page tables */
@@ -1143,18 +1165,30 @@ static inline int mm_cid_clear_lazy_put(int cid)
return cid & ~MM_CID_LAZY_PUT;
}
+/*
+ * mm_cpus_allowed: Union of all mm's threads allowed CPUs.
+ */
+static inline cpumask_t *mm_cpus_allowed(struct mm_struct *mm)
+{
+ unsigned long bitmap = (unsigned long)mm;
+
+ bitmap += offsetof(struct mm_struct, cpu_bitmap);
+ /* Skip cpu_bitmap */
+ bitmap += cpumask_size();
+ return (struct cpumask *)bitmap;
+}
+
/* Accessor for struct mm_struct's cidmask. */
static inline cpumask_t *mm_cidmask(struct mm_struct *mm)
{
- unsigned long cid_bitmap = (unsigned long)mm;
+ unsigned long cid_bitmap = (unsigned long)mm_cpus_allowed(mm);
- cid_bitmap += offsetof(struct mm_struct, cpu_bitmap);
- /* Skip cpu_bitmap */
+ /* Skip mm_cpus_allowed */
cid_bitmap += cpumask_size();
return (struct cpumask *)cid_bitmap;
}
-static inline void mm_init_cid(struct mm_struct *mm)
+static inline void mm_init_cid(struct mm_struct *mm, struct task_struct *p)
{
int i;
@@ -1162,17 +1196,22 @@ static inline void mm_init_cid(struct mm_struct *mm)
struct mm_cid *pcpu_cid = per_cpu_ptr(mm->pcpu_cid, i);
pcpu_cid->cid = MM_CID_UNSET;
+ pcpu_cid->recent_cid = MM_CID_UNSET;
pcpu_cid->time = 0;
}
+ atomic_set(&mm->nr_cpus_allowed, p->nr_cpus_allowed);
+ atomic_set(&mm->nr_cids_used, 0);
+ spin_lock_init(&mm->cpus_allowed_lock);
+ cpumask_copy(mm_cpus_allowed(mm), p->cpus_ptr);
cpumask_clear(mm_cidmask(mm));
}
-static inline int mm_alloc_cid_noprof(struct mm_struct *mm)
+static inline int mm_alloc_cid_noprof(struct mm_struct *mm, struct task_struct *p)
{
mm->pcpu_cid = alloc_percpu_noprof(struct mm_cid);
if (!mm->pcpu_cid)
return -ENOMEM;
- mm_init_cid(mm);
+ mm_init_cid(mm, p);
return 0;
}
#define mm_alloc_cid(...) alloc_hooks(mm_alloc_cid_noprof(__VA_ARGS__))
@@ -1185,16 +1224,31 @@ static inline void mm_destroy_cid(struct mm_struct *mm)
static inline unsigned int mm_cid_size(void)
{
- return cpumask_size();
+ return 2 * cpumask_size(); /* mm_cpus_allowed(), mm_cidmask(). */
+}
+
+static inline void mm_set_cpus_allowed(struct mm_struct *mm, const struct cpumask *cpumask)
+{
+ struct cpumask *mm_allowed = mm_cpus_allowed(mm);
+
+ if (!mm)
+ return;
+ /* The mm_cpus_allowed is the union of each thread allowed CPUs masks. */
+ spin_lock(&mm->cpus_allowed_lock);
+ cpumask_or(mm_allowed, mm_allowed, cpumask);
+ atomic_set(&mm->nr_cpus_allowed, cpumask_weight(mm_allowed));
+ spin_unlock(&mm->cpus_allowed_lock);
}
#else /* CONFIG_SCHED_MM_CID */
-static inline void mm_init_cid(struct mm_struct *mm) { }
-static inline int mm_alloc_cid(struct mm_struct *mm) { return 0; }
+static inline void mm_init_cid(struct mm_struct *mm, struct task_struct *p) { }
+static inline int mm_alloc_cid(struct mm_struct *mm, struct task_struct *p) { return 0; }
static inline void mm_destroy_cid(struct mm_struct *mm) { }
+
static inline unsigned int mm_cid_size(void)
{
return 0;
}
+static inline void mm_set_cpus_allowed(struct mm_struct *mm, const struct cpumask *cpumask) { }
#endif /* CONFIG_SCHED_MM_CID */
struct mmu_gather;
diff --git a/kernel/fork.c b/kernel/fork.c
index 99076dbe27d8..b44f545ad82c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1298,7 +1298,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
if (init_new_context(p, mm))
goto fail_nocontext;
- if (mm_alloc_cid(mm))
+ if (mm_alloc_cid(mm, p))
goto fail_cid;
if (percpu_counter_init_many(mm->rss_stat, 0, GFP_KERNEL_ACCOUNT,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3e84a3b7b7bb..3243e9abfefb 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2784,6 +2784,7 @@ __do_set_cpus_allowed(struct task_struct *p, struct affinity_context *ctx)
put_prev_task(rq, p);
p->sched_class->set_cpus_allowed(p, ctx);
+ mm_set_cpus_allowed(p->mm, ctx->new_mask);
if (queued)
enqueue_task(rq, p, ENQUEUE_RESTORE | ENQUEUE_NOCLOCK);
@@ -11784,6 +11785,7 @@ int __sched_mm_cid_migrate_from_try_steal_cid(struct rq *src_rq,
*/
if (!try_cmpxchg(&src_pcpu_cid->cid, &lazy_cid, MM_CID_UNSET))
return -1;
+ WRITE_ONCE(src_pcpu_cid->recent_cid, MM_CID_UNSET);
return src_cid;
}
@@ -11825,7 +11827,7 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
dst_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, cpu_of(dst_rq));
dst_cid = READ_ONCE(dst_pcpu_cid->cid);
if (!mm_cid_is_unset(dst_cid) &&
- atomic_read(&mm->mm_users) >= t->nr_cpus_allowed)
+ atomic_read(&mm->mm_users) >= atomic_read(&mm->nr_cpus_allowed))
return;
src_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, src_cpu);
src_rq = cpu_rq(src_cpu);
@@ -11843,6 +11845,7 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
/* Move src_cid to dst cpu. */
mm_cid_snapshot_time(dst_rq, mm);
WRITE_ONCE(dst_pcpu_cid->cid, src_cid);
+ WRITE_ONCE(dst_pcpu_cid->recent_cid, src_cid);
}
static void sched_mm_cid_remote_clear(struct mm_struct *mm, struct mm_cid *pcpu_cid,
@@ -12079,7 +12082,7 @@ void sched_mm_cid_after_execve(struct task_struct *t)
* Matches barrier in sched_mm_cid_remote_clear_old().
*/
smp_mb();
- t->last_mm_cid = t->mm_cid = mm_cid_get(rq, mm);
+ t->last_mm_cid = t->mm_cid = mm_cid_get(rq, t, mm);
}
rseq_set_notify_resume(t);
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 38aeedd8a6cc..4d11dbd5847b 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3311,24 +3311,40 @@ static inline void mm_cid_put(struct mm_struct *mm)
__mm_cid_put(mm, mm_cid_clear_lazy_put(cid));
}
-static inline int __mm_cid_try_get(struct mm_struct *mm)
+static inline int __mm_cid_try_get(struct task_struct *t, struct mm_struct *mm)
{
- struct cpumask *cpumask;
- int cid;
+ struct cpumask *cidmask = mm_cidmask(mm);
+ struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
+ int cid = __this_cpu_read(pcpu_cid->recent_cid);
- cpumask = mm_cidmask(mm);
+ /* Try to re-use recent cid. This improves cache locality. */
+ if (!mm_cid_is_unset(cid) && !cpumask_test_and_set_cpu(cid, cidmask))
+ return cid;
+ /*
+ * Expand cid allocation if used cids are below the number cpus
+ * allowed and number of threads. Expanding cid allocation as
+ * much as possible improves cache locality.
+ */
+ cid = atomic_read(&mm->nr_cids_used);
+ while (cid < atomic_read(&mm->nr_cpus_allowed) && cid < atomic_read(&mm->mm_users)) {
+ if (!atomic_try_cmpxchg(&mm->nr_cids_used, &cid, cid + 1))
+ continue;
+ if (!cpumask_test_and_set_cpu(cid, cidmask))
+ return cid;
+ }
/*
+ * Find the first available concurrency id.
* Retry finding first zero bit if the mask is temporarily
* filled. This only happens during concurrent remote-clear
* which owns a cid without holding a rq lock.
*/
for (;;) {
- cid = cpumask_first_zero(cpumask);
- if (cid < nr_cpu_ids)
+ cid = cpumask_first_zero(cidmask);
+ if (cid < atomic_read(&mm->nr_cpus_allowed))
break;
cpu_relax();
}
- if (cpumask_test_and_set_cpu(cid, cpumask))
+ if (cpumask_test_and_set_cpu(cid, cidmask))
return -1;
return cid;
}
@@ -3345,7 +3361,8 @@ static inline void mm_cid_snapshot_time(struct rq *rq, struct mm_struct *mm)
WRITE_ONCE(pcpu_cid->time, rq->clock);
}
-static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
+static inline int __mm_cid_get(struct rq *rq, struct task_struct *t,
+ struct mm_struct *mm)
{
int cid;
@@ -3355,13 +3372,13 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
* guarantee forward progress.
*/
if (!READ_ONCE(use_cid_lock)) {
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
if (cid >= 0)
goto end;
raw_spin_lock(&cid_lock);
} else {
raw_spin_lock(&cid_lock);
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
if (cid >= 0)
goto unlock;
}
@@ -3381,7 +3398,7 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
* all newcoming allocations observe the use_cid_lock flag set.
*/
do {
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
cpu_relax();
} while (cid < 0);
/*
@@ -3397,7 +3414,8 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
return cid;
}
-static inline int mm_cid_get(struct rq *rq, struct mm_struct *mm)
+static inline int mm_cid_get(struct rq *rq, struct task_struct *t,
+ struct mm_struct *mm)
{
struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
struct cpumask *cpumask;
@@ -3414,8 +3432,9 @@ static inline int mm_cid_get(struct rq *rq, struct mm_struct *mm)
if (try_cmpxchg(&this_cpu_ptr(pcpu_cid)->cid, &cid, MM_CID_UNSET))
__mm_cid_put(mm, mm_cid_clear_lazy_put(cid));
}
- cid = __mm_cid_get(rq, mm);
+ cid = __mm_cid_get(rq, t, mm);
__this_cpu_write(pcpu_cid->cid, cid);
+ __this_cpu_write(pcpu_cid->recent_cid, cid);
return cid;
}
@@ -3467,7 +3486,7 @@ static inline void switch_mm_cid(struct rq *rq,
prev->mm_cid = -1;
}
if (next->mm_cid_active)
- next->last_mm_cid = next->mm_cid = mm_cid_get(rq, next->mm);
+ next->last_mm_cid = next->mm_cid = mm_cid_get(rq, next, next->mm);
}
#else
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-09 21:15 ` [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads Mathieu Desnoyers
@ 2024-09-12 16:38 ` Marco Elver
2024-09-12 5:33 ` Mathieu Desnoyers
0 siblings, 1 reply; 11+ messages in thread
From: Marco Elver @ 2024-09-12 16:38 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Valentin Schneider,
Mel Gorman, Steven Rostedt, Vincent Guittot, Dietmar Eggemann,
Ben Segall, Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On Mon, 9 Sept 2024 at 23:15, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
> introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
> a reference to the concurrency id allocated for each CPU. This reference
> expires shortly after a 100ms delay.
>
> These per-CPU references keep the per-mm-cid data cache-local in
> situations where threads are running at least once on each CPU within
> each 100ms window, thus keeping the per-cpu reference alive.
One orthogonal idea that I recall: If a thread from a different thread
group (i.e. another process) was scheduled on that CPU, the CID can
also be invalidated because the caches are likely polluted. Fixed
values like 100ms seem rather arbitrary and it may work for one system
but not another.
> However, intermittent workloads behaving in bursts spaced by more than
> 100ms on each CPU exhibit bad cache locality and degraded performance
> compared to purely per-cpu data indexing, because concurrency IDs are
> allocated over various CPUs and cores, therefore losing cache locality
> of the associated data.
>
> Introduce the following changes to improve per-mm-cid cache locality:
>
> - Add a "recent_cid" field to the per-mm/cpu mm_cid structure to keep
> track of which mm_cid value was last used, and use it as a hint to
> attempt re-allocating the same concurrency ID the next time this
> mm/cpu needs to allocate a concurrency ID,
>
> - Add a per-mm CPUs allowed mask, which keeps track of the union of
> CPUs allowed for all threads belonging to this mm. This cpumask is
> only set during the lifetime of the mm, never cleared, so it
> represents the union of all the CPUs allowed since the beginning of
> the mm lifetime. (note that the mm_cpumask() is really arch-specific
> and tailored to the TLB flush needs, and is thus _not_ a viable
> approach for this)
>
> - Add a per-mm nr_cpus_allowed to keep track of the weight of the
> per-mm CPUs allowed mask (for fast access),
>
> - Add a per-mm nr_cids_used to keep track of the highest concurrency
> ID allocated for the mm. This is used for expanding the concurrency ID
> allocation within the upper bound defined by:
>
> min(mm->nr_cpus_allowed, mm->mm_users)
>
> When the next unused CID value reaches this threshold, stop trying
> to expand the cid allocation and use the first available cid value
> instead.
>
> Spreading allocation to use all the cid values within the range
>
> [ 0, min(mm->nr_cpus_allowed, mm->mm_users) - 1 ]
>
> improves cache locality while preserving mm_cid compactness within the
> expected user limits.
>
> - In __mm_cid_try_get, only return cid values within the range
> [ 0, mm->nr_cpus_allowed ] rather than [ 0, nr_cpu_ids ]. This
> prevents allocating cids above the number of allowed cpus in
> rare scenarios where cid allocation races with a concurrent
> remote-clear of the per-mm/cpu cid. This improvement is made
> possible by the addition of the per-mm CPUs allowed mask.
>
> - In sched_mm_cid_migrate_to, use mm->nr_cpus_allowed rather than
> t->nr_cpus_allowed. This criterion was really meant to compare
> the number of mm->mm_users to the number of CPUs allowed for the
> entire mm. Therefore, the prior comparison worked fine when all
> threads shared the same CPUs allowed mask, but not so much in
> scenarios where those threads have different masks (e.g. each
> thread pinned to a single CPU). This improvement is made
> possible by the addition of the per-mm CPUs allowed mask.
>
> * Benchmarks
>
> Each thread increments 16kB worth of 8-bit integers in bursts, with
> a configurable delay between each thread's execution. Each thread run
> one after the other (no threads run concurrently). The order of
> thread execution in the sequence is random. The thread execution
> sequence begins again after all threads have executed. The 16kB areas
> are allocated with rseq_mempool and indexed by either cpu_id, mm_cid
> (not cache-local), or cache-local mm_cid. Each thread is pinned to its
> own core.
>
> Testing configurations:
>
> 8-core/1-L3: Use 8 cores within a single L3
> 24-core/24-L3: Use 24 cores, 1 core per L3
> 192-core/24-L3: Use 192 cores (all cores in the system)
> 384-thread/24-L3: Use 384 HW threads (all HW threads in the system)
>
> Intermittent workload delays between threads: 200ms, 10ms.
>
> Hardware:
>
> CPU(s): 384
> On-line CPU(s) list: 0-383
> Vendor ID: AuthenticAMD
> Model name: AMD EPYC 9654 96-Core Processor
> Thread(s) per core: 2
> Core(s) per socket: 96
> Socket(s): 2
> Caches (sum of all):
> L1d: 6 MiB (192 instances)
> L1i: 6 MiB (192 instances)
> L2: 192 MiB (192 instances)
> L3: 768 MiB (24 instances)
>
> Each result is an average of 5 test runs. The cache-local speedup
> is calculated as: (cache-local mm_cid) / (mm_cid).
>
> Intermittent workload delay: 200ms
>
> per-cpu mm_cid cache-local mm_cid cache-local speedup
> (ns) (ns) (ns)
> 8-core/1-L3 1374 19289 1336 14.4x
> 24-core/24-L3 2423 26721 1594 16.7x
> 192-core/24-L3 2291 15826 2153 7.3x
> 384-thread/24-L3 1874 13234 1907 6.9x
>
> Intermittent workload delay: 10ms
>
> per-cpu mm_cid cache-local mm_cid cache-local speedup
> (ns) (ns) (ns)
> 8-core/1-L3 662 756 686 1.1x
> 24-core/24-L3 1378 3648 1035 3.5x
> 192-core/24-L3 1439 10833 1482 7.3x
> 384-thread/24-L3 1503 10570 1556 6.8x
>
> [ This deprecates the prior "sched: NUMA-aware per-memory-map concurrency IDs"
> patch series with a simpler and more general approach. ]
I like the simpler and more general approach vs. the NUMA-only
approach! Attempting to reallocate the previously assigned CID seems
to go a long way.
However, this doesn't quite do L3-awareness as mentioned in [1], right?
What I can tell is that this patch improves cache locality for threads
scheduled back on the _same CPU_, but not if those threads are
scheduled on a _set of CPUs_ sharing the _same L3_ cache. So if e.g. a
thread is scheduled from CPU2 to CPU3, but those 2 CPUs share the same
L3 cache, that thread will get a completely new CID and is unlikely to
hit in the L3 cache when accessing the per-CPU data.
[1] https://github.com/google/tcmalloc/issues/144#issuecomment-2307739715
Maybe I missed it, or you are planning to add it in future?
In any case, the current patch is definitely an improvement:
Acked-by: Marco Elver <elver@google.com>
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-12 16:38 ` Marco Elver
@ 2024-09-12 5:33 ` Mathieu Desnoyers
2024-09-13 12:09 ` Marco Elver
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-09-12 5:33 UTC (permalink / raw)
To: Marco Elver
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Valentin Schneider,
Mel Gorman, Steven Rostedt, Vincent Guittot, Dietmar Eggemann,
Ben Segall, Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On 2024-09-12 12:38, Marco Elver wrote:
> On Mon, 9 Sept 2024 at 23:15, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
>> introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
>> a reference to the concurrency id allocated for each CPU. This reference
>> expires shortly after a 100ms delay.
>>
>> These per-CPU references keep the per-mm-cid data cache-local in
>> situations where threads are running at least once on each CPU within
>> each 100ms window, thus keeping the per-cpu reference alive.
>
> One orthogonal idea that I recall: If a thread from a different thread
> group (i.e. another process) was scheduled on that CPU, the CID can
> also be invalidated because the caches are likely polluted. Fixed
> values like 100ms seem rather arbitrary and it may work for one system
> but not another.
That depends on the cache usage pattern of the different thread group:
it's also possible that the other thread group does not perform that
many stores to memory before the original thread group is scheduled
back, thus keeping the cache content untouched.
The ideal metric there would probably be based on PMU counters, but
I doubt we want to go there.
[...]
>
> I like the simpler and more general approach vs. the NUMA-only
> approach! Attempting to reallocate the previously assigned CID seems
> to go a long way.
Indeed it does!
>
> However, this doesn't quite do L3-awareness as mentioned in [1], right?
> What I can tell is that this patch improves cache locality for threads
> scheduled back on the _same CPU_, but not if those threads are
> scheduled on a _set of CPUs_ sharing the _same L3_ cache. So if e.g. a
> thread is scheduled from CPU2 to CPU3, but those 2 CPUs share the same
> L3 cache, that thread will get a completely new CID and is unlikely to
> hit in the L3 cache when accessing the per-CPU data.
>
> [1] https://github.com/google/tcmalloc/issues/144#issuecomment-2307739715
>
> Maybe I missed it, or you are planning to add it in future?
In my benchmarks, I noticed that preserving cache-locality at the L1 and
L2 levels was important as well.
I would like to understand better the use-case you refer to for L3
locality. AFAIU, this implies a scenario where the scheduler migrates
a thread from CPU 2 to CPU 3 (both with the same L3), and you would
like to migrate the concurrency ID along.
When the number of threads is < number of mm allowed cpus, the
migrate hooks steal the concurrency ID from CPU 2 and moves it to
CPU 3 if there is only a single thread from that mm on CPU 2, which
does what you wish.
When the number of threads is >= number of mm allowed cpus, the
migrate hook is skipped, and the concurrency ID from CPU 2 is
left in place, favoring cache locality at L1/L2 levels. In that
case it's the scheduler's decision to migrate the thread from
CPU 2 to CPU 3, so I would think improving the scheduler decisions
about migration and minimizing thread movement would be more
relevant than trying to optimize concurrency ID movement.
But I may not be fully understanding your use-case.
>
> In any case, the current patch is definitely an improvement:
>
> Acked-by: Marco Elver <elver@google.com>
Thanks a lot for your feedback!
Mathieu
>
> Thanks,
> -- Marco
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-12 5:33 ` Mathieu Desnoyers
@ 2024-09-13 12:09 ` Marco Elver
2024-09-15 10:11 ` Mathieu Desnoyers
0 siblings, 1 reply; 11+ messages in thread
From: Marco Elver @ 2024-09-13 12:09 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Valentin Schneider,
Mel Gorman, Steven Rostedt, Vincent Guittot, Dietmar Eggemann,
Ben Segall, Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On Thu, 12 Sept 2024 at 19:34, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2024-09-12 12:38, Marco Elver wrote:
> > On Mon, 9 Sept 2024 at 23:15, Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> >>
> >> commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
> >> introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
> >> a reference to the concurrency id allocated for each CPU. This reference
> >> expires shortly after a 100ms delay.
> >>
> >> These per-CPU references keep the per-mm-cid data cache-local in
> >> situations where threads are running at least once on each CPU within
> >> each 100ms window, thus keeping the per-cpu reference alive.
> >
> > One orthogonal idea that I recall: If a thread from a different thread
> > group (i.e. another process) was scheduled on that CPU, the CID can
> > also be invalidated because the caches are likely polluted. Fixed
> > values like 100ms seem rather arbitrary and it may work for one system
> > but not another.
>
> That depends on the cache usage pattern of the different thread group:
> it's also possible that the other thread group does not perform that
> many stores to memory before the original thread group is scheduled
> back, thus keeping the cache content untouched.
>
> The ideal metric there would probably be based on PMU counters, but
> I doubt we want to go there.
>
> [...]
> >
> > I like the simpler and more general approach vs. the NUMA-only
> > approach! Attempting to reallocate the previously assigned CID seems
> > to go a long way.
>
> Indeed it does!
>
> >
> > However, this doesn't quite do L3-awareness as mentioned in [1], right?
> > What I can tell is that this patch improves cache locality for threads
> > scheduled back on the _same CPU_, but not if those threads are
> > scheduled on a _set of CPUs_ sharing the _same L3_ cache. So if e.g. a
> > thread is scheduled from CPU2 to CPU3, but those 2 CPUs share the same
> > L3 cache, that thread will get a completely new CID and is unlikely to
> > hit in the L3 cache when accessing the per-CPU data.
> >
> > [1] https://github.com/google/tcmalloc/issues/144#issuecomment-2307739715
> >
> > Maybe I missed it, or you are planning to add it in future?
>
> In my benchmarks, I noticed that preserving cache-locality at the L1 and
> L2 levels was important as well.
>
> I would like to understand better the use-case you refer to for L3
> locality. AFAIU, this implies a scenario where the scheduler migrates
> a thread from CPU 2 to CPU 3 (both with the same L3), and you would
> like to migrate the concurrency ID along.
Either migrate it along, _or_ pick a CID from a different thread that
ran on a CPU that shares this L3. E.g. if T1 is migrated from CPU2 to
CPU3, and T2 ran on CPU3 before, then it would be ok for T1 to get its
previous CID or T2's CID from when it ran on CPU3. Or more simply,
CIDs aren't tied to particular threads, but tied to a subset of CPUs
based on topology. If the user could specify that topology / CID
affinity would be nice.
> When the number of threads is < number of mm allowed cpus, the
> migrate hooks steal the concurrency ID from CPU 2 and moves it to
> CPU 3 if there is only a single thread from that mm on CPU 2, which
> does what you wish.
Only if the next CPU shares the cache. What if it moves the thread to
a CPU where that CPU's L3 cache != the previous CPU's L3 cache. In
that case, it'd be preferable to pick a last-used CID from the set of
CPUs that are grouped under that L3 cache.
> When the number of threads is >= number of mm allowed cpus, the
> migrate hook is skipped, and the concurrency ID from CPU 2 is
> left in place, favoring cache locality at L1/L2 levels.
... and any higher level caches, too, I'd assume.
> In that
> case it's the scheduler's decision to migrate the thread from
> CPU 2 to CPU 3, so I would think improving the scheduler decisions
> about migration and minimizing thread movement would be more
> relevant than trying to optimize concurrency ID movement.
From what I gather, if the CID is left in place on a CPU, and the next
thread just grabs it, that's already optimal AFAIK.
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-13 12:09 ` Marco Elver
@ 2024-09-15 10:11 ` Mathieu Desnoyers
2024-09-19 6:00 ` Marco Elver
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-09-15 10:11 UTC (permalink / raw)
To: Marco Elver
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Valentin Schneider,
Mel Gorman, Steven Rostedt, Vincent Guittot, Dietmar Eggemann,
Ben Segall, Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On 2024-09-13 14:09, Marco Elver wrote:
> On Thu, 12 Sept 2024 at 19:34, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> On 2024-09-12 12:38, Marco Elver wrote:
>>> On Mon, 9 Sept 2024 at 23:15, Mathieu Desnoyers
>>> <mathieu.desnoyers@efficios.com> wrote:
>>>>
>>>> commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
>>>> introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
>>>> a reference to the concurrency id allocated for each CPU. This reference
>>>> expires shortly after a 100ms delay.
>>>>
>>>> These per-CPU references keep the per-mm-cid data cache-local in
>>>> situations where threads are running at least once on each CPU within
>>>> each 100ms window, thus keeping the per-cpu reference alive.
>>>
>>> One orthogonal idea that I recall: If a thread from a different thread
>>> group (i.e. another process) was scheduled on that CPU, the CID can
>>> also be invalidated because the caches are likely polluted. Fixed
>>> values like 100ms seem rather arbitrary and it may work for one system
>>> but not another.
>>
>> That depends on the cache usage pattern of the different thread group:
>> it's also possible that the other thread group does not perform that
>> many stores to memory before the original thread group is scheduled
>> back, thus keeping the cache content untouched.
>>
>> The ideal metric there would probably be based on PMU counters, but
>> I doubt we want to go there.
>>
>> [...]
>>>
>>> I like the simpler and more general approach vs. the NUMA-only
>>> approach! Attempting to reallocate the previously assigned CID seems
>>> to go a long way.
>>
>> Indeed it does!
>>
>>>
>>> However, this doesn't quite do L3-awareness as mentioned in [1], right?
>>> What I can tell is that this patch improves cache locality for threads
>>> scheduled back on the _same CPU_, but not if those threads are
>>> scheduled on a _set of CPUs_ sharing the _same L3_ cache. So if e.g. a
>>> thread is scheduled from CPU2 to CPU3, but those 2 CPUs share the same
>>> L3 cache, that thread will get a completely new CID and is unlikely to
>>> hit in the L3 cache when accessing the per-CPU data.
>>>
>>> [1] https://github.com/google/tcmalloc/issues/144#issuecomment-2307739715
>>>
>>> Maybe I missed it, or you are planning to add it in future?
>>
>> In my benchmarks, I noticed that preserving cache-locality at the L1 and
>> L2 levels was important as well.
>>
>> I would like to understand better the use-case you refer to for L3
>> locality. AFAIU, this implies a scenario where the scheduler migrates
>> a thread from CPU 2 to CPU 3 (both with the same L3), and you would
>> like to migrate the concurrency ID along.
>
> Either migrate it along, _or_ pick a CID from a different thread that
> ran on a CPU that shares this L3. E.g. if T1 is migrated from CPU2 to
> CPU3, and T2 ran on CPU3 before, then it would be ok for T1 to get its
> previous CID or T2's CID from when it ran on CPU3. Or more simply,
> CIDs aren't tied to particular threads, but tied to a subset of CPUs
> based on topology. If the user could specify that topology / CID
> affinity would be nice.
There is probably something to improve there, but I suspect this
is beyond the scope of this patch, and would prefer tackling this
topology-aware CID stealing as a separate effort. I fear that attempting
to be too aggressive in keeping the CID allocation compact on migration
may require us to set/clear bits in the mm_cidmask more often, which may
impact some workloads. If we look into this we need to be careful about
regressions.
>
>> When the number of threads is < number of mm allowed cpus, the
>> migrate hooks steal the concurrency ID from CPU 2 and moves it to
>> CPU 3 if there is only a single thread from that mm on CPU 2, which
>> does what you wish.
>
> Only if the next CPU shares the cache. What if it moves the thread to
> a CPU where that CPU's L3 cache != the previous CPU's L3 cache. In
> that case, it'd be preferable to pick a last-used CID from the set of
> CPUs that are grouped under that L3 cache.
Without going all the way towards making this topology-aware, one
improvement I would do for the next version of this patch is to
prevent moving the src CID to the destination cpu (on migration)
when the dst cpu has a recent_cid set. Basically this:
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 3243e9abfefb..3e290d8609fb 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -11798,7 +11798,8 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
{
struct mm_cid *src_pcpu_cid, *dst_pcpu_cid;
struct mm_struct *mm = t->mm;
- int src_cid, dst_cid, src_cpu;
+ int src_cid, src_cpu;
+ bool dst_cid_is_set;
struct rq *src_rq;
lockdep_assert_rq_held(dst_rq);
@@ -11815,9 +11816,9 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
* allocation closest to 0 in cases where few threads migrate around
* many cpus.
*
- * If destination cid is already set, we may have to just clear
- * the src cid to ensure compactness in frequent migrations
- * scenarios.
+ * If destination cid or recent cid is already set, we may have
+ * to just clear the src cid to ensure compactness in frequent
+ * migrations scenarios.
*
* It is not useful to clear the src cid when the number of threads is
* greater or equal to the number of allowed cpus, because user-space
@@ -11825,9 +11826,9 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
* allowed cpus.
*/
dst_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, cpu_of(dst_rq));
- dst_cid = READ_ONCE(dst_pcpu_cid->cid);
- if (!mm_cid_is_unset(dst_cid) &&
- atomic_read(&mm->mm_users) >= atomic_read(&mm->nr_cpus_allowed))
+ dst_cid_is_set = !mm_cid_is_unset(READ_ONCE(dst_pcpu_cid->cid)) ||
+ !mm_cid_is_unset(READ_ONCE(dst_pcpu_cid->recent_cid));
+ if (dst_cid_is_set && atomic_read(&mm->mm_users) >= atomic_read(&mm->nr_cpus_allowed))
return;
src_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, src_cpu);
src_rq = cpu_rq(src_cpu);
@@ -11838,7 +11839,7 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
src_cid);
if (src_cid == -1)
return;
- if (!mm_cid_is_unset(dst_cid)) {
+ if (dst_cid_is_set) {
__mm_cid_put(mm, src_cid);
return;
}
>
>> When the number of threads is >= number of mm allowed cpus, the
>> migrate hook is skipped, and the concurrency ID from CPU 2 is
>> left in place, favoring cache locality at L1/L2 levels.
>
> ... and any higher level caches, too, I'd assume.
Yes.
>
>> In that
>> case it's the scheduler's decision to migrate the thread from
>> CPU 2 to CPU 3, so I would think improving the scheduler decisions
>> about migration and minimizing thread movement would be more
>> relevant than trying to optimize concurrency ID movement.
>
> From what I gather, if the CID is left in place on a CPU, and the next
> thread just grabs it, that's already optimal AFAIK.
That's indeed the behavior when nr threads >= weight(mm cpus allowed),
which is optimal.
When the number of threads is < weight(mm cpus allowed), this is were
we may want to consider being more aggressive in stealing an
unused already-allocated CID or reserve a recent CID value which is
observed close to the destination cpu within the topology on
thread migration in the future, but the benefit of doing that
vs its downsides is not immediately clear to me. E.g. this
could scan the topology_{sibling,core,cluster,llc}_cpumask
in that order for unused recently used CID to reserve, or
lazy-put CIDs to steal.
Thanks,
Mathieu
>
> Thanks,
> -- Marco
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-15 10:11 ` Mathieu Desnoyers
@ 2024-09-19 6:00 ` Marco Elver
0 siblings, 0 replies; 11+ messages in thread
From: Marco Elver @ 2024-09-19 6:00 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Valentin Schneider,
Mel Gorman, Steven Rostedt, Vincent Guittot, Dietmar Eggemann,
Ben Segall, Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On Mon, 16 Sept 2024 at 12:12, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
[...]
> > Either migrate it along, _or_ pick a CID from a different thread that
> > ran on a CPU that shares this L3. E.g. if T1 is migrated from CPU2 to
> > CPU3, and T2 ran on CPU3 before, then it would be ok for T1 to get its
> > previous CID or T2's CID from when it ran on CPU3. Or more simply,
> > CIDs aren't tied to particular threads, but tied to a subset of CPUs
> > based on topology. If the user could specify that topology / CID
> > affinity would be nice.
>
> There is probably something to improve there, but I suspect this
> is beyond the scope of this patch, and would prefer tackling this
> topology-aware CID stealing as a separate effort. I fear that attempting
> to be too aggressive in keeping the CID allocation compact on migration
> may require us to set/clear bits in the mm_cidmask more often, which may
> impact some workloads. If we look into this we need to be careful about
> regressions.
As discussed at LPC, narrowing down a generically optimal policy is
hard. It's easy to overfit for any one particular workload, system,
etc. So the current approach certainly works. One step at a time. :-)
To cater better to different systems or workloads, it might make sense
to consider giving user space more control over the policy. Various
options exist, from tweaking sysctl knobs to eBPF hooks. My preference
here would be towards eBPF hooks that can influence CID partitioning
dynamically. But that's probably something to think about for the
future - I have no good answer, only that some way to experiment more
rapidly would help to narrow down what's optimal.
> >> When the number of threads is < number of mm allowed cpus, the
> >> migrate hooks steal the concurrency ID from CPU 2 and moves it to
> >> CPU 3 if there is only a single thread from that mm on CPU 2, which
> >> does what you wish.
> >
> > Only if the next CPU shares the cache. What if it moves the thread to
> > a CPU where that CPU's L3 cache != the previous CPU's L3 cache. In
> > that case, it'd be preferable to pick a last-used CID from the set of
> > CPUs that are grouped under that L3 cache.
>
> Without going all the way towards making this topology-aware, one
> improvement I would do for the next version of this patch is to
> prevent moving the src CID to the destination cpu (on migration)
> when the dst cpu has a recent_cid set. Basically this:
I think that makes sense.
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/1] sched: Improve cache locality of RSEQ concurrency IDs
@ 2024-09-30 18:59 Mathieu Desnoyers
2024-09-30 18:59 ` [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads Mathieu Desnoyers
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-09-30 18:59 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: linux-kernel, Mathieu Desnoyers, Valentin Schneider, Mel Gorman,
Steven Rostedt, Vincent Guittot, Dietmar Eggemann, Ben Segall,
Yury Norov, Rasmus Villemoes
Intermittent workloads behaving in bursts spaced by more than 100ms
on each CPU exhibit bad cache locality and degraded performance compared
to purely per-cpu data indexing, because concurrency IDs are allocated
over various CPUs and cores, therefore losing cache locality of the
associated data.
This series addresses this shortcoming. I observed speedups up to 16.7x
compared to plain mm_cid indexing in benchmarks.
It is based on tag v6.11.1.
This deprecates the prior "sched: NUMA-aware per-memory-map concurrency
IDs" patch series with a simpler and more general approach.
Peter, can you pick it up through the tip tree ?
Thanks,
Mathieu
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Mathieu Desnoyers (1):
sched: Improve cache locality of RSEQ concurrency IDs for intermittent
workloads
fs/exec.c | 2 +-
include/linux/mm_types.h | 72 +++++++++++++++++++++++++++++++++++-----
kernel/fork.c | 2 +-
kernel/sched/core.c | 22 +++++++-----
kernel/sched/sched.h | 47 ++++++++++++++++++--------
5 files changed, 111 insertions(+), 34 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-30 18:59 [PATCH v2 0/1] sched: Improve cache locality of RSEQ concurrency IDs Mathieu Desnoyers
@ 2024-09-30 18:59 ` Mathieu Desnoyers
2024-10-02 9:49 ` Marco Elver
0 siblings, 1 reply; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-09-30 18:59 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: linux-kernel, Mathieu Desnoyers, Valentin Schneider, Mel Gorman,
Steven Rostedt, Vincent Guittot, Dietmar Eggemann, Ben Segall,
Yury Norov, Rasmus Villemoes, Marco Elver, Dmitry Vyukov
commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
a reference to the concurrency id allocated for each CPU. This reference
expires shortly after a 100ms delay.
These per-CPU references keep the per-mm-cid data cache-local in
situations where threads are running at least once on each CPU within
each 100ms window, thus keeping the per-cpu reference alive.
However, intermittent workloads behaving in bursts spaced by more than
100ms on each CPU exhibit bad cache locality and degraded performance
compared to purely per-cpu data indexing, because concurrency IDs are
allocated over various CPUs and cores, therefore losing cache locality
of the associated data.
Introduce the following changes to improve per-mm-cid cache locality:
- Add a "recent_cid" field to the per-mm/cpu mm_cid structure to keep
track of which mm_cid value was last used, and use it as a hint to
attempt re-allocating the same concurrency ID the next time this
mm/cpu needs to allocate a concurrency ID,
- Add a per-mm CPUs allowed mask, which keeps track of the union of
CPUs allowed for all threads belonging to this mm. This cpumask is
only set during the lifetime of the mm, never cleared, so it
represents the union of all the CPUs allowed since the beginning of
the mm lifetime. (note that the mm_cpumask() is really arch-specific
and tailored to the TLB flush needs, and is thus _not_ a viable
approach for this)
- Add a per-mm nr_cpus_allowed to keep track of the weight of the
per-mm CPUs allowed mask (for fast access),
- Add a per-mm nr_cids_used to keep track of the highest concurrency
ID allocated for the mm. This is used for expanding the concurrency ID
allocation within the upper bound defined by:
min(mm->nr_cpus_allowed, mm->mm_users)
When the next unused CID value reaches this threshold, stop trying
to expand the cid allocation and use the first available cid value
instead.
Spreading allocation to use all the cid values within the range
[ 0, min(mm->nr_cpus_allowed, mm->mm_users) - 1 ]
improves cache locality while preserving mm_cid compactness within the
expected user limits.
- In __mm_cid_try_get, only return cid values within the range
[ 0, mm->nr_cpus_allowed ] rather than [ 0, nr_cpu_ids ]. This
prevents allocating cids above the number of allowed cpus in
rare scenarios where cid allocation races with a concurrent
remote-clear of the per-mm/cpu cid. This improvement is made
possible by the addition of the per-mm CPUs allowed mask.
- In sched_mm_cid_migrate_to, use mm->nr_cpus_allowed rather than
t->nr_cpus_allowed. This criterion was really meant to compare
the number of mm->mm_users to the number of CPUs allowed for the
entire mm. Therefore, the prior comparison worked fine when all
threads shared the same CPUs allowed mask, but not so much in
scenarios where those threads have different masks (e.g. each
thread pinned to a single CPU). This improvement is made
possible by the addition of the per-mm CPUs allowed mask.
* Benchmarks
Each thread increments 16kB worth of 8-bit integers in bursts, with
a configurable delay between each thread's execution. Each thread run
one after the other (no threads run concurrently). The order of
thread execution in the sequence is random. The thread execution
sequence begins again after all threads have executed. The 16kB areas
are allocated with rseq_mempool and indexed by either cpu_id, mm_cid
(not cache-local), or cache-local mm_cid. Each thread is pinned to its
own core.
Testing configurations:
8-core/1-L3: Use 8 cores within a single L3
24-core/24-L3: Use 24 cores, 1 core per L3
192-core/24-L3: Use 192 cores (all cores in the system)
384-thread/24-L3: Use 384 HW threads (all HW threads in the system)
Intermittent workload delays between threads: 200ms, 10ms.
Hardware:
CPU(s): 384
On-line CPU(s) list: 0-383
Vendor ID: AuthenticAMD
Model name: AMD EPYC 9654 96-Core Processor
Thread(s) per core: 2
Core(s) per socket: 96
Socket(s): 2
Caches (sum of all):
L1d: 6 MiB (192 instances)
L1i: 6 MiB (192 instances)
L2: 192 MiB (192 instances)
L3: 768 MiB (24 instances)
Each result is an average of 5 test runs. The cache-local speedup
is calculated as: (cache-local mm_cid) / (mm_cid).
Intermittent workload delay: 200ms
per-cpu mm_cid cache-local mm_cid cache-local speedup
(ns) (ns) (ns)
8-core/1-L3 1374 19289 1336 14.4x
24-core/24-L3 2423 26721 1594 16.7x
192-core/24-L3 2291 15826 2153 7.3x
384-thread/24-L3 1874 13234 1907 6.9x
Intermittent workload delay: 10ms
per-cpu mm_cid cache-local mm_cid cache-local speedup
(ns) (ns) (ns)
8-core/1-L3 662 756 686 1.1x
24-core/24-L3 1378 3648 1035 3.5x
192-core/24-L3 1439 10833 1482 7.3x
384-thread/24-L3 1503 10570 1556 6.8x
[ This deprecates the prior "sched: NUMA-aware per-memory-map concurrency IDs"
patch series with a simpler and more general approach. ]
Link: https://lore.kernel.org/lkml/20240823185946.418340-1-mathieu.desnoyers@efficios.com/
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Acked-by: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Changes since v0:
- On migration, do not move the source cid to the destination cpu if the
destination cpu has a recent cid value set.
Changes since v2:
- Rebase on v6.11.1.
---
fs/exec.c | 2 +-
include/linux/mm_types.h | 72 +++++++++++++++++++++++++++++++++++-----
kernel/fork.c | 2 +-
kernel/sched/core.c | 22 +++++++-----
kernel/sched/sched.h | 47 ++++++++++++++++++--------
5 files changed, 111 insertions(+), 34 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 50e76cc633c4..b685c1945f13 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1065,7 +1065,7 @@ static int exec_mmap(struct mm_struct *mm)
active_mm = tsk->active_mm;
tsk->active_mm = mm;
tsk->mm = mm;
- mm_init_cid(mm);
+ mm_init_cid(mm, tsk);
/*
* This prevents preemption while active_mm is being loaded and
* it and mm are being updated, which could cause problems for
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 485424979254..22c01e91de58 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -771,6 +771,7 @@ struct vm_area_struct {
struct mm_cid {
u64 time;
int cid;
+ int recent_cid;
};
#endif
@@ -841,6 +842,27 @@ struct mm_struct {
* When the next mm_cid scan is due (in jiffies).
*/
unsigned long mm_cid_next_scan;
+ /**
+ * @nr_cpus_allowed: Number of CPUs allowed for mm.
+ *
+ * Number of CPUs allowed in the union of all mm's
+ * threads allowed CPUs.
+ */
+ atomic_t nr_cpus_allowed;
+ /**
+ * @nr_cids_used: Number of used concurrency IDs.
+ *
+ * Track the highest concurrency ID allocated for the
+ * mm: nr_cids_used - 1.
+ */
+ atomic_t nr_cids_used;
+ /**
+ * @cpus_allowed_lock: Lock protecting mm cpus_allowed.
+ *
+ * Provide mutual exclusion for mm cpus_allowed and
+ * mm nr_cpus_allowed updates.
+ */
+ spinlock_t cpus_allowed_lock;
#endif
#ifdef CONFIG_MMU
atomic_long_t pgtables_bytes; /* size of all page tables */
@@ -1159,18 +1181,30 @@ static inline int mm_cid_clear_lazy_put(int cid)
return cid & ~MM_CID_LAZY_PUT;
}
+/*
+ * mm_cpus_allowed: Union of all mm's threads allowed CPUs.
+ */
+static inline cpumask_t *mm_cpus_allowed(struct mm_struct *mm)
+{
+ unsigned long bitmap = (unsigned long)mm;
+
+ bitmap += offsetof(struct mm_struct, cpu_bitmap);
+ /* Skip cpu_bitmap */
+ bitmap += cpumask_size();
+ return (struct cpumask *)bitmap;
+}
+
/* Accessor for struct mm_struct's cidmask. */
static inline cpumask_t *mm_cidmask(struct mm_struct *mm)
{
- unsigned long cid_bitmap = (unsigned long)mm;
+ unsigned long cid_bitmap = (unsigned long)mm_cpus_allowed(mm);
- cid_bitmap += offsetof(struct mm_struct, cpu_bitmap);
- /* Skip cpu_bitmap */
+ /* Skip mm_cpus_allowed */
cid_bitmap += cpumask_size();
return (struct cpumask *)cid_bitmap;
}
-static inline void mm_init_cid(struct mm_struct *mm)
+static inline void mm_init_cid(struct mm_struct *mm, struct task_struct *p)
{
int i;
@@ -1178,17 +1212,22 @@ static inline void mm_init_cid(struct mm_struct *mm)
struct mm_cid *pcpu_cid = per_cpu_ptr(mm->pcpu_cid, i);
pcpu_cid->cid = MM_CID_UNSET;
+ pcpu_cid->recent_cid = MM_CID_UNSET;
pcpu_cid->time = 0;
}
+ atomic_set(&mm->nr_cpus_allowed, p->nr_cpus_allowed);
+ atomic_set(&mm->nr_cids_used, 0);
+ spin_lock_init(&mm->cpus_allowed_lock);
+ cpumask_copy(mm_cpus_allowed(mm), p->cpus_ptr);
cpumask_clear(mm_cidmask(mm));
}
-static inline int mm_alloc_cid_noprof(struct mm_struct *mm)
+static inline int mm_alloc_cid_noprof(struct mm_struct *mm, struct task_struct *p)
{
mm->pcpu_cid = alloc_percpu_noprof(struct mm_cid);
if (!mm->pcpu_cid)
return -ENOMEM;
- mm_init_cid(mm);
+ mm_init_cid(mm, p);
return 0;
}
#define mm_alloc_cid(...) alloc_hooks(mm_alloc_cid_noprof(__VA_ARGS__))
@@ -1201,16 +1240,31 @@ static inline void mm_destroy_cid(struct mm_struct *mm)
static inline unsigned int mm_cid_size(void)
{
- return cpumask_size();
+ return 2 * cpumask_size(); /* mm_cpus_allowed(), mm_cidmask(). */
+}
+
+static inline void mm_set_cpus_allowed(struct mm_struct *mm, const struct cpumask *cpumask)
+{
+ struct cpumask *mm_allowed = mm_cpus_allowed(mm);
+
+ if (!mm)
+ return;
+ /* The mm_cpus_allowed is the union of each thread allowed CPUs masks. */
+ spin_lock(&mm->cpus_allowed_lock);
+ cpumask_or(mm_allowed, mm_allowed, cpumask);
+ atomic_set(&mm->nr_cpus_allowed, cpumask_weight(mm_allowed));
+ spin_unlock(&mm->cpus_allowed_lock);
}
#else /* CONFIG_SCHED_MM_CID */
-static inline void mm_init_cid(struct mm_struct *mm) { }
-static inline int mm_alloc_cid(struct mm_struct *mm) { return 0; }
+static inline void mm_init_cid(struct mm_struct *mm, struct task_struct *p) { }
+static inline int mm_alloc_cid(struct mm_struct *mm, struct task_struct *p) { return 0; }
static inline void mm_destroy_cid(struct mm_struct *mm) { }
+
static inline unsigned int mm_cid_size(void)
{
return 0;
}
+static inline void mm_set_cpus_allowed(struct mm_struct *mm, const struct cpumask *cpumask) { }
#endif /* CONFIG_SCHED_MM_CID */
struct mmu_gather;
diff --git a/kernel/fork.c b/kernel/fork.c
index cc760491f201..7e56a63cc4ed 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1296,7 +1296,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
if (init_new_context(p, mm))
goto fail_nocontext;
- if (mm_alloc_cid(mm))
+ if (mm_alloc_cid(mm, p))
goto fail_cid;
if (percpu_counter_init_many(mm->rss_stat, 0, GFP_KERNEL_ACCOUNT,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f3951e4a55e5..08d57070209a 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2602,6 +2602,7 @@ __do_set_cpus_allowed(struct task_struct *p, struct affinity_context *ctx)
put_prev_task(rq, p);
p->sched_class->set_cpus_allowed(p, ctx);
+ mm_set_cpus_allowed(p->mm, ctx->new_mask);
if (queued)
enqueue_task(rq, p, ENQUEUE_RESTORE | ENQUEUE_NOCLOCK);
@@ -10025,6 +10026,7 @@ int __sched_mm_cid_migrate_from_try_steal_cid(struct rq *src_rq,
*/
if (!try_cmpxchg(&src_pcpu_cid->cid, &lazy_cid, MM_CID_UNSET))
return -1;
+ WRITE_ONCE(src_pcpu_cid->recent_cid, MM_CID_UNSET);
return src_cid;
}
@@ -10037,7 +10039,8 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
{
struct mm_cid *src_pcpu_cid, *dst_pcpu_cid;
struct mm_struct *mm = t->mm;
- int src_cid, dst_cid, src_cpu;
+ int src_cid, src_cpu;
+ bool dst_cid_is_set;
struct rq *src_rq;
lockdep_assert_rq_held(dst_rq);
@@ -10054,9 +10057,9 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
* allocation closest to 0 in cases where few threads migrate around
* many CPUs.
*
- * If destination cid is already set, we may have to just clear
- * the src cid to ensure compactness in frequent migrations
- * scenarios.
+ * If destination cid or recent cid is already set, we may have
+ * to just clear the src cid to ensure compactness in frequent
+ * migrations scenarios.
*
* It is not useful to clear the src cid when the number of threads is
* greater or equal to the number of allowed CPUs, because user-space
@@ -10064,9 +10067,9 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
* allowed CPUs.
*/
dst_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, cpu_of(dst_rq));
- dst_cid = READ_ONCE(dst_pcpu_cid->cid);
- if (!mm_cid_is_unset(dst_cid) &&
- atomic_read(&mm->mm_users) >= t->nr_cpus_allowed)
+ dst_cid_is_set = !mm_cid_is_unset(READ_ONCE(dst_pcpu_cid->cid)) ||
+ !mm_cid_is_unset(READ_ONCE(dst_pcpu_cid->recent_cid));
+ if (dst_cid_is_set && atomic_read(&mm->mm_users) >= atomic_read(&mm->nr_cpus_allowed))
return;
src_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, src_cpu);
src_rq = cpu_rq(src_cpu);
@@ -10077,13 +10080,14 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
src_cid);
if (src_cid == -1)
return;
- if (!mm_cid_is_unset(dst_cid)) {
+ if (dst_cid_is_set) {
__mm_cid_put(mm, src_cid);
return;
}
/* Move src_cid to dst cpu. */
mm_cid_snapshot_time(dst_rq, mm);
WRITE_ONCE(dst_pcpu_cid->cid, src_cid);
+ WRITE_ONCE(dst_pcpu_cid->recent_cid, src_cid);
}
static void sched_mm_cid_remote_clear(struct mm_struct *mm, struct mm_cid *pcpu_cid,
@@ -10320,7 +10324,7 @@ void sched_mm_cid_after_execve(struct task_struct *t)
* Matches barrier in sched_mm_cid_remote_clear_old().
*/
smp_mb();
- t->last_mm_cid = t->mm_cid = mm_cid_get(rq, mm);
+ t->last_mm_cid = t->mm_cid = mm_cid_get(rq, t, mm);
}
rseq_set_notify_resume(t);
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 4c36cc680361..d2fa10d39b46 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3403,24 +3403,40 @@ static inline void mm_cid_put(struct mm_struct *mm)
__mm_cid_put(mm, mm_cid_clear_lazy_put(cid));
}
-static inline int __mm_cid_try_get(struct mm_struct *mm)
+static inline int __mm_cid_try_get(struct task_struct *t, struct mm_struct *mm)
{
- struct cpumask *cpumask;
- int cid;
+ struct cpumask *cidmask = mm_cidmask(mm);
+ struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
+ int cid = __this_cpu_read(pcpu_cid->recent_cid);
- cpumask = mm_cidmask(mm);
+ /* Try to re-use recent cid. This improves cache locality. */
+ if (!mm_cid_is_unset(cid) && !cpumask_test_and_set_cpu(cid, cidmask))
+ return cid;
+ /*
+ * Expand cid allocation if used cids are below the number cpus
+ * allowed and number of threads. Expanding cid allocation as
+ * much as possible improves cache locality.
+ */
+ cid = atomic_read(&mm->nr_cids_used);
+ while (cid < atomic_read(&mm->nr_cpus_allowed) && cid < atomic_read(&mm->mm_users)) {
+ if (!atomic_try_cmpxchg(&mm->nr_cids_used, &cid, cid + 1))
+ continue;
+ if (!cpumask_test_and_set_cpu(cid, cidmask))
+ return cid;
+ }
/*
+ * Find the first available concurrency id.
* Retry finding first zero bit if the mask is temporarily
* filled. This only happens during concurrent remote-clear
* which owns a cid without holding a rq lock.
*/
for (;;) {
- cid = cpumask_first_zero(cpumask);
- if (cid < nr_cpu_ids)
+ cid = cpumask_first_zero(cidmask);
+ if (cid < atomic_read(&mm->nr_cpus_allowed))
break;
cpu_relax();
}
- if (cpumask_test_and_set_cpu(cid, cpumask))
+ if (cpumask_test_and_set_cpu(cid, cidmask))
return -1;
return cid;
@@ -3438,7 +3454,8 @@ static inline void mm_cid_snapshot_time(struct rq *rq, struct mm_struct *mm)
WRITE_ONCE(pcpu_cid->time, rq->clock);
}
-static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
+static inline int __mm_cid_get(struct rq *rq, struct task_struct *t,
+ struct mm_struct *mm)
{
int cid;
@@ -3448,13 +3465,13 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
* guarantee forward progress.
*/
if (!READ_ONCE(use_cid_lock)) {
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
if (cid >= 0)
goto end;
raw_spin_lock(&cid_lock);
} else {
raw_spin_lock(&cid_lock);
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
if (cid >= 0)
goto unlock;
}
@@ -3474,7 +3491,7 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
* all newcoming allocations observe the use_cid_lock flag set.
*/
do {
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
cpu_relax();
} while (cid < 0);
/*
@@ -3491,7 +3508,8 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
return cid;
}
-static inline int mm_cid_get(struct rq *rq, struct mm_struct *mm)
+static inline int mm_cid_get(struct rq *rq, struct task_struct *t,
+ struct mm_struct *mm)
{
struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
struct cpumask *cpumask;
@@ -3508,8 +3526,9 @@ static inline int mm_cid_get(struct rq *rq, struct mm_struct *mm)
if (try_cmpxchg(&this_cpu_ptr(pcpu_cid)->cid, &cid, MM_CID_UNSET))
__mm_cid_put(mm, mm_cid_clear_lazy_put(cid));
}
- cid = __mm_cid_get(rq, mm);
+ cid = __mm_cid_get(rq, t, mm);
__this_cpu_write(pcpu_cid->cid, cid);
+ __this_cpu_write(pcpu_cid->recent_cid, cid);
return cid;
}
@@ -3562,7 +3581,7 @@ static inline void switch_mm_cid(struct rq *rq,
prev->mm_cid = -1;
}
if (next->mm_cid_active)
- next->last_mm_cid = next->mm_cid = mm_cid_get(rq, next->mm);
+ next->last_mm_cid = next->mm_cid = mm_cid_get(rq, next, next->mm);
}
#else /* !CONFIG_SCHED_MM_CID: */
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-09-30 18:59 ` [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads Mathieu Desnoyers
@ 2024-10-02 9:49 ` Marco Elver
2024-10-02 12:45 ` Mathieu Desnoyers
0 siblings, 1 reply; 11+ messages in thread
From: Marco Elver @ 2024-10-02 9:49 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Valentin Schneider,
Mel Gorman, Steven Rostedt, Vincent Guittot, Dietmar Eggemann,
Ben Segall, Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On Mon, 30 Sept 2024 at 21:01, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
> introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
> a reference to the concurrency id allocated for each CPU. This reference
> expires shortly after a 100ms delay.
>
> These per-CPU references keep the per-mm-cid data cache-local in
> situations where threads are running at least once on each CPU within
> each 100ms window, thus keeping the per-cpu reference alive.
>
> However, intermittent workloads behaving in bursts spaced by more than
> 100ms on each CPU exhibit bad cache locality and degraded performance
> compared to purely per-cpu data indexing, because concurrency IDs are
> allocated over various CPUs and cores, therefore losing cache locality
> of the associated data.
>
> Introduce the following changes to improve per-mm-cid cache locality:
>
> - Add a "recent_cid" field to the per-mm/cpu mm_cid structure to keep
> track of which mm_cid value was last used, and use it as a hint to
> attempt re-allocating the same concurrency ID the next time this
> mm/cpu needs to allocate a concurrency ID,
>
> - Add a per-mm CPUs allowed mask, which keeps track of the union of
> CPUs allowed for all threads belonging to this mm. This cpumask is
> only set during the lifetime of the mm, never cleared, so it
> represents the union of all the CPUs allowed since the beginning of
> the mm lifetime. (note that the mm_cpumask() is really arch-specific
> and tailored to the TLB flush needs, and is thus _not_ a viable
> approach for this)
>
> - Add a per-mm nr_cpus_allowed to keep track of the weight of the
> per-mm CPUs allowed mask (for fast access),
>
> - Add a per-mm nr_cids_used to keep track of the highest concurrency
> ID allocated for the mm. This is used for expanding the concurrency ID
> allocation within the upper bound defined by:
>
> min(mm->nr_cpus_allowed, mm->mm_users)
>
> When the next unused CID value reaches this threshold, stop trying
> to expand the cid allocation and use the first available cid value
> instead.
>
> Spreading allocation to use all the cid values within the range
>
> [ 0, min(mm->nr_cpus_allowed, mm->mm_users) - 1 ]
>
> improves cache locality while preserving mm_cid compactness within the
> expected user limits.
>
> - In __mm_cid_try_get, only return cid values within the range
> [ 0, mm->nr_cpus_allowed ] rather than [ 0, nr_cpu_ids ]. This
> prevents allocating cids above the number of allowed cpus in
> rare scenarios where cid allocation races with a concurrent
> remote-clear of the per-mm/cpu cid. This improvement is made
> possible by the addition of the per-mm CPUs allowed mask.
>
> - In sched_mm_cid_migrate_to, use mm->nr_cpus_allowed rather than
> t->nr_cpus_allowed. This criterion was really meant to compare
> the number of mm->mm_users to the number of CPUs allowed for the
> entire mm. Therefore, the prior comparison worked fine when all
> threads shared the same CPUs allowed mask, but not so much in
> scenarios where those threads have different masks (e.g. each
> thread pinned to a single CPU). This improvement is made
> possible by the addition of the per-mm CPUs allowed mask.
>
> * Benchmarks
>
> Each thread increments 16kB worth of 8-bit integers in bursts, with
> a configurable delay between each thread's execution. Each thread run
> one after the other (no threads run concurrently). The order of
> thread execution in the sequence is random. The thread execution
> sequence begins again after all threads have executed. The 16kB areas
> are allocated with rseq_mempool and indexed by either cpu_id, mm_cid
> (not cache-local), or cache-local mm_cid. Each thread is pinned to its
> own core.
>
> Testing configurations:
>
> 8-core/1-L3: Use 8 cores within a single L3
> 24-core/24-L3: Use 24 cores, 1 core per L3
> 192-core/24-L3: Use 192 cores (all cores in the system)
> 384-thread/24-L3: Use 384 HW threads (all HW threads in the system)
>
> Intermittent workload delays between threads: 200ms, 10ms.
>
> Hardware:
>
> CPU(s): 384
> On-line CPU(s) list: 0-383
> Vendor ID: AuthenticAMD
> Model name: AMD EPYC 9654 96-Core Processor
> Thread(s) per core: 2
> Core(s) per socket: 96
> Socket(s): 2
> Caches (sum of all):
> L1d: 6 MiB (192 instances)
> L1i: 6 MiB (192 instances)
> L2: 192 MiB (192 instances)
> L3: 768 MiB (24 instances)
>
> Each result is an average of 5 test runs. The cache-local speedup
> is calculated as: (cache-local mm_cid) / (mm_cid).
>
> Intermittent workload delay: 200ms
>
> per-cpu mm_cid cache-local mm_cid cache-local speedup
> (ns) (ns) (ns)
> 8-core/1-L3 1374 19289 1336 14.4x
> 24-core/24-L3 2423 26721 1594 16.7x
> 192-core/24-L3 2291 15826 2153 7.3x
> 384-thread/24-L3 1874 13234 1907 6.9x
>
> Intermittent workload delay: 10ms
>
> per-cpu mm_cid cache-local mm_cid cache-local speedup
> (ns) (ns) (ns)
> 8-core/1-L3 662 756 686 1.1x
> 24-core/24-L3 1378 3648 1035 3.5x
> 192-core/24-L3 1439 10833 1482 7.3x
> 384-thread/24-L3 1503 10570 1556 6.8x
>
> [ This deprecates the prior "sched: NUMA-aware per-memory-map concurrency IDs"
> patch series with a simpler and more general approach. ]
>
> Link: https://lore.kernel.org/lkml/20240823185946.418340-1-mathieu.desnoyers@efficios.com/
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Acked-by: Marco Elver <elver@google.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Valentin Schneider <vschneid@redhat.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Cc: Ben Segall <bsegall@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Marco Elver <elver@google.com>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> Changes since v0:
> - On migration, do not move the source cid to the destination cpu if the
> destination cpu has a recent cid value set.
>
> Changes since v2:
> - Rebase on v6.11.1.
I think the versioning and changelog got confused. I see the changes
from [1] which was already v2 are included in this one.
[1] https://lore.kernel.org/all/5cf2c0a5-7a99-4294-b316-eee07896ddf6@efficios.com/T/#u
In any case, I'll reiterate my Ack as this looks like an improvement
for the common case.
Acked-by: Marco Elver <elver@google.com>
Thanks,
-- Marco
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
2024-10-02 9:49 ` Marco Elver
@ 2024-10-02 12:45 ` Mathieu Desnoyers
0 siblings, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-10-02 12:45 UTC (permalink / raw)
To: Marco Elver, Peter Zijlstra
Cc: Ingo Molnar, linux-kernel, Valentin Schneider, Mel Gorman,
Steven Rostedt, Vincent Guittot, Dietmar Eggemann, Ben Segall,
Yury Norov, Rasmus Villemoes, Dmitry Vyukov
On 2024-10-02 11:49, Marco Elver wrote:
> On Mon, 30 Sept 2024 at 21:01, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
>> introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
>> a reference to the concurrency id allocated for each CPU. This reference
>> expires shortly after a 100ms delay.
>>
>> These per-CPU references keep the per-mm-cid data cache-local in
>> situations where threads are running at least once on each CPU within
>> each 100ms window, thus keeping the per-cpu reference alive.
>>
>> However, intermittent workloads behaving in bursts spaced by more than
>> 100ms on each CPU exhibit bad cache locality and degraded performance
>> compared to purely per-cpu data indexing, because concurrency IDs are
>> allocated over various CPUs and cores, therefore losing cache locality
>> of the associated data.
>>
>> Introduce the following changes to improve per-mm-cid cache locality:
>>
>> - Add a "recent_cid" field to the per-mm/cpu mm_cid structure to keep
>> track of which mm_cid value was last used, and use it as a hint to
>> attempt re-allocating the same concurrency ID the next time this
>> mm/cpu needs to allocate a concurrency ID,
>>
>> - Add a per-mm CPUs allowed mask, which keeps track of the union of
>> CPUs allowed for all threads belonging to this mm. This cpumask is
>> only set during the lifetime of the mm, never cleared, so it
>> represents the union of all the CPUs allowed since the beginning of
>> the mm lifetime. (note that the mm_cpumask() is really arch-specific
>> and tailored to the TLB flush needs, and is thus _not_ a viable
>> approach for this)
>>
>> - Add a per-mm nr_cpus_allowed to keep track of the weight of the
>> per-mm CPUs allowed mask (for fast access),
>>
>> - Add a per-mm nr_cids_used to keep track of the highest concurrency
>> ID allocated for the mm. This is used for expanding the concurrency ID
>> allocation within the upper bound defined by:
>>
>> min(mm->nr_cpus_allowed, mm->mm_users)
>>
>> When the next unused CID value reaches this threshold, stop trying
>> to expand the cid allocation and use the first available cid value
>> instead.
>>
>> Spreading allocation to use all the cid values within the range
>>
>> [ 0, min(mm->nr_cpus_allowed, mm->mm_users) - 1 ]
>>
>> improves cache locality while preserving mm_cid compactness within the
>> expected user limits.
>>
>> - In __mm_cid_try_get, only return cid values within the range
>> [ 0, mm->nr_cpus_allowed ] rather than [ 0, nr_cpu_ids ]. This
>> prevents allocating cids above the number of allowed cpus in
>> rare scenarios where cid allocation races with a concurrent
>> remote-clear of the per-mm/cpu cid. This improvement is made
>> possible by the addition of the per-mm CPUs allowed mask.
>>
>> - In sched_mm_cid_migrate_to, use mm->nr_cpus_allowed rather than
>> t->nr_cpus_allowed. This criterion was really meant to compare
>> the number of mm->mm_users to the number of CPUs allowed for the
>> entire mm. Therefore, the prior comparison worked fine when all
>> threads shared the same CPUs allowed mask, but not so much in
>> scenarios where those threads have different masks (e.g. each
>> thread pinned to a single CPU). This improvement is made
>> possible by the addition of the per-mm CPUs allowed mask.
>>
>> * Benchmarks
>>
>> Each thread increments 16kB worth of 8-bit integers in bursts, with
>> a configurable delay between each thread's execution. Each thread run
>> one after the other (no threads run concurrently). The order of
>> thread execution in the sequence is random. The thread execution
>> sequence begins again after all threads have executed. The 16kB areas
>> are allocated with rseq_mempool and indexed by either cpu_id, mm_cid
>> (not cache-local), or cache-local mm_cid. Each thread is pinned to its
>> own core.
>>
>> Testing configurations:
>>
>> 8-core/1-L3: Use 8 cores within a single L3
>> 24-core/24-L3: Use 24 cores, 1 core per L3
>> 192-core/24-L3: Use 192 cores (all cores in the system)
>> 384-thread/24-L3: Use 384 HW threads (all HW threads in the system)
>>
>> Intermittent workload delays between threads: 200ms, 10ms.
>>
>> Hardware:
>>
>> CPU(s): 384
>> On-line CPU(s) list: 0-383
>> Vendor ID: AuthenticAMD
>> Model name: AMD EPYC 9654 96-Core Processor
>> Thread(s) per core: 2
>> Core(s) per socket: 96
>> Socket(s): 2
>> Caches (sum of all):
>> L1d: 6 MiB (192 instances)
>> L1i: 6 MiB (192 instances)
>> L2: 192 MiB (192 instances)
>> L3: 768 MiB (24 instances)
>>
>> Each result is an average of 5 test runs. The cache-local speedup
>> is calculated as: (cache-local mm_cid) / (mm_cid).
>>
>> Intermittent workload delay: 200ms
>>
>> per-cpu mm_cid cache-local mm_cid cache-local speedup
>> (ns) (ns) (ns)
>> 8-core/1-L3 1374 19289 1336 14.4x
>> 24-core/24-L3 2423 26721 1594 16.7x
>> 192-core/24-L3 2291 15826 2153 7.3x
>> 384-thread/24-L3 1874 13234 1907 6.9x
>>
>> Intermittent workload delay: 10ms
>>
>> per-cpu mm_cid cache-local mm_cid cache-local speedup
>> (ns) (ns) (ns)
>> 8-core/1-L3 662 756 686 1.1x
>> 24-core/24-L3 1378 3648 1035 3.5x
>> 192-core/24-L3 1439 10833 1482 7.3x
>> 384-thread/24-L3 1503 10570 1556 6.8x
>>
>> [ This deprecates the prior "sched: NUMA-aware per-memory-map concurrency IDs"
>> patch series with a simpler and more general approach. ]
>>
>> Link: https://lore.kernel.org/lkml/20240823185946.418340-1-mathieu.desnoyers@efficios.com/
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Acked-by: Marco Elver <elver@google.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Valentin Schneider <vschneid@redhat.com>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Vincent Guittot <vincent.guittot@linaro.org>
>> Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
>> Cc: Ben Segall <bsegall@google.com>
>> Cc: Dmitry Vyukov <dvyukov@google.com>
>> Cc: Marco Elver <elver@google.com>
>> Cc: Yury Norov <yury.norov@gmail.com>
>> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>> Changes since v0:
>> - On migration, do not move the source cid to the destination cpu if the
>> destination cpu has a recent cid value set.
>>
>> Changes since v2:
>> - Rebase on v6.11.1.
>
> I think the versioning and changelog got confused. I see the changes
> from [1] which was already v2 are included in this one.
>
> [1] https://lore.kernel.org/all/5cf2c0a5-7a99-4294-b316-eee07896ddf6@efficios.com/T/#u
Which means I should have tagged this series [PATCH v3]. Sorry about
that.
>
> In any case, I'll reiterate my Ack as this looks like an improvement
> for the common case.
>
> Acked-by: Marco Elver <elver@google.com>
Thanks!
Peter, should I re-send as is with a v3 tag, or is it OK for merge ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads
@ 2024-10-09 13:50 Mathieu Desnoyers
0 siblings, 0 replies; 11+ messages in thread
From: Mathieu Desnoyers @ 2024-10-09 13:50 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Mathieu Desnoyers, Marco Elver, Ingo Molnar,
Paul E. McKenney, Boqun Feng, Valentin Schneider, Mel Gorman,
Steven Rostedt, Vincent Guittot, Dietmar Eggemann, Ben Segall,
Dmitry Vyukov, Yury Norov, Rasmus Villemoes
commit 223baf9d17f25 ("sched: Fix performance regression introduced by mm_cid")
introduced a per-mm/cpu current concurrency id (mm_cid), which keeps
a reference to the concurrency id allocated for each CPU. This reference
expires shortly after a 100ms delay.
These per-CPU references keep the per-mm-cid data cache-local in
situations where threads are running at least once on each CPU within
each 100ms window, thus keeping the per-cpu reference alive.
However, intermittent workloads behaving in bursts spaced by more than
100ms on each CPU exhibit bad cache locality and degraded performance
compared to purely per-cpu data indexing, because concurrency IDs are
allocated over various CPUs and cores, therefore losing cache locality
of the associated data.
Introduce the following changes to improve per-mm-cid cache locality:
- Add a "recent_cid" field to the per-mm/cpu mm_cid structure to keep
track of which mm_cid value was last used, and use it as a hint to
attempt re-allocating the same concurrency ID the next time this
mm/cpu needs to allocate a concurrency ID,
- Add a per-mm CPUs allowed mask, which keeps track of the union of
CPUs allowed for all threads belonging to this mm. This cpumask is
only set during the lifetime of the mm, never cleared, so it
represents the union of all the CPUs allowed since the beginning of
the mm lifetime (note that the mm_cpumask() is really arch-specific
and tailored to the TLB flush needs, and is thus _not_ a viable
approach for this),
- Add a per-mm nr_cpus_allowed to keep track of the weight of the
per-mm CPUs allowed mask (for fast access),
- Add a per-mm max_nr_cid to keep track of the highest number of
concurrency IDs allocated for the mm. This is used for expanding the
concurrency ID allocation within the upper bound defined by:
min(mm->nr_cpus_allowed, mm->mm_users)
When the next unused CID value reaches this threshold, stop trying
to expand the cid allocation and use the first available cid value
instead.
Spreading allocation to use all the cid values within the range
[ 0, min(mm->nr_cpus_allowed, mm->mm_users) - 1 ]
improves cache locality while preserving mm_cid compactness within the
expected user limits,
- In __mm_cid_try_get, only return cid values within the range
[ 0, mm->nr_cpus_allowed ] rather than [ 0, nr_cpu_ids ]. This
prevents allocating cids above the number of allowed cpus in
rare scenarios where cid allocation races with a concurrent
remote-clear of the per-mm/cpu cid. This improvement is made
possible by the addition of the per-mm CPUs allowed mask,
- In sched_mm_cid_migrate_to, use mm->nr_cpus_allowed rather than
t->nr_cpus_allowed. This criterion was really meant to compare
the number of mm->mm_users to the number of CPUs allowed for the
entire mm. Therefore, the prior comparison worked fine when all
threads shared the same CPUs allowed mask, but not so much in
scenarios where those threads have different masks (e.g. each
thread pinned to a single CPU). This improvement is made
possible by the addition of the per-mm CPUs allowed mask.
* Benchmarks
Each thread increments 16kB worth of 8-bit integers in bursts, with
a configurable delay between each thread's execution. Each thread run
one after the other (no threads run concurrently). The order of
thread execution in the sequence is random. The thread execution
sequence begins again after all threads have executed. The 16kB areas
are allocated with rseq_mempool and indexed by either cpu_id, mm_cid
(not cache-local), or cache-local mm_cid. Each thread is pinned to its
own core.
Testing configurations:
8-core/1-L3: Use 8 cores within a single L3
24-core/24-L3: Use 24 cores, 1 core per L3
192-core/24-L3: Use 192 cores (all cores in the system)
384-thread/24-L3: Use 384 HW threads (all HW threads in the system)
Intermittent workload delays between threads: 200ms, 10ms.
Hardware:
CPU(s): 384
On-line CPU(s) list: 0-383
Vendor ID: AuthenticAMD
Model name: AMD EPYC 9654 96-Core Processor
Thread(s) per core: 2
Core(s) per socket: 96
Socket(s): 2
Caches (sum of all):
L1d: 6 MiB (192 instances)
L1i: 6 MiB (192 instances)
L2: 192 MiB (192 instances)
L3: 768 MiB (24 instances)
Each result is an average of 5 test runs. The cache-local speedup
is calculated as: (cache-local mm_cid) / (mm_cid).
Intermittent workload delay: 200ms
per-cpu mm_cid cache-local mm_cid cache-local speedup
(ns) (ns) (ns)
8-core/1-L3 1374 19289 1336 14.4x
24-core/24-L3 2423 26721 1594 16.7x
192-core/24-L3 2291 15826 2153 7.3x
384-thread/24-L3 1874 13234 1907 6.9x
Intermittent workload delay: 10ms
per-cpu mm_cid cache-local mm_cid cache-local speedup
(ns) (ns) (ns)
8-core/1-L3 662 756 686 1.1x
24-core/24-L3 1378 3648 1035 3.5x
192-core/24-L3 1439 10833 1482 7.3x
384-thread/24-L3 1503 10570 1556 6.8x
[ This deprecates the prior "sched: NUMA-aware per-memory-map concurrency IDs"
patch series with a simpler and more general approach. ]
[ This patch applies on top of v6.12-rc1. ]
Link: https://lore.kernel.org/lkml/20240823185946.418340-1-mathieu.desnoyers@efficios.com/
Acked-by: Marco Elver <elver@google.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Valentin Schneider <vschneid@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Marco Elver <elver@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
Changes since v1:
- Take care of feedback from Peter Zijlstra.
---
fs/exec.c | 2 +-
include/linux/mm_types.h | 72 +++++++++++++++++++++++++++++++++++-----
kernel/fork.c | 2 +-
kernel/sched/core.c | 22 +++++++-----
kernel/sched/sched.h | 48 +++++++++++++++++++--------
5 files changed, 112 insertions(+), 34 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 6c53920795c2..aaa605529a75 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -990,7 +990,7 @@ static int exec_mmap(struct mm_struct *mm)
active_mm = tsk->active_mm;
tsk->active_mm = mm;
tsk->mm = mm;
- mm_init_cid(mm);
+ mm_init_cid(mm, tsk);
/*
* This prevents preemption while active_mm is being loaded and
* it and mm are being updated, which could cause problems for
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 6e3bdf8e38bc..381d22eba088 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -782,6 +782,7 @@ struct vm_area_struct {
struct mm_cid {
u64 time;
int cid;
+ int recent_cid;
};
#endif
@@ -852,6 +853,27 @@ struct mm_struct {
* When the next mm_cid scan is due (in jiffies).
*/
unsigned long mm_cid_next_scan;
+ /**
+ * @nr_cpus_allowed: Number of CPUs allowed for mm.
+ *
+ * Number of CPUs allowed in the union of all mm's
+ * threads allowed CPUs.
+ */
+ unsigned int nr_cpus_allowed;
+ /**
+ * @max_nr_cid: Maximum number of concurrency IDs allocated.
+ *
+ * Track the highest number of concurrency IDs allocated for the
+ * mm.
+ */
+ atomic_t max_nr_cid;
+ /**
+ * @cpus_allowed_lock: Lock protecting mm cpus_allowed.
+ *
+ * Provide mutual exclusion for mm cpus_allowed and
+ * mm nr_cpus_allowed updates.
+ */
+ raw_spinlock_t cpus_allowed_lock;
#endif
#ifdef CONFIG_MMU
atomic_long_t pgtables_bytes; /* size of all page tables */
@@ -1170,18 +1192,30 @@ static inline int mm_cid_clear_lazy_put(int cid)
return cid & ~MM_CID_LAZY_PUT;
}
+/*
+ * mm_cpus_allowed: Union of all mm's threads allowed CPUs.
+ */
+static inline cpumask_t *mm_cpus_allowed(struct mm_struct *mm)
+{
+ unsigned long bitmap = (unsigned long)mm;
+
+ bitmap += offsetof(struct mm_struct, cpu_bitmap);
+ /* Skip cpu_bitmap */
+ bitmap += cpumask_size();
+ return (struct cpumask *)bitmap;
+}
+
/* Accessor for struct mm_struct's cidmask. */
static inline cpumask_t *mm_cidmask(struct mm_struct *mm)
{
- unsigned long cid_bitmap = (unsigned long)mm;
+ unsigned long cid_bitmap = (unsigned long)mm_cpus_allowed(mm);
- cid_bitmap += offsetof(struct mm_struct, cpu_bitmap);
- /* Skip cpu_bitmap */
+ /* Skip mm_cpus_allowed */
cid_bitmap += cpumask_size();
return (struct cpumask *)cid_bitmap;
}
-static inline void mm_init_cid(struct mm_struct *mm)
+static inline void mm_init_cid(struct mm_struct *mm, struct task_struct *p)
{
int i;
@@ -1189,17 +1223,22 @@ static inline void mm_init_cid(struct mm_struct *mm)
struct mm_cid *pcpu_cid = per_cpu_ptr(mm->pcpu_cid, i);
pcpu_cid->cid = MM_CID_UNSET;
+ pcpu_cid->recent_cid = MM_CID_UNSET;
pcpu_cid->time = 0;
}
+ mm->nr_cpus_allowed = p->nr_cpus_allowed;
+ atomic_set(&mm->max_nr_cid, 0);
+ raw_spin_lock_init(&mm->cpus_allowed_lock);
+ cpumask_copy(mm_cpus_allowed(mm), &p->cpus_mask);
cpumask_clear(mm_cidmask(mm));
}
-static inline int mm_alloc_cid_noprof(struct mm_struct *mm)
+static inline int mm_alloc_cid_noprof(struct mm_struct *mm, struct task_struct *p)
{
mm->pcpu_cid = alloc_percpu_noprof(struct mm_cid);
if (!mm->pcpu_cid)
return -ENOMEM;
- mm_init_cid(mm);
+ mm_init_cid(mm, p);
return 0;
}
#define mm_alloc_cid(...) alloc_hooks(mm_alloc_cid_noprof(__VA_ARGS__))
@@ -1212,16 +1251,31 @@ static inline void mm_destroy_cid(struct mm_struct *mm)
static inline unsigned int mm_cid_size(void)
{
- return cpumask_size();
+ return 2 * cpumask_size(); /* mm_cpus_allowed(), mm_cidmask(). */
+}
+
+static inline void mm_set_cpus_allowed(struct mm_struct *mm, const struct cpumask *cpumask)
+{
+ struct cpumask *mm_allowed = mm_cpus_allowed(mm);
+
+ if (!mm)
+ return;
+ /* The mm_cpus_allowed is the union of each thread allowed CPUs masks. */
+ raw_spin_lock(&mm->cpus_allowed_lock);
+ cpumask_or(mm_allowed, mm_allowed, cpumask);
+ WRITE_ONCE(mm->nr_cpus_allowed, cpumask_weight(mm_allowed));
+ raw_spin_unlock(&mm->cpus_allowed_lock);
}
#else /* CONFIG_SCHED_MM_CID */
-static inline void mm_init_cid(struct mm_struct *mm) { }
-static inline int mm_alloc_cid(struct mm_struct *mm) { return 0; }
+static inline void mm_init_cid(struct mm_struct *mm, struct task_struct *p) { }
+static inline int mm_alloc_cid(struct mm_struct *mm, struct task_struct *p) { return 0; }
static inline void mm_destroy_cid(struct mm_struct *mm) { }
+
static inline unsigned int mm_cid_size(void)
{
return 0;
}
+static inline void mm_set_cpus_allowed(struct mm_struct *mm, const struct cpumask *cpumask) { }
#endif /* CONFIG_SCHED_MM_CID */
struct mmu_gather;
diff --git a/kernel/fork.c b/kernel/fork.c
index 60c0b4868fd4..18bf37ae73a5 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1298,7 +1298,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
if (init_new_context(p, mm))
goto fail_nocontext;
- if (mm_alloc_cid(mm))
+ if (mm_alloc_cid(mm, p))
goto fail_cid;
if (percpu_counter_init_many(mm->rss_stat, 0, GFP_KERNEL_ACCOUNT,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 43e453ab7e20..6133240abffa 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2691,6 +2691,7 @@ __do_set_cpus_allowed(struct task_struct *p, struct affinity_context *ctx)
put_prev_task(rq, p);
p->sched_class->set_cpus_allowed(p, ctx);
+ mm_set_cpus_allowed(p->mm, ctx->new_mask);
if (queued)
enqueue_task(rq, p, ENQUEUE_RESTORE | ENQUEUE_NOCLOCK);
@@ -10228,6 +10229,7 @@ int __sched_mm_cid_migrate_from_try_steal_cid(struct rq *src_rq,
*/
if (!try_cmpxchg(&src_pcpu_cid->cid, &lazy_cid, MM_CID_UNSET))
return -1;
+ WRITE_ONCE(src_pcpu_cid->recent_cid, MM_CID_UNSET);
return src_cid;
}
@@ -10240,7 +10242,8 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
{
struct mm_cid *src_pcpu_cid, *dst_pcpu_cid;
struct mm_struct *mm = t->mm;
- int src_cid, dst_cid, src_cpu;
+ int src_cid, src_cpu;
+ bool dst_cid_is_set;
struct rq *src_rq;
lockdep_assert_rq_held(dst_rq);
@@ -10257,9 +10260,9 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
* allocation closest to 0 in cases where few threads migrate around
* many CPUs.
*
- * If destination cid is already set, we may have to just clear
- * the src cid to ensure compactness in frequent migrations
- * scenarios.
+ * If destination cid or recent cid is already set, we may have
+ * to just clear the src cid to ensure compactness in frequent
+ * migrations scenarios.
*
* It is not useful to clear the src cid when the number of threads is
* greater or equal to the number of allowed CPUs, because user-space
@@ -10267,9 +10270,9 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
* allowed CPUs.
*/
dst_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, cpu_of(dst_rq));
- dst_cid = READ_ONCE(dst_pcpu_cid->cid);
- if (!mm_cid_is_unset(dst_cid) &&
- atomic_read(&mm->mm_users) >= t->nr_cpus_allowed)
+ dst_cid_is_set = !mm_cid_is_unset(READ_ONCE(dst_pcpu_cid->cid)) ||
+ !mm_cid_is_unset(READ_ONCE(dst_pcpu_cid->recent_cid));
+ if (dst_cid_is_set && atomic_read(&mm->mm_users) >= READ_ONCE(mm->nr_cpus_allowed))
return;
src_pcpu_cid = per_cpu_ptr(mm->pcpu_cid, src_cpu);
src_rq = cpu_rq(src_cpu);
@@ -10280,13 +10283,14 @@ void sched_mm_cid_migrate_to(struct rq *dst_rq, struct task_struct *t)
src_cid);
if (src_cid == -1)
return;
- if (!mm_cid_is_unset(dst_cid)) {
+ if (dst_cid_is_set) {
__mm_cid_put(mm, src_cid);
return;
}
/* Move src_cid to dst cpu. */
mm_cid_snapshot_time(dst_rq, mm);
WRITE_ONCE(dst_pcpu_cid->cid, src_cid);
+ WRITE_ONCE(dst_pcpu_cid->recent_cid, src_cid);
}
static void sched_mm_cid_remote_clear(struct mm_struct *mm, struct mm_cid *pcpu_cid,
@@ -10523,7 +10527,7 @@ void sched_mm_cid_after_execve(struct task_struct *t)
* Matches barrier in sched_mm_cid_remote_clear_old().
*/
smp_mb();
- t->last_mm_cid = t->mm_cid = mm_cid_get(rq, mm);
+ t->last_mm_cid = t->mm_cid = mm_cid_get(rq, t, mm);
}
rseq_set_notify_resume(t);
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index b1c3588a8f00..8e23f1299081 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3596,24 +3596,41 @@ static inline void mm_cid_put(struct mm_struct *mm)
__mm_cid_put(mm, mm_cid_clear_lazy_put(cid));
}
-static inline int __mm_cid_try_get(struct mm_struct *mm)
+static inline int __mm_cid_try_get(struct task_struct *t, struct mm_struct *mm)
{
- struct cpumask *cpumask;
- int cid;
+ struct cpumask *cidmask = mm_cidmask(mm);
+ struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
+ int cid = __this_cpu_read(pcpu_cid->recent_cid);
- cpumask = mm_cidmask(mm);
+ /* Try to re-use recent cid. This improves cache locality. */
+ if (!mm_cid_is_unset(cid) && !cpumask_test_and_set_cpu(cid, cidmask))
+ return cid;
+ /*
+ * Expand cid allocation if the maximum number of concurrency
+ * IDs allocated (max_nr_cid) is below the number cpus allowed
+ * and number of threads. Expanding cid allocation as much as
+ * possible improves cache locality.
+ */
+ cid = atomic_read(&mm->max_nr_cid);
+ while (cid < READ_ONCE(mm->nr_cpus_allowed) && cid < atomic_read(&mm->mm_users)) {
+ if (!atomic_try_cmpxchg(&mm->max_nr_cid, &cid, cid + 1))
+ continue;
+ if (!cpumask_test_and_set_cpu(cid, cidmask))
+ return cid;
+ }
/*
+ * Find the first available concurrency id.
* Retry finding first zero bit if the mask is temporarily
* filled. This only happens during concurrent remote-clear
* which owns a cid without holding a rq lock.
*/
for (;;) {
- cid = cpumask_first_zero(cpumask);
- if (cid < nr_cpu_ids)
+ cid = cpumask_first_zero(cidmask);
+ if (cid < READ_ONCE(mm->nr_cpus_allowed))
break;
cpu_relax();
}
- if (cpumask_test_and_set_cpu(cid, cpumask))
+ if (cpumask_test_and_set_cpu(cid, cidmask))
return -1;
return cid;
@@ -3631,7 +3648,8 @@ static inline void mm_cid_snapshot_time(struct rq *rq, struct mm_struct *mm)
WRITE_ONCE(pcpu_cid->time, rq->clock);
}
-static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
+static inline int __mm_cid_get(struct rq *rq, struct task_struct *t,
+ struct mm_struct *mm)
{
int cid;
@@ -3641,13 +3659,13 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
* guarantee forward progress.
*/
if (!READ_ONCE(use_cid_lock)) {
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
if (cid >= 0)
goto end;
raw_spin_lock(&cid_lock);
} else {
raw_spin_lock(&cid_lock);
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
if (cid >= 0)
goto unlock;
}
@@ -3667,7 +3685,7 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
* all newcoming allocations observe the use_cid_lock flag set.
*/
do {
- cid = __mm_cid_try_get(mm);
+ cid = __mm_cid_try_get(t, mm);
cpu_relax();
} while (cid < 0);
/*
@@ -3684,7 +3702,8 @@ static inline int __mm_cid_get(struct rq *rq, struct mm_struct *mm)
return cid;
}
-static inline int mm_cid_get(struct rq *rq, struct mm_struct *mm)
+static inline int mm_cid_get(struct rq *rq, struct task_struct *t,
+ struct mm_struct *mm)
{
struct mm_cid __percpu *pcpu_cid = mm->pcpu_cid;
struct cpumask *cpumask;
@@ -3701,8 +3720,9 @@ static inline int mm_cid_get(struct rq *rq, struct mm_struct *mm)
if (try_cmpxchg(&this_cpu_ptr(pcpu_cid)->cid, &cid, MM_CID_UNSET))
__mm_cid_put(mm, mm_cid_clear_lazy_put(cid));
}
- cid = __mm_cid_get(rq, mm);
+ cid = __mm_cid_get(rq, t, mm);
__this_cpu_write(pcpu_cid->cid, cid);
+ __this_cpu_write(pcpu_cid->recent_cid, cid);
return cid;
}
@@ -3755,7 +3775,7 @@ static inline void switch_mm_cid(struct rq *rq,
prev->mm_cid = -1;
}
if (next->mm_cid_active)
- next->last_mm_cid = next->mm_cid = mm_cid_get(rq, next->mm);
+ next->last_mm_cid = next->mm_cid = mm_cid_get(rq, next, next->mm);
}
#else /* !CONFIG_SCHED_MM_CID: */
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-10-09 13:52 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-09 21:15 [PATCH v2 0/1] sched: Improve cache locality of RSEQ concurrency IDs Mathieu Desnoyers
2024-09-09 21:15 ` [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads Mathieu Desnoyers
2024-09-12 16:38 ` Marco Elver
2024-09-12 5:33 ` Mathieu Desnoyers
2024-09-13 12:09 ` Marco Elver
2024-09-15 10:11 ` Mathieu Desnoyers
2024-09-19 6:00 ` Marco Elver
2024-09-30 18:59 [PATCH v2 0/1] sched: Improve cache locality of RSEQ concurrency IDs Mathieu Desnoyers
2024-09-30 18:59 ` [PATCH v2 1/1] sched: Improve cache locality of RSEQ concurrency IDs for intermittent workloads Mathieu Desnoyers
2024-10-02 9:49 ` Marco Elver
2024-10-02 12:45 ` Mathieu Desnoyers
2024-10-09 13:50 Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®