Re: [PATCH] mm: memcg: use ratelimited stats flush in obj_cgroup_may_zswap()

[Chengfeng Lin <lin2530632123@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4335 bytes --]

Hi Joanne, Yosry, and all,

With existing rstat updates retained, hierarchical per-CPU counters reduced
single-worker PAGEOUT CPU by 44.69% and whole-program CPU by 21.78% in this
bare-metal test.

One worker uses MADV_PAGEOUT on 32 MiB of compressible anonymous memory in
a private cgroup, with THP disabled and memory.zswap.max=64 MiB. The limit
is not reached. The host is an i7-12700KF with 20 online logical CPUs,
full preemption, LZO and zsmalloc. Unlimited controls change only the
zswap limit to max.

A matched parent/commit/parent test of 0efc297a3c49
("cgroup/rstat: avoid disabling irqs for O(num_cpu)") showed:

  CPU time, ms / 32 MiB         Parent A   Commit    Parent B
  PAGEOUT                       17.180     24.464    17.316
  Whole test program            46.215     53.419    46.235

PAGEOUT increased by 7.216 ms (41.84%) and whole-program CPU by 7.193 ms
(15.56%). Both versions stored 8192 pages and performed 8192 limit-check
flushes: a finite zswap limit forces a stats flush before each store.

To limit interrupt-disabled time, the commit
releases and reacquires the rstat lock after processing each possible CPU,
with cond_resched() in between and cpu_relax() if it does not reschedule.
Previously this was conditional on rescheduling or lock contention.
Per-page flushing repeats these extra operations.

On MM baseline 2967f3ccdb5f, I adapted Joanne's hierarchical per-CPU option
[1]. The limit pre-check uses dedicated counters, retaining MEMCG_ZSWAP_B
rstat updates and the IRQ-latency protection. Extra updates are included:

  CPU time, ms / 32 MiB         Base A     Counter   Base B
  PAGEOUT                       23.903     13.113    23.514
  Whole test program            49.247     38.569    49.363

PAGEOUT CPU fell by 10.595 ms (44.69%) and whole-program CPU by 10.737 ms
(21.78%). Stores were unchanged; forced flushes fell from 8192 to zero.
These two comparisons use different baselines.

Both comparisons used fresh A/B/A boots, two warmups and nine clean
samples per condition per boot. PAGEOUT return drift was 0.80%/-1.63%,
with maximum within-boot CV of 2.82%/3.55%. PAGEOUT measures worker CPU;
whole-program CPU includes preparation, re-access, verification
and cleanup, but not external cgroup setup or host-wide background work.

A single-worker unlimited control changed PAGEOUT/whole-program CPU by
+0.41%/+0.86%, with PAGEOUT CV up to 4.74%; there was no clear difference.

A supplementary A/B/A test compared the MM baseline with the counter
derivative, with eight workers pinned to different P cores, each processing
32 MiB (256 MiB total). Their shared cgroup had memory.max=2 GiB and
memory.swap.max=1 GiB.
The zswap limit remained 64 MiB; compressed usage was about 18.75 MiB,
below the limit. Counter-kernel means follow, with baseline-relative changes:

  Eight-worker metric, per 256 MiB   Finite limit          Unlimited
  Summed PAGEOUT CPU                199.238 ms (-84.04%)  196.272 ms (+1.17%)
  Summed re-access/verify CPU        215.365 ms ( -1.36%)  214.950 ms (+0.09%)
  Whole-program CPU                 663.404 ms (-61.39%)  660.535 ms (+0.24%)
  Whole-program elapsed time         94.618 ms (-58.26%)   95.183 ms (+0.99%)

Both kernels stored 65,536 pages. Finite-limit forced flushes fell from
65,536 to zero; unlimited controls performed none. Counter-kernel PAGEOUT
CVs were 4.51%/4.91%, with baseline return drift of +5.10%/+0.97%.
All nine finite-limit counter samples were below both baseline groups,
but baseline drift limits the precision of the gain estimate.
The small unlimited increase does not
establish a regression. Re-access and elapsed-time CVs were at most 2.81%.

Elapsed time includes preparation and cleanup; it is not summed CPU time.
The re-access metric includes reading and verifying the data; zswap
decompression was not timed separately.

Both kernels passed bounded leaf/ancestor-limit, dynamic-limit,
cross-CPU-freeing and cgroup-offlining checks. Deep cgroup hierarchies, CPU
hotplug, and 32-bit systems were not tested.

I hope these results help assess the benefit and update cost of dedicated
counters while retaining the existing statistics. The reproducer sources and
the measured counter patch are attached.

[1] https://github.com/joannekoong/linux/commit/9092e0057f038ffca43ae3f35faf58c33b1d7c9b

Thanks,
Chengfeng

[-- Attachment #2: zswap-reproducer.tar.gz --]
[-- Type: application/gzip, Size: 7170 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: hierarchical-with-rstat.patch --]
[-- Type: text/x-patch, Size: 2579 bytes --]

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 4d00748..13e705b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -674,6 +674,11 @@ struct memcg_vmstats_percpu {
 	struct memcg_vmstats_percpu __percpu	*parent_pcpu;
 	struct memcg_vmstats			*vmstats;
 
+#ifdef CONFIG_ZSWAP
+	/* Hierarchical compressed bytes, updated eagerly and summed on read. */
+	long			zswap_hier;
+#endif
+
 	/* The above should fit a single cacheline for memcg_rstat_updated() */
 
 	/* Local (CPU and cgroup) page state & events */
@@ -6212,6 +6217,35 @@ static struct cftype swap_files[] = {
 };
 
 #ifdef CONFIG_ZSWAP
+/*
+ * Evaluation of Joanne Koong's hierarchical per-CPU experiment (9092e0057f03).
+ * Keep the existing rstat updates: public usage and shrinker accounting still
+ * consume them. Only the limit pre-check uses this additional counter.
+ * Callers hold rcu_read_lock() and use the same memcg as their rstat update.
+ */
+static void memcg_zswap_hier_add(struct mem_cgroup *memcg, long val)
+{
+	struct memcg_vmstats_percpu __percpu *statc_pcpu;
+	struct memcg_vmstats_percpu *statc;
+
+	statc_pcpu = memcg->vmstats_percpu;
+	for (; statc_pcpu; statc_pcpu = statc->parent_pcpu) {
+		statc = this_cpu_ptr(statc_pcpu);
+		this_cpu_add(statc_pcpu->zswap_hier, val);
+	}
+}
+
+static unsigned long memcg_zswap_hier_sum(struct mem_cgroup *memcg)
+{
+	long sum = 0;
+	int cpu;
+
+	for_each_possible_cpu(cpu)
+		sum += READ_ONCE(per_cpu_ptr(memcg->vmstats_percpu,
+					     cpu)->zswap_hier);
+	return max(sum, 0L);
+}
+
 /**
  * obj_cgroup_may_zswap - check if this cgroup can zswap
  * @objcg: the object cgroup
@@ -6245,9 +6279,7 @@ bool obj_cgroup_may_zswap(struct obj_cgroup *objcg)
 			break;
 		}
 
-		/* Force flush to get accurate stats for charging */
-		__mem_cgroup_flush_stats(memcg, true);
-		pages = memcg_page_state(memcg, MEMCG_ZSWAP_B) / PAGE_SIZE;
+		pages = memcg_zswap_hier_sum(memcg) / PAGE_SIZE;
 		if (pages < max)
 			continue;
 		ret = false;
@@ -6283,6 +6315,7 @@ void obj_cgroup_charge_zswap(struct obj_cgroup *objcg, size_t size)
 
 	rcu_read_lock();
 	memcg = obj_cgroup_memcg(objcg);
+	memcg_zswap_hier_add(memcg, size);
 	mod_memcg_state(memcg, MEMCG_ZSWAP_B, size);
 	mod_memcg_state(memcg, MEMCG_ZSWAPPED, 1);
 	if (size == PAGE_SIZE)
@@ -6311,6 +6344,7 @@ void obj_cgroup_uncharge_zswap(struct obj_cgroup *objcg, size_t size)
 
 	rcu_read_lock();
 	memcg = obj_cgroup_memcg(objcg);
+	memcg_zswap_hier_add(memcg, -(long)size);
 	mod_memcg_state(memcg, MEMCG_ZSWAP_B, -size);
 	mod_memcg_state(memcg, MEMCG_ZSWAPPED, -1);
 	if (size == PAGE_SIZE)