[PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn]

v6 --> v7
=========
- Preserve two cgroup v1 proprerties that previously followed implicitly
  from sharing a stock:
  - memory.usage_in_bytes <= memory.memsw.usage_in_bytes
  - Reclaim does not swap when memory and memsw have equal limits.
- Wordsmithing commit messages for 4/5 and 5/5 to document the
  cgroupv1 divergence (bounded) between memory and memsw caused by
  independent stock.

INTRODUCTION
============
Memcg keeps a per-CPU stock of precharged pages so that small, frequent
allocations do not walk the page_counter hierarchy every time.
Today, the stock implementation is within memcontrol code, even though
the operation it caches is a page_counter charge. This makes it
difficult to add new page_counters to a memcg and preserve the fast
path behavior.

This matters for future work like my tiered memcg limits series [1]
which introduces multiple new page_counters to memcg. Without making
stock a page_counter-level property, it means that every memcg charge
now goes through multiple page_counter hierarchy walks, instead of
being able to cache these charges.

To make future page_counters scalable and performant, move stock from
mem_cgroup to page_counter so that each page_counter can opt into its
own per-CPU cache of pre-charged pages.

We get an added benefit of simplifying try_charge_memcg(), which now
has all the stock management handled transparently within the
page_counter layer.

EFFECT ON MEMCG V2 USERS
========================
This series has no functional changes intended for memcg v2 users. We
preserve all draining, refilling, and (un)charging behavior, including
the uncharge path's refills / direct uncharges.

EFFECT ON MEMCG V1 USERS
========================
For memcg v1 users, the decoupling of the memsw and memory stock means
that each of them now manage their own independent stocks and can lead
to a different size of precharged cache for each.

Cgroup v1 has an invariant that memory.memsw.usage_in_bytes is larger
than or equal to memory.usage_in_bytes, because memsw is a superset of
memory. With separate stocking, this could have been broken in scenarios
where the memory stock is bigger than the memsw stock, leading to
memory usage appearing to be inflated and greater than memsw usage,
even though the real usage preserves the invariant.

Preserve the user-visible invariant by reporting memory.usage_in_bytes
as the smaller of the raw memory and memsw values. Both counters are
upper bounds on charged memory, so this remains a bounded stock-related
overestimate and importantly does not affect limit enforcement.

Based on latest mm-new as of 10/21/26:
fcec4f4bcc110 "riscv: mm: fix concurrency in mark_new_valid_map()"

[1] https://lore.kernel.org/all/20260807202059.2620949-1-joshua.hahnjy@gmail.com/

Joshua Hahn (5):
  mm/memcontrol: flatten try_charge_memcg control flow
  mm/page_counter: introduce per-CPU stock
  mm/page_counter: make page_counter_try_charge() stock-aware
  mm/memcontrol: move memory stock to page counters
  mm/memcontrol: add stock to the memsw page counter

 include/linux/page_counter.h |  35 +++-
 kernel/cgroup/dmem.c         |   2 +-
 mm/hugetlb_cgroup.c          |   2 +-
 mm/memcontrol-v1.c           |  17 +-
 mm/memcontrol.c              | 352 ++++++++++++-----------------------
 mm/page_counter.c            | 209 ++++++++++++++++++++-
 6 files changed, 361 insertions(+), 256 deletions(-)

-- 
2.53.0-Meta

[PATCH v7 1/5] mm/memcontrol: flatten try_charge_memcg control flow

[Joshua Hahn]

Refactor try_charge_memcg by flattening the nested memsw/memory
page_counter operations to separate the logic between the two.

When page_counter_try_charge is made stock-aware, this flattening makes
the control flow easier to follow since each page counter now has its
own success/failure paths.

No functional changes intended.

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Reviewed-by: Harry Yoo (Meta) <harry@kernel.org>
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 mm/memcontrol.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a05d9a8baa1b6..cdd6aba91e444 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2721,18 +2721,21 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 		batch = nr_pages;
 
 	reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
-	if (!do_memsw_account() ||
-	    page_counter_try_charge(&memcg->memsw, batch, &counter)) {
-		if (page_counter_try_charge(&memcg->memory, batch, &counter))
-			goto done_restock;
-		if (do_memsw_account())
-			page_counter_uncharge(&memcg->memsw, batch);
-		mem_over_limit = mem_cgroup_from_counter(counter, memory);
-	} else {
+	if (do_memsw_account() &&
+	    !page_counter_try_charge(&memcg->memsw, batch, &counter)) {
 		mem_over_limit = mem_cgroup_from_counter(counter, memsw);
 		reclaim_options &= ~MEMCG_RECLAIM_MAY_SWAP;
+		goto reclaim;
 	}
 
+	if (page_counter_try_charge(&memcg->memory, batch, &counter))
+		goto done_restock;
+
+	if (do_memsw_account())
+		page_counter_uncharge(&memcg->memsw, batch);
+	mem_over_limit = mem_cgroup_from_counter(counter, memory);
+
+reclaim:
 	if (batch > nr_pages) {
 		batch = nr_pages;
 		goto retry;
-- 
2.53.0-Meta

[PATCH v7 2/5] mm/page_counter: introduce per-CPU stock

[Joshua Hahn]

Introduce a copy of the seven-slot per-CPU stock representation from
memcg in the page_counter layer. struct page_counter_stock_pcp preserves
everything from struct memcg_stock_pcp, but adds a new backpointer to
the base of the percpu stock, since there will be multiple percpu stock
base pointers later in the series (one for memory, one for memsw).

struct page_counter also gets a pointer to the base of the percpu stock,
as well as a struct cgroup_subsys_state pointer to pin its owning CSS
as long as the cached counter remains reachable.

Let's also copy over the drain, refill, and "flush required" functions,
preserving all behaviors from the memcg equivalent. In this patch they
are not connected.

No functional changes intended.

Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 include/linux/page_counter.h |  31 +++++++++
 mm/page_counter.c            | 127 +++++++++++++++++++++++++++++++++++
 2 files changed, 158 insertions(+)

diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
index 2baf7a2b29b2e..cf627c5090093 100644
--- a/include/linux/page_counter.h
+++ b/include/linux/page_counter.h
@@ -5,8 +5,30 @@
 #include <linux/atomic.h>
 #include <linux/cache.h>
 #include <linux/limits.h>
+#include <linux/percpu.h>
+#include <linux/local_lock.h>
+#include <linux/workqueue_types.h>
 #include <asm/page.h>
 
+/*
+ * The value of NR_PAGE_COUNTER_STOCK is selected to keep the cached counters
+ * and their nr_pages in a single cacheline. This may change in the future.
+ */
+#define NR_PAGE_COUNTER_STOCK 7
+#define PAGE_COUNTER_STOCK_BATCH 64UL
+struct cgroup_subsys_state;
+struct page_counter;
+struct page_counter_stock_pcp {
+	local_trylock_t lock;
+	u8 nr_pages[NR_PAGE_COUNTER_STOCK];
+	struct page_counter *cached[NR_PAGE_COUNTER_STOCK];
+
+	struct page_counter_stock_pcp __percpu *base;
+	struct work_struct work;
+	unsigned long flags;
+	u8 drain_idx;
+};
+
 struct page_counter {
 	/*
 	 * Make sure 'usage' does not share cacheline with any other field in
@@ -41,6 +63,8 @@ struct page_counter {
 	unsigned long high;
 	unsigned long max;
 	struct page_counter *parent;
+	struct page_counter_stock_pcp __percpu *stock;
+	struct cgroup_subsys_state *stock_css;
 } ____cacheline_internodealigned_in_smp;
 
 #if BITS_PER_LONG == 32
@@ -61,6 +85,8 @@ static inline void page_counter_init(struct page_counter *counter,
 	counter->parent = parent;
 	counter->protection_support = protection_support;
 	counter->track_failcnt = false;
+	counter->stock = NULL;
+	counter->stock_css = NULL;
 }
 
 static inline unsigned long page_counter_read(const struct page_counter *counter)
@@ -74,6 +100,11 @@ void page_counter_charge(struct page_counter *counter, unsigned long nr_pages);
 bool page_counter_try_charge(struct page_counter *counter,
 			     unsigned long nr_pages,
 			     struct page_counter **fail);
+void page_counter_refill_stock(struct page_counter *counter,
+			       unsigned long nr_pages);
+void page_counter_drain_stock_fully(struct page_counter_stock_pcp *stock);
+bool page_counter_stock_flush_required(struct page_counter_stock_pcp *stock,
+				       struct cgroup_subsys_state *root_css);
 void page_counter_uncharge(struct page_counter *counter, unsigned long nr_pages);
 void page_counter_set_min(struct page_counter *counter, unsigned long nr_pages);
 void page_counter_set_low(struct page_counter *counter, unsigned long nr_pages);
diff --git a/mm/page_counter.c b/mm/page_counter.c
index 9167ffd1380c8..71c7a89026cb9 100644
--- a/mm/page_counter.c
+++ b/mm/page_counter.c
@@ -7,6 +7,7 @@
 
 #include <linux/page_counter.h>
 #include <linux/atomic.h>
+#include <linux/cgroup.h>
 #include <linux/kernel.h>
 #include <linux/math64.h>
 #include <linux/string.h>
@@ -14,6 +15,14 @@
 #include <linux/bug.h>
 #include <asm/page.h>
 
+/*
+ * Watermarks for a charge stock slot, in the spirit of pcp->high and
+ * pcp->batch: PAGE_COUNTER_STOCK_HIGH is the high watermark at which a slot is
+ * trimmed down to PAGE_COUNTER_STOCK_LOW rather than emptied.
+ */
+#define PAGE_COUNTER_STOCK_LOW (PAGE_COUNTER_STOCK_BATCH / 2)
+#define PAGE_COUNTER_STOCK_HIGH PAGE_COUNTER_STOCK_BATCH
+
 static bool track_protection(struct page_counter *c)
 {
 	return c->protection_support;
@@ -192,6 +201,124 @@ bool page_counter_try_charge(struct page_counter *counter,
 	return false;
 }
 
+static void page_counter_drain_stock(struct page_counter_stock_pcp *stock,
+				     int i)
+{
+	struct page_counter *counter = READ_ONCE(stock->cached[i]);
+	u8 nr_pages;
+
+	if (!counter)
+		return;
+
+	nr_pages = READ_ONCE(stock->nr_pages[i]);
+	if (nr_pages) {
+		page_counter_uncharge(counter, nr_pages);
+		WRITE_ONCE(stock->nr_pages[i], 0);
+	}
+	WRITE_ONCE(stock->cached[i], NULL);
+	css_put(counter->stock_css);
+}
+
+void page_counter_drain_stock_fully(struct page_counter_stock_pcp *stock)
+{
+	int i;
+
+	for (i = 0; i < NR_PAGE_COUNTER_STOCK; i++)
+		page_counter_drain_stock(stock, i);
+}
+
+bool page_counter_stock_flush_required(struct page_counter_stock_pcp *stock,
+				       struct cgroup_subsys_state *root_css)
+{
+	struct cgroup_subsys_state *css;
+	struct page_counter *counter;
+	bool flush = false;
+	int i;
+
+	rcu_read_lock();
+	for (i = 0; i < NR_PAGE_COUNTER_STOCK; i++) {
+		counter = READ_ONCE(stock->cached[i]);
+		if (!counter)
+			continue;
+		css = READ_ONCE(counter->stock_css);
+
+		if (READ_ONCE(stock->nr_pages[i]) &&
+		    cgroup_is_descendant(css->cgroup, root_css->cgroup)) {
+			flush = true;
+			break;
+		}
+	}
+	rcu_read_unlock();
+	return flush;
+}
+
+/**
+ * page_counter_refill_stock - return pages to a counter's stock
+ * @counter: counter to return pages to
+ * @nr_pages: number of pages to return
+ *
+ * If the stock cannot accept the pages, uncharge them from the hierarchy.
+ */
+void page_counter_refill_stock(struct page_counter *counter,
+			       unsigned long nr_pages)
+{
+	struct page_counter_stock_pcp __percpu *stock = counter->stock;
+	struct page_counter_stock_pcp *pcp_stock;
+	unsigned int stock_pages;
+	int empty_slot = -1;
+	int i;
+
+	/*
+	 * nr_pages[] is a u8 and a slot is capped at PAGE_COUNTER_STOCK_HIGH.
+	 * Raising PAGE_COUNTER_STOCK_BATCH beyond 127 would need careful
+	 * handling of nr_pages[] in struct page_counter_stock_pcp.
+	 */
+	BUILD_BUG_ON(PAGE_COUNTER_STOCK_BATCH > S8_MAX);
+	BUILD_BUG_ON(PAGE_COUNTER_STOCK_HIGH > U8_MAX);
+
+	if (!stock || nr_pages > PAGE_COUNTER_STOCK_BATCH ||
+	    !local_trylock(&stock->lock)) {
+		/*
+		 * For a larger-than-batch refill or an unlikely failure to lock
+		 * the per-CPU stock, uncharge the hierarchy directly.
+		 */
+		page_counter_uncharge(counter, nr_pages);
+		return;
+	}
+
+	pcp_stock = this_cpu_ptr(stock);
+	for (i = 0; i < NR_PAGE_COUNTER_STOCK; i++) {
+		struct page_counter *cached = READ_ONCE(pcp_stock->cached[i]);
+
+		if (!cached && empty_slot == -1)
+			empty_slot = i;
+		if (counter != cached)
+			continue;
+
+		stock_pages = READ_ONCE(pcp_stock->nr_pages[i]) + nr_pages;
+		if (stock_pages > PAGE_COUNTER_STOCK_HIGH) {
+			page_counter_uncharge(counter,
+					      stock_pages - PAGE_COUNTER_STOCK_LOW);
+			stock_pages = PAGE_COUNTER_STOCK_LOW;
+		}
+		WRITE_ONCE(pcp_stock->nr_pages[i], stock_pages);
+		local_unlock(&stock->lock);
+		return;
+	}
+
+	i = empty_slot;
+	if (i == -1) {
+		i = pcp_stock->drain_idx++;
+		if (pcp_stock->drain_idx == NR_PAGE_COUNTER_STOCK)
+			pcp_stock->drain_idx = 0;
+		page_counter_drain_stock(pcp_stock, i);
+	}
+	css_get(counter->stock_css);
+	WRITE_ONCE(pcp_stock->cached[i], counter);
+	WRITE_ONCE(pcp_stock->nr_pages[i], nr_pages);
+	local_unlock(&stock->lock);
+}
+
 /**
  * page_counter_uncharge - hierarchically uncharge pages
  * @counter: counter
-- 
2.53.0-Meta

[PATCH v7 3/5] mm/page_counter: make page_counter_try_charge() stock-aware

[Joshua Hahn]

Make page_counter_try_charge() consume stock transparently to callers
while preserving the same semantics as try_charge_memcg's greedy
charge attempt, refill, and !allow_spinning special case.

page_counter_try_charge gets two new parameters, may_batch and
nr_charged.

may_batch is intended to preserve the behavior of !allow_spinning
scenarios in try_charge_memcg, where the goal is to charge as quickly
as possible, without evicting other stock slots or making a greedy
charge to refill the stock.

nr_charged is used to reflect the size of the successful hierarchy
charge, preserving the existing batch-based memory.high accounting
for current->memcg_nr_pages_over_high. As with refill_stock() today,
the full charge is reported even if the excess is later uncharged
because the stock could not accept it. This only affects memory.high
throttling heuristics and never fails a charge. This behavior may be
fixed to properly account the net charge in a future follow-up.

As of this patch, no page_counter has stock yet, so there are no
functional changes intended.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 include/linux/page_counter.h |  4 +-
 kernel/cgroup/dmem.c         |  2 +-
 mm/hugetlb_cgroup.c          |  2 +-
 mm/memcontrol-v1.c           |  3 +-
 mm/memcontrol.c              |  8 ++--
 mm/page_counter.c            | 82 +++++++++++++++++++++++++++++++-----
 6 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
index cf627c5090093..58532259e7f59 100644
--- a/include/linux/page_counter.h
+++ b/include/linux/page_counter.h
@@ -98,8 +98,8 @@ long page_counter_margin(const struct page_counter *counter);
 void page_counter_cancel(struct page_counter *counter, unsigned long nr_pages);
 void page_counter_charge(struct page_counter *counter, unsigned long nr_pages);
 bool page_counter_try_charge(struct page_counter *counter,
-			     unsigned long nr_pages,
-			     struct page_counter **fail);
+			     unsigned long nr_pages, struct page_counter **fail,
+			     bool may_batch, unsigned long *nr_charged);
 void page_counter_refill_stock(struct page_counter *counter,
 			       unsigned long nr_pages);
 void page_counter_drain_stock_fully(struct page_counter_stock_pcp *stock);
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 4683f3d680226..569307aa4bd62 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -736,7 +736,7 @@ int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
 		goto err;
 	}
 
-	if (!page_counter_try_charge(&pool->cnt, size, &fail)) {
+	if (!page_counter_try_charge(&pool->cnt, size, &fail, false, NULL)) {
 		if (ret_limit_pool) {
 			*ret_limit_pool = container_of(fail, struct dmem_cgroup_pool_state, cnt);
 			css_get(&(*ret_limit_pool)->cs->css);
diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
index 3fb41311e4c7d..bbc2dd3fa3c97 100644
--- a/mm/hugetlb_cgroup.c
+++ b/mm/hugetlb_cgroup.c
@@ -304,7 +304,7 @@ static int __hugetlb_cgroup_charge_cgroup(int idx, unsigned long nr_pages,
 
 	if (!page_counter_try_charge(
 		    __hugetlb_cgroup_counter_from_cgroup(h_cg, idx, rsvd),
-		    nr_pages, &counter)) {
+		    nr_pages, &counter, false, NULL)) {
 		ret = -ENOMEM;
 		hugetlb_event(h_cg, idx, HUGETLB_MAX);
 		css_put(&h_cg->css);
diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index b7f2868885071..a146e54c6f9f7 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -2193,7 +2193,8 @@ bool memcg1_charge_skmem(struct mem_cgroup *memcg, unsigned int nr_pages,
 {
 	struct page_counter *fail;
 
-	if (page_counter_try_charge(&memcg->tcpmem, nr_pages, &fail)) {
+	if (page_counter_try_charge(&memcg->tcpmem, nr_pages, &fail,
+				    false, NULL)) {
 		memcg->tcpmem_pressure = 0;
 		return true;
 	}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index cdd6aba91e444..f232ccac69c36 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2722,13 +2722,14 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 
 	reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
 	if (do_memsw_account() &&
-	    !page_counter_try_charge(&memcg->memsw, batch, &counter)) {
+	    !page_counter_try_charge(&memcg->memsw, batch, &counter, false,
+				     NULL)) {
 		mem_over_limit = mem_cgroup_from_counter(counter, memsw);
 		reclaim_options &= ~MEMCG_RECLAIM_MAY_SWAP;
 		goto reclaim;
 	}
 
-	if (page_counter_try_charge(&memcg->memory, batch, &counter))
+	if (page_counter_try_charge(&memcg->memory, batch, &counter, false, NULL))
 		goto done_restock;
 
 	if (do_memsw_account())
@@ -6028,7 +6029,8 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
 		private_id = mem_cgroup_private_id_get(memcg, nr_pages);
 
 		if (!mem_cgroup_private_id_is_root(private_id) &&
-		    !page_counter_try_charge(&memcg->swap, nr_pages, &counter)) {
+		    !page_counter_try_charge(&memcg->swap, nr_pages, &counter,
+					     false, NULL)) {
 			memcg_memory_event(memcg, MEMCG_SWAP_MAX);
 			memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
 			mem_cgroup_private_id_put(private_id, nr_pages);
diff --git a/mm/page_counter.c b/mm/page_counter.c
index 71c7a89026cb9..4c30abf51e966 100644
--- a/mm/page_counter.c
+++ b/mm/page_counter.c
@@ -136,23 +136,76 @@ void page_counter_charge(struct page_counter *counter, unsigned long nr_pages)
 	}
 }
 
+/*
+ * Consume the cached charge if enough nr_pages are present, otherwise return
+ * failure. Also return failure for charge requests larger than
+ * PAGE_COUNTER_STOCK_BATCH or if the local lock is already taken.
+ */
+static bool page_counter_consume_stock(struct page_counter *counter,
+				       unsigned long nr_pages)
+{
+	struct page_counter_stock_pcp __percpu *stock = counter->stock;
+	struct page_counter_stock_pcp *pcp_stock;
+	u8 stock_pages;
+	bool ret = false;
+	int i;
+
+	if (nr_pages > PAGE_COUNTER_STOCK_BATCH ||
+	    !local_trylock(&stock->lock))
+		return false;
+
+	pcp_stock = this_cpu_ptr(stock);
+	for (i = 0; i < NR_PAGE_COUNTER_STOCK; i++) {
+		if (counter != READ_ONCE(pcp_stock->cached[i]))
+			continue;
+
+		stock_pages = READ_ONCE(pcp_stock->nr_pages[i]);
+		if (stock_pages >= nr_pages) {
+			stock_pages -= nr_pages;
+			WRITE_ONCE(pcp_stock->nr_pages[i], stock_pages);
+			if (!stock_pages) {
+				WRITE_ONCE(pcp_stock->cached[i], NULL);
+				css_put(counter->stock_css);
+			}
+			ret = true;
+		}
+		break;
+	}
+	local_unlock(&stock->lock);
+
+	return ret;
+}
+
 /**
- * page_counter_try_charge - try to hierarchically charge pages
+ * page_counter_try_charge - try to hierarchically charge pages using stock
  * @counter: counter
- * @nr_pages: number of pages to charge
- * @fail: points first counter to hit its limit, if any
+ * @nr_pages: number of pages requested
+ * @fail: points to the first counter to hit its limit, if any
+ * @may_batch: whether a stock miss may trigger a batch charge
+ * @nr_charged: optional; set to the hierarchy charge size on success
  *
- * Returns %true on success, or %false and @fail if the counter or one
- * of its ancestors has hit its configured limit.
+ * Return: %true if the request was satisfied. A failed batch charge may update
+ * @fail before an exact retry succeeds.
  */
 bool page_counter_try_charge(struct page_counter *counter,
-			     unsigned long nr_pages,
-			     struct page_counter **fail)
+			     unsigned long nr_pages, struct page_counter **fail,
+			     bool may_batch, unsigned long *nr_charged)
 {
+	unsigned long charge = nr_pages;
 	struct page_counter *c;
 	bool protection = track_protection(counter);
 	bool track_failcnt = counter->track_failcnt;
 
+	if (counter->stock && may_batch)
+		charge = max(nr_pages, PAGE_COUNTER_STOCK_BATCH);
+
+retry:
+	if (counter->stock && page_counter_consume_stock(counter, nr_pages)) {
+		if (nr_charged)
+			*nr_charged = 0;
+		return true;
+	}
+
 	for (c = counter; c; c = c->parent) {
 		long new;
 		/*
@@ -169,9 +222,9 @@ bool page_counter_try_charge(struct page_counter *counter,
 		 * we either see the new limit or the setter sees the
 		 * counter has changed and retries.
 		 */
-		new = atomic_long_add_return(nr_pages, &c->usage);
+		new = atomic_long_add_return(charge, &c->usage);
 		if (new > c->max) {
-			atomic_long_sub(nr_pages, &c->usage);
+			atomic_long_sub(charge, &c->usage);
 			/*
 			 * This is racy, but we can live with some
 			 * inaccuracy in the failcnt which is only used
@@ -192,11 +245,20 @@ bool page_counter_try_charge(struct page_counter *counter,
 				WRITE_ONCE(c->watermark, new);
 		}
 	}
+	if (charge > nr_pages)
+		page_counter_refill_stock(counter, charge - nr_pages);
+	if (nr_charged)
+		*nr_charged = charge;
 	return true;
 
 failed:
 	for (c = counter; c != *fail; c = c->parent)
-		page_counter_cancel(c, nr_pages);
+		page_counter_cancel(c, charge);
+
+	if (charge > nr_pages) {
+		charge = nr_pages;
+		goto retry;
+	}
 
 	return false;
 }
-- 
2.53.0-Meta

[PATCH v7 4/5] mm/memcontrol: move memory stock to page counters

[Joshua Hahn]

Transition memcg to use the page_counter_stock for the memory
page_counter instead of relying on a memcg-wide stock.

One aspect that remains non-transparent to memcg is the uncharge path.
This is intentional, as the caller is responsible for managing the
batching. Cacheable releases refill the stock, while already-batched
frees (i.e. uncharge_gather), rollbacks, and accounting transfers
uncharge the hierarchy directly. The refill helper itself already falls
back to a raw uncharge when the stock cannot accept the pages anyways.

Because the memory and memsw counters no longer share one stock, their
raw values can temporarily diverge, and the raw memory counter can
exceed the raw memsw counter. Preserve the legacy user-visible
memory <= memory+swap relationship by reporting the smaller of the two
raw values for memory.usage_in_bytes.

We accept this fuzziness over breaking the invariant, as usage_in_bytes
has always been a fuzzy value due to stock charges being included.
Moreover, the raw counters are upper bounds on charged memory, so the
reported value stays within that fuzz.

One final nuance is that with divergent charges now, it is possible for
memory to fail first even when memory and memsw have equal limits.
When this happens, make sure reclaim doesn't swap, as if memsw failed.
Factor out this logic into memcg_charge_reclaim_options() for clarity.

With this transition, remove all memcg code that is no longer used.
After all of this, there should be no functional change for cgroup v2
users, and the cgroup-v1 equal-limit no-swap behavior is preserved. All
v2 behaviors from memcg are preserved, just moved from memcg to
page_counter code, so that future work can introduce additional
page_counters without removing the fast path.
Explicitly, the preserved behaviors are:
 - 7-slot stock
 - drain policy works locally and remotely through the memcg_wq
 - check whether a stock requires flushing before taking action
 - exact charging for non-spinning callers
 - report hierarchy growth for memory.high overage accounting

As of this patch, this leaves memsw un-stocked and always taking the
slow path (raw hierarchy charge). The next patch will make memsw
stocked, which will close the fast path gap for legacy cgroup users.

Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 mm/memcontrol-v1.c |  14 ++-
 mm/memcontrol.c    | 303 +++++++++++----------------------------------
 2 files changed, 83 insertions(+), 234 deletions(-)

diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index a146e54c6f9f7..2f29315b69fc3 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -122,10 +122,18 @@ static unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap)
 		if (swap)
 			val += total_swap_pages - get_nr_swap_pages();
 	} else {
-		if (!swap)
-			val = page_counter_read(&memcg->memory);
-		else
+		if (!swap) {
+			/*
+			 * Both counters remain upper bounds on charged memory,
+			 * but cached charges can make their raw values drift.
+			 * Use the smaller value to preserve the user-visible
+			 * memory <= memsw invariant.
+			 */
+			val = min(page_counter_read(&memcg->memory),
+				  page_counter_read(&memcg->memsw));
+		} else {
 			val = page_counter_read(&memcg->memsw);
+		}
 	}
 	return val;
 }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f232ccac69c36..ee1e46e58e38f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2063,33 +2063,11 @@ void mem_cgroup_print_oom_group(struct mem_cgroup *memcg)
 	pr_cont(" are going to be killed due to memory.oom.group set\n");
 }
 
-/*
- * The value of NR_MEMCG_STOCK is selected to keep the cached memcgs and their
- * nr_pages in a single cacheline. This may change in future.
- */
-#define NR_MEMCG_STOCK 7
-
-/*
- * Watermarks for a charge stock slot, in the spirit of pcp->high and
- * pcp->batch: MEMCG_STOCK_HIGH is the high watermark at which a slot is
- * trimmed, and it is trimmed down to MEMCG_STOCK_LOW rather than emptied.
- */
-#define MEMCG_STOCK_LOW		(MEMCG_CHARGE_BATCH / 2)
-#define MEMCG_STOCK_HIGH	(MEMCG_CHARGE_BATCH)
-
 #define FLUSHING_CACHED_CHARGE	0
-struct memcg_stock_pcp {
-	local_trylock_t lock;
-	uint8_t nr_pages[NR_MEMCG_STOCK];
-	struct mem_cgroup *cached[NR_MEMCG_STOCK];
 
-	struct work_struct work;
-	unsigned long flags;
-	uint8_t drain_idx;
-};
-
-static DEFINE_PER_CPU_ALIGNED(struct memcg_stock_pcp, memcg_stock) = {
+static DEFINE_PER_CPU_ALIGNED(struct page_counter_stock_pcp, memory_stock) = {
 	.lock = INIT_LOCAL_TRYLOCK(lock),
+	.base = &memory_stock,
 };
 
 /*
@@ -2139,52 +2117,6 @@ static void drain_obj_stock(struct obj_stock_pcp *stock);
 static bool obj_stock_flush_required(struct obj_stock_pcp *stock,
 				     struct mem_cgroup *root_memcg);
 
-/**
- * consume_stock: Try to consume stocked charge on this cpu.
- * @memcg: memcg to consume from.
- * @nr_pages: how many pages to charge.
- *
- * Consume the cached charge if enough nr_pages are present otherwise return
- * failure. Also return failure for charge request larger than
- * MEMCG_CHARGE_BATCH or if the local lock is already taken.
- *
- * returns true if successful, false otherwise.
- */
-static bool consume_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
-{
-	struct memcg_stock_pcp *stock;
-	uint8_t stock_pages;
-	bool ret = false;
-	int i;
-
-	if (nr_pages > MEMCG_CHARGE_BATCH ||
-	    !local_trylock(&memcg_stock.lock))
-		return ret;
-
-	stock = this_cpu_ptr(&memcg_stock);
-
-	for (i = 0; i < NR_MEMCG_STOCK; ++i) {
-		if (memcg != READ_ONCE(stock->cached[i]))
-			continue;
-
-		stock_pages = READ_ONCE(stock->nr_pages[i]);
-		if (stock_pages >= nr_pages) {
-			stock_pages -= nr_pages;
-			WRITE_ONCE(stock->nr_pages[i], stock_pages);
-			if (!stock_pages) {
-				WRITE_ONCE(stock->cached[i], NULL);
-				css_put(&memcg->css);
-			}
-			ret = true;
-		}
-		break;
-	}
-
-	local_unlock(&memcg_stock.lock);
-
-	return ret;
-}
-
 static void memcg_uncharge(struct mem_cgroup *memcg, unsigned int nr_pages)
 {
 	page_counter_uncharge(&memcg->memory, nr_pages);
@@ -2192,49 +2124,22 @@ static void memcg_uncharge(struct mem_cgroup *memcg, unsigned int nr_pages)
 		page_counter_uncharge(&memcg->memsw, nr_pages);
 }
 
-/*
- * Returns stocks cached in percpu and reset cached information.
- */
-static void drain_stock(struct memcg_stock_pcp *stock, int i)
+static void drain_local_stock(struct work_struct *work)
 {
-	struct mem_cgroup *old = READ_ONCE(stock->cached[i]);
-	uint8_t stock_pages;
-
-	if (!old)
-		return;
-
-	stock_pages = READ_ONCE(stock->nr_pages[i]);
-	if (stock_pages) {
-		memcg_uncharge(old, stock_pages);
-		WRITE_ONCE(stock->nr_pages[i], 0);
-	}
-
-	WRITE_ONCE(stock->cached[i], NULL);
-	css_put(&old->css);
-}
-
-static void drain_stock_fully(struct memcg_stock_pcp *stock)
-{
-	int i;
-
-	for (i = 0; i < NR_MEMCG_STOCK; ++i)
-		drain_stock(stock, i);
-}
-
-static void drain_local_memcg_stock(struct work_struct *dummy)
-{
-	struct memcg_stock_pcp *stock;
+	struct page_counter_stock_pcp *pcp_stock;
+	struct page_counter_stock_pcp __percpu *stock;
 
 	if (WARN_ONCE(!in_task(), "drain in non-task context"))
 		return;
 
-	local_lock(&memcg_stock.lock);
+	stock = container_of(work, struct page_counter_stock_pcp, work)->base;
+	local_lock(&stock->lock);
 
-	stock = this_cpu_ptr(&memcg_stock);
-	drain_stock_fully(stock);
-	clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags);
+	pcp_stock = this_cpu_ptr(stock);
+	page_counter_drain_stock_fully(pcp_stock);
+	clear_bit(FLUSHING_CACHED_CHARGE, &pcp_stock->flags);
 
-	local_unlock(&memcg_stock.lock);
+	local_unlock(&stock->lock);
 }
 
 static void drain_local_obj_stock(struct work_struct *dummy)
@@ -2253,92 +2158,6 @@ static void drain_local_obj_stock(struct work_struct *dummy)
 	local_unlock(&obj_stock.lock);
 }
 
-static void refill_stock(struct mem_cgroup *memcg, unsigned int nr_pages)
-{
-	struct memcg_stock_pcp *stock;
-	struct mem_cgroup *cached;
-	unsigned int stock_pages;
-	bool success = false;
-	int empty_slot = -1;
-	int i;
-
-	/*
-	 * nr_pages[] is a uint8_t and a slot's count is capped at
-	 * MEMCG_STOCK_HIGH. Raising MEMCG_CHARGE_BATCH beyond 127 would need
-	 * more careful handling of nr_pages[] in struct memcg_stock_pcp.
-	 */
-	BUILD_BUG_ON(MEMCG_CHARGE_BATCH > S8_MAX);
-	BUILD_BUG_ON(MEMCG_STOCK_HIGH > U8_MAX);
-
-	VM_WARN_ON_ONCE(mem_cgroup_is_root(memcg));
-
-	if (nr_pages > MEMCG_CHARGE_BATCH ||
-	    !local_trylock(&memcg_stock.lock)) {
-		/*
-		 * In case of larger than batch refill or unlikely failure to
-		 * lock the percpu memcg_stock.lock, uncharge memcg directly.
-		 */
-		memcg_uncharge(memcg, nr_pages);
-		return;
-	}
-
-	stock = this_cpu_ptr(&memcg_stock);
-	for (i = 0; i < NR_MEMCG_STOCK; ++i) {
-		cached = READ_ONCE(stock->cached[i]);
-		if (!cached && empty_slot == -1)
-			empty_slot = i;
-		if (memcg == READ_ONCE(stock->cached[i])) {
-			stock_pages = READ_ONCE(stock->nr_pages[i]) + nr_pages;
-			if (stock_pages > MEMCG_STOCK_HIGH) {
-				memcg_uncharge(memcg,
-					       stock_pages - MEMCG_STOCK_LOW);
-				stock_pages = MEMCG_STOCK_LOW;
-			}
-			WRITE_ONCE(stock->nr_pages[i], stock_pages);
-			success = true;
-			break;
-		}
-	}
-
-	if (!success) {
-		i = empty_slot;
-		if (i == -1) {
-			i = stock->drain_idx++;
-			if (stock->drain_idx == NR_MEMCG_STOCK)
-				stock->drain_idx = 0;
-			drain_stock(stock, i);
-		}
-		css_get(&memcg->css);
-		WRITE_ONCE(stock->cached[i], memcg);
-		WRITE_ONCE(stock->nr_pages[i], nr_pages);
-	}
-
-	local_unlock(&memcg_stock.lock);
-}
-
-static bool is_memcg_drain_needed(struct memcg_stock_pcp *stock,
-				  struct mem_cgroup *root_memcg)
-{
-	struct mem_cgroup *memcg;
-	bool flush = false;
-	int i;
-
-	rcu_read_lock();
-	for (i = 0; i < NR_MEMCG_STOCK; ++i) {
-		memcg = READ_ONCE(stock->cached[i]);
-		if (!memcg)
-			continue;
-
-		if (READ_ONCE(stock->nr_pages[i]) &&
-		    mem_cgroup_is_descendant(memcg, root_memcg)) {
-			flush = true;
-			break;
-		}
-	}
-	rcu_read_unlock();
-	return flush;
-}
-
 static bool schedule_drain_work(int cpu, struct work_struct *work)
 {
 	/*
@@ -2370,23 +2189,25 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 	 * Notify other cpus that system-wide "drain" is running
 	 * We do not care about races with the cpu hotplug because cpu down
 	 * as well as workers from this path always operate on the local
-	 * per-cpu data. CPU up doesn't touch memcg_stock at all.
+	 * per-cpu data. CPU up doesn't touch the stocks at all.
 	 */
 	migrate_disable();
 	curcpu = smp_processor_id();
 	for_each_online_cpu(cpu) {
-		struct memcg_stock_pcp *memcg_st = &per_cpu(memcg_stock, cpu);
+		struct page_counter_stock_pcp *memory_st =
+			per_cpu_ptr(&memory_stock, cpu);
 		struct obj_stock_pcp *obj_st = &per_cpu(obj_stock, cpu);
 
-		if (!test_bit(FLUSHING_CACHED_CHARGE, &memcg_st->flags) &&
-		    is_memcg_drain_needed(memcg_st, root_memcg) &&
+		if (!test_bit(FLUSHING_CACHED_CHARGE, &memory_st->flags) &&
+		    page_counter_stock_flush_required(memory_st,
+						      &root_memcg->css) &&
 		    !test_and_set_bit(FLUSHING_CACHED_CHARGE,
-				      &memcg_st->flags)) {
+				      &memory_st->flags)) {
 			if (cpu == curcpu)
-				drain_local_memcg_stock(&memcg_st->work);
-			else if (!schedule_drain_work(cpu, &memcg_st->work))
+				drain_local_stock(&memory_st->work);
+			else if (!schedule_drain_work(cpu, &memory_st->work))
 				clear_bit(FLUSHING_CACHED_CHARGE,
-					  &memcg_st->flags);
+					  &memory_st->flags);
 		}
 
 		if (!test_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags) &&
@@ -2406,12 +2227,14 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 
 static int memcg_hotplug_cpu_dead(unsigned int cpu)
 {
-	struct memcg_stock_pcp *memcg_st = &per_cpu(memcg_stock, cpu);
+	struct page_counter_stock_pcp *stock;
 	struct obj_stock_pcp *obj_st = &per_cpu(obj_stock, cpu);
 
 	/* no need for the local lock */
 	drain_obj_stock(obj_st);
-	drain_stock_fully(memcg_st);
+	stock = per_cpu_ptr(&memory_stock, cpu);
+	page_counter_drain_stock_fully(stock);
+	clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags);
 
 	/*
 	 * A drain work queued before the CPU went away is executed by an
@@ -2419,7 +2242,6 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
 	 * clear the flags here to make these stocks drainable again once
 	 * the CPU comes back online.
 	 */
-	clear_bit(FLUSHING_CACHED_CHARGE, &memcg_st->flags);
 	clear_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags);
 
 	return 0;
@@ -2696,13 +2518,33 @@ void __mem_cgroup_handle_over_high(gfp_t gfp_mask)
 	css_put(&memcg->css);
 }
 
+static unsigned int memcg_charge_reclaim_options(struct mem_cgroup *memcg,
+						 struct page_counter *counter)
+{
+	if (!do_memsw_account())
+		return MEMCG_RECLAIM_MAY_SWAP;
+
+	if (counter == &memcg->memsw)
+		return 0;
+
+	/*
+	 * With separate stocks, raw memory can exceed raw memsw, so memory can
+	 * fail first even when equal limits leave no room for swap. Don't swap
+	 * in that case, as if memsw had failed.
+	 */
+	if (READ_ONCE(memcg->memory.max) >= READ_ONCE(memcg->memsw.max))
+		return 0;
+
+	return MEMCG_RECLAIM_MAY_SWAP;
+}
+
 static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 			    unsigned int nr_pages)
 {
-	unsigned int batch = max(MEMCG_CHARGE_BATCH, nr_pages);
 	int nr_retries = MAX_RECLAIM_RETRIES;
 	struct mem_cgroup *mem_over_limit;
 	struct page_counter *counter;
+	unsigned long nr_charged;
 	unsigned long nr_reclaimed;
 	bool passed_oom = false;
 	unsigned int reclaim_options;
@@ -2710,37 +2552,30 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 	bool raised_max_event = false;
 	unsigned long pflags;
 	bool allow_spinning = gfpflags_allow_spinning(gfp_mask);
+	bool may_batch = allow_spinning;
 	int ret = 0;
 
 retry:
-	if (consume_stock(memcg, nr_pages))
-		return ret;
-
-	if (!allow_spinning)
-		/* Avoid the refill and flush of the older stock */
-		batch = nr_pages;
-
-	reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
 	if (do_memsw_account() &&
-	    !page_counter_try_charge(&memcg->memsw, batch, &counter, false,
+	    !page_counter_try_charge(&memcg->memsw, nr_pages, &counter, false,
 				     NULL)) {
 		mem_over_limit = mem_cgroup_from_counter(counter, memsw);
-		reclaim_options &= ~MEMCG_RECLAIM_MAY_SWAP;
 		goto reclaim;
 	}
 
-	if (page_counter_try_charge(&memcg->memory, batch, &counter, false, NULL))
-		goto done_restock;
+	if (page_counter_try_charge(&memcg->memory, nr_pages, &counter,
+				    may_batch, &nr_charged))
+		goto check_high;
 
 	if (do_memsw_account())
-		page_counter_uncharge(&memcg->memsw, batch);
+		page_counter_uncharge(&memcg->memsw, nr_pages);
 	mem_over_limit = mem_cgroup_from_counter(counter, memory);
 
 reclaim:
-	if (batch > nr_pages) {
-		batch = nr_pages;
-		goto retry;
-	}
+	reclaim_options = memcg_charge_reclaim_options(mem_over_limit, counter);
+
+	/* Do not retry speculative batch charges after the first miss. */
+	may_batch = false;
 
 	/*
 	 * Prevent unbounded recursion when reclaim operations need to
@@ -2853,10 +2688,9 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 
 	return ret;
 
-done_restock:
-	if (batch > nr_pages)
-		refill_stock(memcg, batch - nr_pages);
-
+check_high:
+	if (!nr_charged)
+		return ret;
 	/*
 	 * If the hierarchy is above the normal consumption range, schedule
 	 * reclaim on returning to userland.  We can perform reclaim here
@@ -2896,7 +2730,7 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 			 * and distribute reclaim work and delay penalties
 			 * based on how much each task is actually allocating.
 			 */
-			current->memcg_nr_pages_over_high += batch;
+			current->memcg_nr_pages_over_high += nr_charged;
 			set_notify_resume(current);
 			break;
 		}
@@ -3201,8 +3035,11 @@ static void obj_cgroup_uncharge_pages(struct obj_cgroup *objcg,
 
 	account_kmem_nmi_safe(memcg, -nr_pages);
 	memcg1_account_kmem(memcg, -nr_pages);
-	if (!mem_cgroup_is_root(memcg))
-		refill_stock(memcg, nr_pages);
+	if (!mem_cgroup_is_root(memcg)) {
+		page_counter_refill_stock(&memcg->memory, nr_pages);
+		if (do_memsw_account())
+			page_counter_uncharge(&memcg->memsw, nr_pages);
+	}
 
 	css_put(&memcg->css);
 }
@@ -4349,6 +4186,8 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 	page_counter_set_high(&memcg->swap, PAGE_COUNTER_MAX);
 	if (parent) {
 		page_counter_init(&memcg->memory, &parent->memory, memcg_on_dfl);
+		memcg->memory.stock = &memory_stock;
+		memcg->memory.stock_css = &memcg->css;
 		page_counter_init(&memcg->swap, &parent->swap, false);
 #ifdef CONFIG_MEMCG_V1
 		WRITE_ONCE(memcg->swappiness, mem_cgroup_swappiness(parent));
@@ -5817,7 +5656,7 @@ void mem_cgroup_sk_uncharge(const struct sock *sk, unsigned int nr_pages)
 
 	mod_memcg_state(memcg, MEMCG_SOCK, -nr_pages);
 
-	refill_stock(memcg, nr_pages);
+	page_counter_refill_stock(&memcg->memory, nr_pages);
 }
 
 void mem_cgroup_flush_workqueue(void)
@@ -5965,6 +5804,8 @@ int __init mem_cgroup_init(void)
 	 * exceed S32_MAX / PAGE_SIZE.
 	 */
 	BUILD_BUG_ON(MEMCG_CHARGE_BATCH > S32_MAX / PAGE_SIZE);
+	/* Batched page-counter charges feed memcg's memory.high accounting. */
+	BUILD_BUG_ON(MEMCG_CHARGE_BATCH != PAGE_COUNTER_STOCK_BATCH);
 
 	memcg_struct_check();
 
@@ -5975,8 +5816,8 @@ int __init mem_cgroup_init(void)
 	WARN_ON(!memcg_wq);
 
 	for_each_possible_cpu(cpu) {
-		INIT_WORK(&per_cpu_ptr(&memcg_stock, cpu)->work,
-			  drain_local_memcg_stock);
+		INIT_WORK(&per_cpu_ptr(&memory_stock, cpu)->work,
+			  drain_local_stock);
 		INIT_WORK(&per_cpu_ptr(&obj_stock, cpu)->work,
 			  drain_local_obj_stock);
 	}
-- 
2.53.0-Meta

[PATCH v7 5/5] mm/memcontrol: add stock to the memsw page counter

[Joshua Hahn]

Attach legacy memsw page counters to their own seven-slot per-CPU
stock. Charge, refill, drain, and hotplug handling now operate on the
memory and memsw banks independently.

Factor the common drain scheduling into schedule_stock_drain() now
that both stocks use it.

If the memory charge fails, roll back only nr_pages from memsw. Any
batch surplus the memsw charge already placed in the memsw stock
stays cached, like any other stock slot.

The separate banks can hit, contend, evict, and drain independently,
so their raw counters can temporarily drift by their cached amounts.
The previous patch preserves the user-visible cgroup-v1
memory <= memory + swap invariant by reporting memory.usage_in_bytes
as the smaller raw value.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 mm/memcontrol.c | 62 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ee1e46e58e38f..aa3531a3828e5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2070,6 +2070,13 @@ static DEFINE_PER_CPU_ALIGNED(struct page_counter_stock_pcp, memory_stock) = {
 	.base = &memory_stock,
 };
 
+#ifdef CONFIG_MEMCG_V1
+static DEFINE_PER_CPU_ALIGNED(struct page_counter_stock_pcp, memsw_stock) = {
+	.lock = INIT_LOCAL_TRYLOCK(lock),
+	.base = &memsw_stock,
+};
+#endif
+
 /*
  * NR_OBJ_STOCK is sized so the entire hot path of obj_stock_pcp
  * (lock, accounting metadata, nr_bytes[] and cached[]) fits within a
@@ -2174,12 +2181,30 @@ static bool schedule_drain_work(int cpu, struct work_struct *work)
 	return true;
 }
 
+static void schedule_stock_drain(struct page_counter_stock_pcp __percpu *stock,
+				 struct cgroup_subsys_state *root_css,
+				 int cpu, int curcpu)
+{
+	struct page_counter_stock_pcp *pcp_stock = per_cpu_ptr(stock, cpu);
+
+	if (test_bit(FLUSHING_CACHED_CHARGE, &pcp_stock->flags) ||
+	    !page_counter_stock_flush_required(pcp_stock, root_css) ||
+	    test_and_set_bit(FLUSHING_CACHED_CHARGE, &pcp_stock->flags))
+		return;
+
+	if (cpu == curcpu)
+		drain_local_stock(&pcp_stock->work);
+	else if (!schedule_drain_work(cpu, &pcp_stock->work))
+		clear_bit(FLUSHING_CACHED_CHARGE, &pcp_stock->flags);
+}
+
 /*
  * Drains all per-CPU charge caches for given root_memcg resp. subtree
  * of the hierarchy under it.
  */
 void drain_all_stock(struct mem_cgroup *root_memcg)
 {
+	struct cgroup_subsys_state *root_css = &root_memcg->css;
 	int cpu, curcpu;
 
 	/* If someone's already draining, avoid adding running more workers. */
@@ -2194,21 +2219,13 @@ void drain_all_stock(struct mem_cgroup *root_memcg)
 	migrate_disable();
 	curcpu = smp_processor_id();
 	for_each_online_cpu(cpu) {
-		struct page_counter_stock_pcp *memory_st =
-			per_cpu_ptr(&memory_stock, cpu);
 		struct obj_stock_pcp *obj_st = &per_cpu(obj_stock, cpu);
 
-		if (!test_bit(FLUSHING_CACHED_CHARGE, &memory_st->flags) &&
-		    page_counter_stock_flush_required(memory_st,
-						      &root_memcg->css) &&
-		    !test_and_set_bit(FLUSHING_CACHED_CHARGE,
-				      &memory_st->flags)) {
-			if (cpu == curcpu)
-				drain_local_stock(&memory_st->work);
-			else if (!schedule_drain_work(cpu, &memory_st->work))
-				clear_bit(FLUSHING_CACHED_CHARGE,
-					  &memory_st->flags);
-		}
+		schedule_stock_drain(&memory_stock, root_css, cpu, curcpu);
+#ifdef CONFIG_MEMCG_V1
+		if (do_memsw_account())
+			schedule_stock_drain(&memsw_stock, root_css, cpu, curcpu);
+#endif
 
 		if (!test_bit(FLUSHING_CACHED_CHARGE, &obj_st->flags) &&
 		    obj_stock_flush_required(obj_st, root_memcg) &&
@@ -2235,6 +2252,11 @@ static int memcg_hotplug_cpu_dead(unsigned int cpu)
 	stock = per_cpu_ptr(&memory_stock, cpu);
 	page_counter_drain_stock_fully(stock);
 	clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags);
+#ifdef CONFIG_MEMCG_V1
+	stock = per_cpu_ptr(&memsw_stock, cpu);
+	page_counter_drain_stock_fully(stock);
+	clear_bit(FLUSHING_CACHED_CHARGE, &stock->flags);
+#endif
 
 	/*
 	 * A drain work queued before the CPU went away is executed by an
@@ -2557,8 +2579,8 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
 
 retry:
 	if (do_memsw_account() &&
-	    !page_counter_try_charge(&memcg->memsw, nr_pages, &counter, false,
-				     NULL)) {
+	    !page_counter_try_charge(&memcg->memsw, nr_pages, &counter,
+				     may_batch, NULL)) {
 		mem_over_limit = mem_cgroup_from_counter(counter, memsw);
 		goto reclaim;
 	}
@@ -3038,7 +3060,7 @@ static void obj_cgroup_uncharge_pages(struct obj_cgroup *objcg,
 	if (!mem_cgroup_is_root(memcg)) {
 		page_counter_refill_stock(&memcg->memory, nr_pages);
 		if (do_memsw_account())
-			page_counter_uncharge(&memcg->memsw, nr_pages);
+			page_counter_refill_stock(&memcg->memsw, nr_pages);
 	}
 
 	css_put(&memcg->css);
@@ -4190,6 +4212,10 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
 		memcg->memory.stock_css = &memcg->css;
 		page_counter_init(&memcg->swap, &parent->swap, false);
 #ifdef CONFIG_MEMCG_V1
+		if (!memcg_on_dfl) {
+			memcg->memsw.stock = &memsw_stock;
+			memcg->memsw.stock_css = &memcg->css;
+		}
 		WRITE_ONCE(memcg->swappiness, mem_cgroup_swappiness(parent));
 		memcg->memory.track_failcnt = !memcg_on_dfl;
 		memcg->memsw.track_failcnt = !memcg_on_dfl;
@@ -5818,6 +5844,10 @@ int __init mem_cgroup_init(void)
 	for_each_possible_cpu(cpu) {
 		INIT_WORK(&per_cpu_ptr(&memory_stock, cpu)->work,
 			  drain_local_stock);
+#ifdef CONFIG_MEMCG_V1
+		INIT_WORK(&per_cpu_ptr(&memsw_stock, cpu)->work,
+			  drain_local_stock);
+#endif
 		INIT_WORK(&per_cpu_ptr(&obj_stock, cpu)->work,
 			  drain_local_obj_stock);
 	}
-- 
2.53.0-Meta

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Michal Hocko]

[Sorry for coming to this really late]

On Wed 21-10-26 15:55:09, Joshua Hahn wrote:
> v6 --> v7
> =========
> - Preserve two cgroup v1 proprerties that previously followed implicitly
>   from sharing a stock:
>   - memory.usage_in_bytes <= memory.memsw.usage_in_bytes
>   - Reclaim does not swap when memory and memsw have equal limits.
> - Wordsmithing commit messages for 4/5 and 5/5 to document the
>   cgroupv1 divergence (bounded) between memory and memsw caused by
>   independent stock.
> 
> INTRODUCTION
> ============
> Memcg keeps a per-CPU stock of precharged pages so that small, frequent
> allocations do not walk the page_counter hierarchy every time.
> Today, the stock implementation is within memcontrol code, even though
> the operation it caches is a page_counter charge. This makes it
> difficult to add new page_counters to a memcg and preserve the fast
> path behavior.
> 
> This matters for future work like my tiered memcg limits series [1]
> which introduces multiple new page_counters to memcg. Without making
> stock a page_counter-level property, it means that every memcg charge
> now goes through multiple page_counter hierarchy walks, instead of
> being able to cache these charges.
> 
> To make future page_counters scalable and performant, move stock from
> mem_cgroup to page_counter so that each page_counter can opt into its
> own per-CPU cache of pre-charged pages.

What does it mean that each counter can opt into its own per-cpu
caching?

> We get an added benefit of simplifying try_charge_memcg(), which now
> has all the stock management handled transparently within the
> page_counter layer.

This is quite a large change and I believe it will be really benefitial
to have some high level design overview in the cover rather than trying to
chase it in respective patches.
Namely, are there any changes to the locking (which is arguably quite
tricky to allow completely lockless fast path)? Also what are
implications to caching at different levels of the hierarchy? An example
of cache propagation up/down the hierarchy during
charge/uncharge/draining would be really useful to wrap ones head around
how this is supposed to work.

> EFFECT ON MEMCG V2 USERS
> ========================
> This series has no functional changes intended for memcg v2 users. We
> preserve all draining, refilling, and (un)charging behavior, including
> the uncharge path's refills / direct uncharges.
> 
> EFFECT ON MEMCG V1 USERS
> ========================
> For memcg v1 users, the decoupling of the memsw and memory stock means
> that each of them now manage their own independent stocks and can lead
> to a different size of precharged cache for each.
> 
> Cgroup v1 has an invariant that memory.memsw.usage_in_bytes is larger
> than or equal to memory.usage_in_bytes, because memsw is a superset of
> memory. With separate stocking, this could have been broken in scenarios
> where the memory stock is bigger than the memsw stock, leading to
> memory usage appearing to be inflated and greater than memsw usage,
> even though the real usage preserves the invariant.

How can this happen when counters are always charged together?

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn]

On Mon, 26 Oct 2026 10:03:58 +0100 Michal Hocko <mhocko@suse.com> wrote:

> [Sorry for coming to this really late]

Hi Michal, no worries at all : -) Thanks for taking a look!

> On Wed 21-10-26 15:55:09, Joshua Hahn wrote:
> > v6 --> v7
> > =========
> > - Preserve two cgroup v1 proprerties that previously followed implicitly
> >   from sharing a stock:
> >   - memory.usage_in_bytes <= memory.memsw.usage_in_bytes
> >   - Reclaim does not swap when memory and memsw have equal limits.
> > - Wordsmithing commit messages for 4/5 and 5/5 to document the
> >   cgroupv1 divergence (bounded) between memory and memsw caused by
> >   independent stock.
> > 
> > INTRODUCTION
> > ============
> > Memcg keeps a per-CPU stock of precharged pages so that small, frequent
> > allocations do not walk the page_counter hierarchy every time.
> > Today, the stock implementation is within memcontrol code, even though
> > the operation it caches is a page_counter charge. This makes it
> > difficult to add new page_counters to a memcg and preserve the fast
> > path behavior.
> > 
> > This matters for future work like my tiered memcg limits series [1]
> > which introduces multiple new page_counters to memcg. Without making
> > stock a page_counter-level property, it means that every memcg charge
> > now goes through multiple page_counter hierarchy walks, instead of
> > being able to cache these charges.
> > 
> > To make future page_counters scalable and performant, move stock from
> > mem_cgroup to page_counter so that each page_counter can opt into its
> > own per-CPU cache of pre-charged pages.
> 
> What does it mean that each counter can opt into its own per-cpu
> caching?

So right now for cgroupv2, there's kind of a 1-1 mapping between
page_counter and memcg. I say "kind of" because there still is a
page_counter for swap, but it takes a completely different charging
path (i.e. not try_charge_memcg).

This means that currently stocking charge for memcg is equivalent to
stocking charge for the memcg->memory page_counter.

However, one of the series that I'm currently working on [1] adds new
page_counters to struct memcg to track the usage of memory
per-memcg, per-tier. The goal is to (1) track how much memory of a
cgroup lives on which tier, and (2) to limit the usage so that a single
cgroup doesn't use up all the fast tier memory alone, and shares it
fairly with all the other cgroups on the host.

When adding a new per-tier page_counter to struct mem_cgroup that gets
charged for every memcg_charge, I cannot use the stock fastpath since
the stock does not distinguish where the memory resides. It would
essentialy boil down to this pattern:

try_charge_memcg
   \__ page_couner_try_charge(memcg->tier[0])
   \__ consume_stock()

Where every path still has to do a hierarchy charge to the tier it's
charging to, whereas this patch gets us:

try_charge_memcg
   \__ page_counter_try_charge(memcg->tier[0])
          \__ consume_stock(memcg->tier[0])
   \__ page_countertry_charge(memcg->memory)
          \__ consume_stock(memcg->memory)

So that we can avoid the hierarchy walk for most charges.

I'd be happy to submit a new cover letter which better explains this
idea!

> > We get an added benefit of simplifying try_charge_memcg(), which now
> > has all the stock management handled transparently within the
> > page_counter layer.
> 
> This is quite a large change and I believe it will be really benefitial
> to have some high level design overview in the cover rather than trying to
> chase it in respective patches.

Sounds good with me. I can add the call traces above which should
explain my thought process a bit more. Hopefully the problem I am
trying to solve is evident there.

> Namely, are there any changes to the locking (which is arguably quite
> tricky to allow completely lockless fast path)?

No changes to locking. In fact this series is mostly just moving code
from mm/memcontrol.c to mm/page_counter.c, everything is essentially
the same. 

> Also what are implications to caching at different levels of the
> hierarchy? An example of cache propagation up/down the hierarchy during
> charge/uncharge/draining would be really useful to wrap ones head around
> how this is supposed to work.

This series should not have any changes for cgroup v2, including when
and where the cache is stocked / drained in the hierarchy. I can
include a snippet on how stocking currently works to add a visual
explanation, if that helps too.

> > EFFECT ON MEMCG V2 USERS
> > ========================
> > This series has no functional changes intended for memcg v2 users. We
> > preserve all draining, refilling, and (un)charging behavior, including
> > the uncharge path's refills / direct uncharges.
> > 
> > EFFECT ON MEMCG V1 USERS
> > ========================
> > For memcg v1 users, the decoupling of the memsw and memory stock means
> > that each of them now manage their own independent stocks and can lead
> > to a different size of precharged cache for each.
> > 
> > Cgroup v1 has an invariant that memory.memsw.usage_in_bytes is larger
> > than or equal to memory.usage_in_bytes, because memsw is a superset of
> > memory. With separate stocking, this could have been broken in scenarios
> > where the memory stock is bigger than the memsw stock, leading to
> > memory usage appearing to be inflated and greater than memsw usage,
> > even though the real usage preserves the invariant.
> 
> How can this happen when counters are always charged together?

Yeah, so this part is a little bit annoying. Let's just look at a single
leaf cgroup for example, where the limit for memsw and memory are both
64 pages.

charge_memcg(64)
	raw memory: 64			raw memsw: 64
	reported memory: 64		reported memsw: 64
	memory stock: 0			memory stock: 0

swap out 32 pages
	raw memory: 32			raw memsw: 64
	reported memory: 32		reported memsw: 64
	memory stock: 0			memsw stock: 0

free 32 pages
	raw memory: 0			raw memsw: 32
	reported memory: 0		reported memsw: 32
	memory stock: 0			memsw stock: 0

charge_memcg(1)
	raw memory: 1			raw memsw: 33
	reported memory: 64		reported memsw: 33
	memory stock: 63		memsw stock: 0

Where the last charge_memcg() causes the inversion. memsw tries to
charge a batch of 64 to refill the stock, but fails since 32+64=96
puts the memsw over its max limit. So it falls back to direct charging
where it charges the page_counter directly, and ends up with 0 stock
and reporting a usage of 33.

On the other hand, memory will greedily refill the stock with 64 pages
and since usage is 0, the batch refill will succeed, and it will report
a usage of 64.

So the idea with reporting the minimum is to report the minimum of the
two reported values, we will just fuzzily report the stocked value.
IIRC usage_in_bytes has always been fuzzy anyways, due to how much
memory has been stocked.

Thanks again for taking a look, Michal! I hope you have a great day : -)
Joshua

> -- 
> Michal Hocko
> SUSE Labs

[1] https://lore.kernel.org/all/20260807202059.2620949-1-joshua.hahnjy@gmail.com/

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Michal Hocko]

On Mon 26-10-26 08:59:37, Joshua Hahn wrote:
> On Mon, 26 Oct 2026 10:03:58 +0100 Michal Hocko <mhocko@suse.com> wrote:
> 
> > [Sorry for coming to this really late]
> 
> Hi Michal, no worries at all : -) Thanks for taking a look!
> 
> > On Wed 21-10-26 15:55:09, Joshua Hahn wrote:
> > > v6 --> v7
> > > =========
> > > - Preserve two cgroup v1 proprerties that previously followed implicitly
> > >   from sharing a stock:
> > >   - memory.usage_in_bytes <= memory.memsw.usage_in_bytes
> > >   - Reclaim does not swap when memory and memsw have equal limits.
> > > - Wordsmithing commit messages for 4/5 and 5/5 to document the
> > >   cgroupv1 divergence (bounded) between memory and memsw caused by
> > >   independent stock.
> > > 
> > > INTRODUCTION
> > > ============
> > > Memcg keeps a per-CPU stock of precharged pages so that small, frequent
> > > allocations do not walk the page_counter hierarchy every time.
> > > Today, the stock implementation is within memcontrol code, even though
> > > the operation it caches is a page_counter charge. This makes it
> > > difficult to add new page_counters to a memcg and preserve the fast
> > > path behavior.
> > > 
> > > This matters for future work like my tiered memcg limits series [1]
> > > which introduces multiple new page_counters to memcg. Without making
> > > stock a page_counter-level property, it means that every memcg charge
> > > now goes through multiple page_counter hierarchy walks, instead of
> > > being able to cache these charges.
> > > 
> > > To make future page_counters scalable and performant, move stock from
> > > mem_cgroup to page_counter so that each page_counter can opt into its
> > > own per-CPU cache of pre-charged pages.
> > 
> > What does it mean that each counter can opt into its own per-cpu
> > caching?
> 
> So right now for cgroupv2, there's kind of a 1-1 mapping between
> page_counter and memcg. I say "kind of" because there still is a
> page_counter for swap, but it takes a completely different charging
> path (i.e. not try_charge_memcg).
> 
> This means that currently stocking charge for memcg is equivalent to
> stocking charge for the memcg->memory page_counter.

correct

> However, one of the series that I'm currently working on [1] adds new
> page_counters to struct memcg to track the usage of memory
> per-memcg, per-tier. The goal is to (1) track how much memory of a
> cgroup lives on which tier, and (2) to limit the usage so that a single
> cgroup doesn't use up all the fast tier memory alone, and shares it
> fairly with all the other cgroups on the host.

Right. I guess you have layed down your motivation quite well in your
original cover letter.

> When adding a new per-tier page_counter to struct mem_cgroup that gets
> charged for every memcg_charge, I cannot use the stock fastpath since
> the stock does not distinguish where the memory resides. It would
> essentialy boil down to this pattern:
> 
> try_charge_memcg
>    \__ page_couner_try_charge(memcg->tier[0])
>    \__ consume_stock()
> 
> Where every path still has to do a hierarchy charge to the tier it's
> charging to, whereas this patch gets us:
> 
> try_charge_memcg
>    \__ page_counter_try_charge(memcg->tier[0])
>           \__ consume_stock(memcg->tier[0])
>    \__ page_countertry_charge(memcg->memory)
>           \__ consume_stock(memcg->memory)
> 
> So that we can avoid the hierarchy walk for most charges.
> 
> I'd be happy to submit a new cover letter which better explains this
> idea!

I guess I still do not understand what you mean by opting-in to the
caching. I believe you should focus less about what you plan to do and
rather explain what _this_ patchset achieves and how it works.
Also focus on describing why do we need to keep caches on all layers of
the hierarchy.

> > > We get an added benefit of simplifying try_charge_memcg(), which now
> > > has all the stock management handled transparently within the
> > > page_counter layer.
> > 
> > This is quite a large change and I believe it will be really benefitial
> > to have some high level design overview in the cover rather than trying to
> > chase it in respective patches.
> 
> Sounds good with me. I can add the call traces above which should
> explain my thought process a bit more. Hopefully the problem I am
> trying to solve is evident there.

I believe I follow where you are heading. I am not sure this is the
right path but that is a separate discussion. I would focus on whether
moving cache to the counter makes sense on its own (without that future
work) at this stage. Quite honestly I am still not convinced this move
is the right thing but I would like to fully understand the mechanics
first. Having discrepancies among different counters just sounds like a
potential source of future problems.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn]

On Mon, 2 Nov 2026 13:25:28 +0100 Michal Hocko <mhocko@suse.com> wrote:

> On Mon 26-10-26 08:59:37, Joshua Hahn wrote:
> > > > To make future page_counters scalable and performant, move stock from
> > > > mem_cgroup to page_counter so that each page_counter can opt into its
> > > > own per-CPU cache of pre-charged pages.
> > > 
> > > What does it mean that each counter can opt into its own per-cpu
> > > caching?
> > 
> > So right now for cgroupv2, there's kind of a 1-1 mapping between
> > page_counter and memcg. I say "kind of" because there still is a
> > page_counter for swap, but it takes a completely different charging
> > path (i.e. not try_charge_memcg).
> > 
> > This means that currently stocking charge for memcg is equivalent to
> > stocking charge for the memcg->memory page_counter.
> 
> correct
> 
> > However, one of the series that I'm currently working on [1] adds new
> > page_counters to struct memcg to track the usage of memory
> > per-memcg, per-tier. The goal is to (1) track how much memory of a
> > cgroup lives on which tier, and (2) to limit the usage so that a single
> > cgroup doesn't use up all the fast tier memory alone, and shares it
> > fairly with all the other cgroups on the host.
> 
> Right. I guess you have layed down your motivation quite well in your
> original cover letter.

Thanks, Michal!

> > When adding a new per-tier page_counter to struct mem_cgroup that gets
> > charged for every memcg_charge, I cannot use the stock fastpath since
> > the stock does not distinguish where the memory resides. It would
> > essentialy boil down to this pattern:
> > 
> > try_charge_memcg
> >    \__ page_couner_try_charge(memcg->tier[0])
> >    \__ consume_stock()
> > 
> > Where every path still has to do a hierarchy charge to the tier it's
> > charging to, whereas this patch gets us:
> > 
> > try_charge_memcg
> >    \__ page_counter_try_charge(memcg->tier[0])
> >           \__ consume_stock(memcg->tier[0])
> >    \__ page_countertry_charge(memcg->memory)
> >           \__ consume_stock(memcg->memory)
> > 
> > So that we can avoid the hierarchy walk for most charges.
> > 
> > I'd be happy to submit a new cover letter which better explains this
> > idea!
> 
> I guess I still do not understand what you mean by opting-in to the
> caching.

Reading my original cover letter again, yeah. It does feel a bit strange
to call it "opt-in"... I guess I just meant that not all page_coutners
will be getting it, it would only really be the memory page_counter
(and memsw page_counter for cgroup v2). I'll change the wording in the
next version.

> I believe you should focus less about what you plan to do and
> rather explain what _this_ patchset achieves and how it works.
> Also focus on describing why do we need to keep caches on all layers of
> the hierarchy.

Yes, I'll do that. Actually, this patchset is really quite limited in
what it does. I actually explicitly tried to do no optimizations or
behavioral changes at all, this is mostly just a clean-cut code
move from memcg code to page_counter code. 

You may recall earlier versions of this series did a bunch more stuff
(making the stock scale past 7 memcgs, dynamically allocating the stock,
at some point I even had a version that only cached leaf cgroups, etc.)

I got some feedback that these optimizations were orthogonal to the
movement of stock from memcg to page_counter, which is why this version
explicitly does none of the fancy optimizations on top. It mostly is
just moving code from memcg to page_counter.

> > > > We get an added benefit of simplifying try_charge_memcg(), which now
> > > > has all the stock management handled transparently within the
> > > > page_counter layer.
> > > 
> > > This is quite a large change and I believe it will be really benefitial
> > > to have some high level design overview in the cover rather than trying to
> > > chase it in respective patches.
> > 
> > Sounds good with me. I can add the call traces above which should
> > explain my thought process a bit more. Hopefully the problem I am
> > trying to solve is evident there.
> 
> I believe I follow where you are heading. I am not sure this is the
> right path but that is a separate discussion. I would focus on whether
> moving cache to the counter makes sense on its own (without that future
> work) at this stage.

If I'm being honest, most of the generic benefits that exist beyond that
work were in the other part of this series that I got rid of to scale
down the scope of this version. But to explicitly name the generic
benefits, it would be unbounded memcg stock caching per-cpu, eliminating
random evictions, (like you mentioned above as well) limiting stocks
to leaf memcgs only, etc.

I think there was a proposal to make the number of memcg stocks per cpu
even lower than it already is, because the author wanted to include
additional fields into the stock that would make 7 no longer fit into
a single cacheline. I feel like we are reaching some limits with that
side as well, so the "unbounded" portion seems to be quite opportune.

But I believe those benefits that we would like to see are made better
by this series, which makes the stocking transparent such that the new
optimizations don't make the existing memcg charging code any more
complex than it is. By abstracting away this mechanism, we can make sure
that memcg just handles the logical charging semantics, and page_counter
handles the implementation-level details of stock management.

Personally I am a big fan of how try_charge_memcg looks after this
series, what do you think? 

> Quite honestly I am still not convinced this move
> is the right thing but I would like to fully understand the mechanics
> first. Having discrepancies among different counters just sounds like a
> potential source of future problems.

I hope that I can present a good argument for why this is not as
concern as it may seem initially.

My first point is that the memory usage counters were always fuzzy, as
specified in the memcg v1 documentation too: 

usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
usage, it's a fuzz value for efficient access.

Secondly, the raw value of the usage is actually always accurate. It is
just the stock portion that overcharge the memory at times, making it
seem like there is a discrepancy. Flushing the memcg stock immediately
brings the counters to their real values (like what happens right now).

Finally, this is a change that is only limited to cgroup v1 users.
This entire series actually has no effect at all on cgroup v2 users,
which I think is a nice perk. Obviously I'm not out here trying to
make cgroup v1 unusable, but I wonder if anyone is still using the
legacy feature.

I hope this wasn't too much of a brain dump on you. Thanks for your
feedback Michal. Please let me know if there is anything I can do to
clarify my intent.

Would you prefer if we continud the discussion here, or if I sent out
a new series with just the cover letter changed? (I don't anticipate
any code changes, based on sashiko review or anything else that I see).

Thanks again. I hope you have a great day!
Joshua

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Michal Hocko]

On Mon 02-11-26 07:20:17, Joshua Hahn wrote:
> On Mon, 2 Nov 2026 13:25:28 +0100 Michal Hocko <mhocko@suse.com> wrote:
> 
> > On Mon 26-10-26 08:59:37, Joshua Hahn wrote:
> > I guess I still do not understand what you mean by opting-in to the
> > caching.
> 
> Reading my original cover letter again, yeah. It does feel a bit strange
> to call it "opt-in"... I guess I just meant that not all page_coutners
> will be getting it, it would only really be the memory page_counter
> (and memsw page_counter for cgroup v2). I'll change the wording in the
> next version.

OK, that makes it clear to me now.

[...]
> > > > > We get an added benefit of simplifying try_charge_memcg(), which now
> > > > > has all the stock management handled transparently within the
> > > > > page_counter layer.
> > > > 
> > > > This is quite a large change and I believe it will be really benefitial
> > > > to have some high level design overview in the cover rather than trying to
> > > > chase it in respective patches.
> > > 
> > > Sounds good with me. I can add the call traces above which should
> > > explain my thought process a bit more. Hopefully the problem I am
> > > trying to solve is evident there.
> > 
> > I believe I follow where you are heading. I am not sure this is the
> > right path but that is a separate discussion. I would focus on whether
> > moving cache to the counter makes sense on its own (without that future
> > work) at this stage.
> 
> If I'm being honest, most of the generic benefits that exist beyond that
> work were in the other part of this series that I got rid of to scale
> down the scope of this version. But to explicitly name the generic
> benefits, it would be unbounded memcg stock caching per-cpu, eliminating
> random evictions, (like you mentioned above as well) limiting stocks
> to leaf memcgs only, etc.

limiting cache to leaf memcgs makes a lot of sense and addresses a big
concern I had. How exactly does this work?
 
> I think there was a proposal to make the number of memcg stocks per cpu
> even lower than it already is, because the author wanted to include
> additional fields into the stock that would make 7 no longer fit into
> a single cacheline. I feel like we are reaching some limits with that
> side as well, so the "unbounded" portion seems to be quite opportune.
> 
> But I believe those benefits that we would like to see are made better
> by this series, which makes the stocking transparent such that the new
> optimizations don't make the existing memcg charging code any more
> complex than it is. By abstracting away this mechanism, we can make sure
> that memcg just handles the logical charging semantics, and page_counter
> handles the implementation-level details of stock management.

Yes, these are all good arguments. Especially when clarified that this
leaf only feature. I have to say I was struggling to see how caching on
all levels of the hierarchy would work in practice.

> Personally I am a big fan of how try_charge_memcg looks after this
> series, what do you think? 
> 
> > Quite honestly I am still not convinced this move
> > is the right thing but I would like to fully understand the mechanics
> > first. Having discrepancies among different counters just sounds like a
> > potential source of future problems.
> 
> I hope that I can present a good argument for why this is not as
> concern as it may seem initially.
> 
> My first point is that the memory usage counters were always fuzzy, as
> specified in the memcg v1 documentation too: 
> 
> usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> usage, it's a fuzz value for efficient access.
> 
> Secondly, the raw value of the usage is actually always accurate. It is
> just the stock portion that overcharge the memory at times, making it
> seem like there is a discrepancy. Flushing the memcg stock immediately
> bring the counters to their real values (like what happens right now).
> 
> Finally, this is a change that is only limited to cgroup v1 users.

Right but is there any actual point in adding caching for memsw charges
now? If you have a way to opt-in to caching then you can simply disabled
that for memsw counter altogether. Fewer problems to deal with, explain
etc...

> This entire series actually has no effect at all on cgroup v2 users,
> which I think is a nice perk. Obviously I'm not out here trying to
> make cgroup v1 unusable, but I wonder if anyone is still using the
> legacy feature.
> 
> I hope this wasn't too much of a brain dump on you. Thanks for your
> feedback Michal. Please let me know if there is anything I can do to
> clarify my intent.
> 
> Would you prefer if we continud the discussion here, or if I sent out
> a new series with just the cover letter changed? (I don't anticipate
> any code changes, based on sashiko review or anything else that I see).

Let's just iron out things first.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn]

On Mon, 2 Nov 2026 17:16:12 +0100 Michal Hocko <mhocko@suse.com> wrote:

> > > > > > We get an added benefit of simplifying try_charge_memcg(), which now
> > > > > > has all the stock management handled transparently within the
> > > > > > page_counter layer.
> > > > > 
> > > > > This is quite a large change and I believe it will be really benefitial
> > > > > to have some high level design overview in the cover rather than trying to
> > > > > chase it in respective patches.
> > > > 
> > > > Sounds good with me. I can add the call traces above which should
> > > > explain my thought process a bit more. Hopefully the problem I am
> > > > trying to solve is evident there.
> > > 
> > > I believe I follow where you are heading. I am not sure this is the
> > > right path but that is a separate discussion. I would focus on whether
> > > moving cache to the counter makes sense on its own (without that future
> > > work) at this stage.
> > 
> > If I'm being honest, most of the generic benefits that exist beyond that
> > work were in the other part of this series that I got rid of to scale
> > down the scope of this version. But to explicitly name the generic
> > benefits, it would be unbounded memcg stock caching per-cpu, eliminating
> > random evictions, (like you mentioned above as well) limiting stocks
> > to leaf memcgs only, etc.
> 
> limiting cache to leaf memcgs makes a lot of sense and addresses a big
> concern I had. How exactly does this work?

It's not a big change, actually. This was the implementation from the
origianl RFC of the series:

https://lore.kernel.org/all/20260410210742.550489-8-joshua.hahnjy@gmail.com/

The idea is that you treat a cgroup as a leaf until it becomes a parent,
at which point you drain the stock. Stock can land on the parent due
to hierarchy walks, but I can also write another part to this that
prevents stock from getting cached when a cgroup has children.

> > I think there was a proposal to make the number of memcg stocks per cpu
> > even lower than it already is, because the author wanted to include
> > additional fields into the stock that would make 7 no longer fit into
> > a single cacheline. I feel like we are reaching some limits with that
> > side as well, so the "unbounded" portion seems to be quite opportune.
> > 
> > But I believe those benefits that we would like to see are made better
> > by this series, which makes the stocking transparent such that the new
> > optimizations don't make the existing memcg charging code any more
> > complex than it is. By abstracting away this mechanism, we can make sure
> > that memcg just handles the logical charging semantics, and page_counter
> > handles the implementation-level details of stock management.
> 
> Yes, these are all good arguments. Especially when clarified that this
> leaf only feature. I have to say I was struggling to see how caching on
> all levels of the hierarchy would work in practice.

Sounds good. If it makes sense to you then, I can fold in the
leaf-only caching into this series to maintain more parity with the
existing code. How does that sound? 

> > Personally I am a big fan of how try_charge_memcg looks after this
> > series, what do you think? 
> > 
> > > Quite honestly I am still not convinced this move
> > > is the right thing but I would like to fully understand the mechanics
> > > first. Having discrepancies among different counters just sounds like a
> > > potential source of future problems.
> > 
> > I hope that I can present a good argument for why this is not as
> > concern as it may seem initially.
> > 
> > My first point is that the memory usage counters were always fuzzy, as
> > specified in the memcg v1 documentation too: 
> > 
> > usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> > usage, it's a fuzz value for efficient access.
> > 
> > Secondly, the raw value of the usage is actually always accurate. It is
> > just the stock portion that overcharge the memory at times, making it
> > seem like there is a discrepancy. Flushing the memcg stock immediately
> > bring the counters to their real values (like what happens right now).
> > 
> > Finally, this is a change that is only limited to cgroup v1 users.
> 
> Right but is there any actual point in adding caching for memsw charges
> now? If you have a way to opt-in to caching then you can simply disabled
> that for memsw counter altogether. Fewer problems to deal with, explain
> etc...

I think the problem actually still exists, unfortunately. In fact I
think the issue becomes bigger if you remove stocking for memsw.

Let's say only memory gets stock. Then on a single page charge for a
new cgroup:

memory page_counter charges 64 pages to refill the stock, memsw only
charges 1 page to fulfill the current allocation. Then you have
memory: 64 pages
memsw: 1 page

And the invariant that memory <= memsw is immediately broken.

Unfortunately only stocking for memsw is not a solution as well since
successful memsw charges still charge memory and will therefore go
through a hierarchy walk.

Actually maybe then, the solution is just to remove stocking for
cgroup v1 users? Does that sound like a fair tradeoff to you?

> > This entire series actually has no effect at all on cgroup v2 users,
> > which I think is a nice perk. Obviously I'm not out here trying to
> > make cgroup v1 unusable, but I wonder if anyone is still using the
> > legacy feature.
> > 
> > I hope this wasn't too much of a brain dump on you. Thanks for your
> > feedback Michal. Please let me know if there is anything I can do to
> > clarify my intent.
> > 
> > Would you prefer if we continud the discussion here, or if I sent out
> > a new series with just the cover letter changed? (I don't anticipate
> > any code changes, based on sashiko review or anything else that I see).
> 
> Let's just iron out things first.

Sounds good to me. Thanks!

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Michal Hocko]

On Mon 02-11-26 08:30:14, Joshua Hahn wrote:
> On Mon, 2 Nov 2026 17:16:12 +0100 Michal Hocko <mhocko@suse.com> wrote:
> 
> > > > > > > We get an added benefit of simplifying try_charge_memcg(), which now
> > > > > > > has all the stock management handled transparently within the
> > > > > > > page_counter layer.
> > > > > > 
> > > > > > This is quite a large change and I believe it will be really benefitial
> > > > > > to have some high level design overview in the cover rather than trying to
> > > > > > chase it in respective patches.
> > > > > 
> > > > > Sounds good with me. I can add the call traces above which should
> > > > > explain my thought process a bit more. Hopefully the problem I am
> > > > > trying to solve is evident there.
> > > > 
> > > > I believe I follow where you are heading. I am not sure this is the
> > > > right path but that is a separate discussion. I would focus on whether
> > > > moving cache to the counter makes sense on its own (without that future
> > > > work) at this stage.
> > > 
> > > If I'm being honest, most of the generic benefits that exist beyond that
> > > work were in the other part of this series that I got rid of to scale
> > > down the scope of this version. But to explicitly name the generic
> > > benefits, it would be unbounded memcg stock caching per-cpu, eliminating
> > > random evictions, (like you mentioned above as well) limiting stocks
> > > to leaf memcgs only, etc.
> > 
> > limiting cache to leaf memcgs makes a lot of sense and addresses a big
> > concern I had. How exactly does this work?
> 
> It's not a big change, actually. This was the implementation from the
> origianl RFC of the series:
> 
> https://lore.kernel.org/all/20260410210742.550489-8-joshua.hahnjy@gmail.com/
> 
> The idea is that you treat a cgroup as a leaf until it becomes a parent,
> at which point you drain the stock. Stock can land on the parent due
> to hierarchy walks, but I can also write another part to this that
> prevents stock from getting cached when a cgroup has children.

OK, makes sense.

> > > I think there was a proposal to make the number of memcg stocks per cpu
> > > even lower than it already is, because the author wanted to include
> > > additional fields into the stock that would make 7 no longer fit into
> > > a single cacheline. I feel like we are reaching some limits with that
> > > side as well, so the "unbounded" portion seems to be quite opportune.
> > > 
> > > But I believe those benefits that we would like to see are made better
> > > by this series, which makes the stocking transparent such that the new
> > > optimizations don't make the existing memcg charging code any more
> > > complex than it is. By abstracting away this mechanism, we can make sure
> > > that memcg just handles the logical charging semantics, and page_counter
> > > handles the implementation-level details of stock management.
> > 
> > Yes, these are all good arguments. Especially when clarified that this
> > leaf only feature. I have to say I was struggling to see how caching on
> > all levels of the hierarchy would work in practice.
> 
> Sounds good. If it makes sense to you then, I can fold in the
> leaf-only caching into this series to maintain more parity with the
> existing code. How does that sound? 

good

> > > Personally I am a big fan of how try_charge_memcg looks after this
> > > series, what do you think? 
> > > 
> > > > Quite honestly I am still not convinced this move
> > > > is the right thing but I would like to fully understand the mechanics
> > > > first. Having discrepancies among different counters just sounds like a
> > > > potential source of future problems.
> > > 
> > > I hope that I can present a good argument for why this is not as
> > > concern as it may seem initially.
> > > 
> > > My first point is that the memory usage counters were always fuzzy, as
> > > specified in the memcg v1 documentation too: 
> > > 
> > > usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> > > usage, it's a fuzz value for efficient access.
> > > 
> > > Secondly, the raw value of the usage is actually always accurate. It is
> > > just the stock portion that overcharge the memory at times, making it
> > > seem like there is a discrepancy. Flushing the memcg stock immediately
> > > bring the counters to their real values (like what happens right now).
> > > 
> > > Finally, this is a change that is only limited to cgroup v1 users.
> > 
> > Right but is there any actual point in adding caching for memsw charges
> > now? If you have a way to opt-in to caching then you can simply disabled
> > that for memsw counter altogether. Fewer problems to deal with, explain
> > etc...
> 
> I think the problem actually still exists, unfortunately. In fact I
> think the issue becomes bigger if you remove stocking for memsw.

Yes, you are right. We do actually cache memsw through the charging path
which charges both mem and memsw with the batch. 
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn]

> > > > Personally I am a big fan of how try_charge_memcg looks after this
> > > > series, what do you think?
> > > >
> > > > > Quite honestly I am still not convinced this move
> > > > > is the right thing but I would like to fully understand the mechanics
> > > > > first. Having discrepancies among different counters just sounds like a
> > > > > potential source of future problems.
> > > >
> > > > I hope that I can present a good argument for why this is not as
> > > > concern as it may seem initially.
> > > >
> > > > My first point is that the memory usage counters were always fuzzy, as
> > > > specified in the memcg v1 documentation too:
> > > >
> > > > usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> > > > usage, it's a fuzz value for efficient access.
> > > >
> > > > Secondly, the raw value of the usage is actually always accurate. It is
> > > > just the stock portion that overcharge the memory at times, making it
> > > > seem like there is a discrepancy. Flushing the memcg stock immediately
> > > > bring the counters to their real values (like what happens right now).
> > > >
> > > > Finally, this is a change that is only limited to cgroup v1 users.
> > >
> > > Right but is there any actual point in adding caching for memsw charges
> > > now? If you have a way to opt-in to caching then you can simply disabled
> > > that for memsw counter altogether. Fewer problems to deal with, explain
> > > etc...
> >
> > I think the problem actually still exists, unfortunately. In fact I
> > think the issue becomes bigger if you remove stocking for memsw.
>
> Yes, you are right. We do actually cache memsw through the charging path
> which charges both mem and memsw with the batch.

Thanks, Michal! So it seems like we are in alignment on everything
except the cgroup v1 implications for fuzziness. FWIW I think nothing
really changes from the userspace POV. memsw and memory usage has
always been fuzzy due to stocking, so all this series does is to
change "why" and "when" the stocking becomes fuzzy.

What do you think about this? The other alternative would be to remove
stocking for cgroup v1 users completely. But I am not sure that is a
good solution either. For the record, I tried making the stock tied
together so that they refill together, but that was a nightmare in
terms of added complexity.

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Michal Hocko]

On Mon 02-11-26 12:40:33, Joshua Hahn wrote:
> > > > > Personally I am a big fan of how try_charge_memcg looks after this
> > > > > series, what do you think?
> > > > >
> > > > > > Quite honestly I am still not convinced this move
> > > > > > is the right thing but I would like to fully understand the mechanics
> > > > > > first. Having discrepancies among different counters just sounds like a
> > > > > > potential source of future problems.
> > > > >
> > > > > I hope that I can present a good argument for why this is not as
> > > > > concern as it may seem initially.
> > > > >
> > > > > My first point is that the memory usage counters were always fuzzy, as
> > > > > specified in the memcg v1 documentation too:
> > > > >
> > > > > usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> > > > > usage, it's a fuzz value for efficient access.
> > > > >
> > > > > Secondly, the raw value of the usage is actually always accurate. It is
> > > > > just the stock portion that overcharge the memory at times, making it
> > > > > seem like there is a discrepancy. Flushing the memcg stock immediately
> > > > > bring the counters to their real values (like what happens right now).
> > > > >
> > > > > Finally, this is a change that is only limited to cgroup v1 users.
> > > >
> > > > Right but is there any actual point in adding caching for memsw charges
> > > > now? If you have a way to opt-in to caching then you can simply disabled
> > > > that for memsw counter altogether. Fewer problems to deal with, explain
> > > > etc...
> > >
> > > I think the problem actually still exists, unfortunately. In fact I
> > > think the issue becomes bigger if you remove stocking for memsw.
> >
> > Yes, you are right. We do actually cache memsw through the charging path
> > which charges both mem and memsw with the batch.
> 
> Thanks, Michal! So it seems like we are in alignment on everything
> except the cgroup v1 implications for fuzziness. FWIW I think nothing
> really changes from the userspace POV. memsw and memory usage has
> always been fuzzy due to stocking, so all this series does is to
> change "why" and "when" the stocking becomes fuzzy.

Right. The discrepancy was not so obvious but I agree that this change
doesn't fundamentally change the picture.

> What do you think about this? The other alternative would be to remove
> stocking for cgroup v1 users completely. But I am not sure that is a
> good solution either. For the record, I tried making the stock tied
> together so that they refill together, but that was a nightmare in
> terms of added complexity.

Nah, that seems overreaching without much added value. While we do not
support new features in v1 we shouldn't cripple performance for those
users unless there is no other way around.

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v7 0/5] mm/page_counter: move stock from mem_cgroup to page_counter

[Joshua Hahn]

On Mon, Nov 2, 2026 at 1:11 PM Michal Hocko <mhocko@suse.com> wrote:
>
> On Mon 02-11-26 12:40:33, Joshua Hahn wrote:
> > > > > > Personally I am a big fan of how try_charge_memcg looks after this
> > > > > > series, what do you think?
> > > > > >
> > > > > > > Quite honestly I am still not convinced this move
> > > > > > > is the right thing but I would like to fully understand the mechanics
> > > > > > > first. Having discrepancies among different counters just sounds like a
> > > > > > > potential source of future problems.
> > > > > >
> > > > > > I hope that I can present a good argument for why this is not as
> > > > > > concern as it may seem initially.
> > > > > >
> > > > > > My first point is that the memory usage counters were always fuzzy, as
> > > > > > specified in the memcg v1 documentation too:
> > > > > >
> > > > > > usage_in_bytes ... doesn't show 'exact' value of memory (and swap)
> > > > > > usage, it's a fuzz value for efficient access.
> > > > > >
> > > > > > Secondly, the raw value of the usage is actually always accurate. It is
> > > > > > just the stock portion that overcharge the memory at times, making it
> > > > > > seem like there is a discrepancy. Flushing the memcg stock immediately
> > > > > > bring the counters to their real values (like what happens right now).
> > > > > >
> > > > > > Finally, this is a change that is only limited to cgroup v1 users.
> > > > >
> > > > > Right but is there any actual point in adding caching for memsw charges
> > > > > now? If you have a way to opt-in to caching then you can simply disabled
> > > > > that for memsw counter altogether. Fewer problems to deal with, explain
> > > > > etc...
> > > >
> > > > I think the problem actually still exists, unfortunately. In fact I
> > > > think the issue becomes bigger if you remove stocking for memsw.
> > >
> > > Yes, you are right. We do actually cache memsw through the charging path
> > > which charges both mem and memsw with the batch.
> >
> > Thanks, Michal! So it seems like we are in alignment on everything
> > except the cgroup v1 implications for fuzziness. FWIW I think nothing
> > really changes from the userspace POV. memsw and memory usage has
> > always been fuzzy due to stocking, so all this series does is to
> > change "why" and "when" the stocking becomes fuzzy.
>
> Right. The discrepancy was not so obvious but I agree that this change
> doesn't fundamentally change the picture.

Thanks!

> > What do you think about this? The other alternative would be to remove
> > stocking for cgroup v1 users completely. But I am not sure that is a
> > good solution either. For the record, I tried making the stock tied
> > together so that they refill together, but that was a nightmare in
> > terms of added complexity.
>
> Nah, that seems overreaching without much added value. While we do not
> support new features in v1 we shouldn't cripple performance for those
> users unless there is no other way around.

Sounds good with me, and I also agree here. OK! I think it seems like
we are in alignment with everything. Let me spin up a new cover letter
being more explicit about everything we've covered here and send it
out soon. This was really helpful for me, thank you for your time
Michal : -)

Have a great day!!
Joshua