Re: [patch V3 03/20] sched/mmcid: Cacheline align MM CID storage
From: Mathieu Desnoyers
Date: Thu Oct 30 2025 - 10:15:08 EST
On 2025-10-29 17:09, Thomas Gleixner wrote:
On Wed, Oct 29 2025 at 11:39, Mathieu Desnoyers wrote:I did notice false sharing in the past within the mm_struct between
On 2025-10-29 09:08, Thomas Gleixner wrote:
[...]
struct mm_cid_pcpu {
unsigned int cid;
-};
+}____cacheline_aligned_in_smp;
What's the point in cacheline aligning this per-CPU variable ?
Should we expect other accesses to per-CPU variables sharing the
same cache line to update them frequently from remote CPUs ?
I did not cacheline align it expecting that per-CPU variables are
typically updated from their respective CPUs. So perhaps reality
don't match my expectations, but that's news to me.
It depends. While per CPU variables are typically updated only on the
local CPU there are situations where there is cross CPU access and it
really depends in which proximity it ends up. I made it that way because
I saw the accesses prominent in perf top, which means there is
contention on the cache line.
the mm_count field and the mm_cid percpu _pointer load_:
commit c1753fd02a00 ("mm: move mm_count into its own cache line")
Before understanding that this was actually the pointer load
that was false-sharing with mm_count, I initially thought that the
per-cpu memory somehow had false sharing because I was reading
the perf profiles incorrectly.
I just want to make sure that what you have identified in the perf
profiles is indeed false sharing of the per-cpu memory and not false
sharing of the per-cpu pointer load. Otherwise we'd been adding entirely
useless padding to percpu structures.
Note that in the current layout, atomic_t mm_users is right besides
the pcpu_cid percpu pointer, which may cause false sharing if mm_users
is updated often. But if that's indeed the culprit, then just adding
the cacheline align on the new struct mm_mm_cid suffices.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com