Re: [PATCH v2 7/7] mm: switch deferred split shrinker to list_lru

Johannes Weiner
2026-03-20T16:02:50Z

On Thu, Mar 19, 2026 at 08:21:21AM +0100, David Hildenbrand (Arm) wrote:
> >>> @@ -3802,33 +3706,28 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
> >>> struct folio *new_folio, *next;
> >>> int old_order = folio_order(folio);
> >>> int ret = 0;
> >>> - struct deferred_split *ds_queue;
> >>> + struct list_lru_one *l;
> >>>
> >>> VM_WARN_ON_ONCE(!mapping && end);
> >>> /* Prevent deferred_split_scan() touching ->_refcount */
> >>> - ds_queue = folio_split_queue_lock(folio);
> >>> + rcu_read_lock();
> >>
> >> The RCU lock is for the folio_memcg(), right?
> >>
> >> I recall I raised in the past that some get/put-like logic (that wraps
> >> the rcu_read_lock() + folio_memcg()) might make this a lot easier to get.
> >>
> >>
> >> memcg = folio_memcg_lookup(folio)
> >>
> >> ... do stuff
> >>
> >> folio_memcg_putback(folio, memcg);
> >>
> >> Or sth like that.
> >>
> >>
> >> Alternativey, you could have some helpers that do the
> >> list_lru_lock+unlock etc.
> >>
> >> folio_memcg_list_lru_lock()
> >> ...
> >> folio_memcg_list_ru_unlock(l);
> >>
> >> Just some thoughts as inspiration :)
> >
> > I remember you raising this in the objcg + reparenting patches. There
> > are a few more instances of
> >
> > rcu_read_lock()
> > foo = folio_memcg()
> > ...
> > rcu_read_unlock()
> >
> > in other parts of the code not touched by these patches here, so the
> > first pattern is a more universal encapsulation.
> >
> > Let me look into this. Would you be okay with a follow-up that covers
> > the others as well?
>
> Of course :) If list_lru lock helpers would be the right thing to do, it
> might be better placed in this series.

I'm playing around with the below. But there are a few things that
seem suboptimal:

- We need a local @memcg, which makes sites that just pass
folio_memcg() somewhere else fatter. More site LOC on average.
- Despite being more verbose, it communicates less. rcu_read_lock()
is universally understood, folio_memcg_foo() is cryptic.
- It doesn't cover similar accessors with the same lifetime rules,
like folio_lruvec(), folio_memcg_check()

include/linux/memcontrol.h | 35 ++++++++++++++++++++++++++---------
mm/huge_memory.c | 34 ++++++++++++++++++----------------
mm/list_lru.c | 5 ++---
mm/memcontrol.c | 17 +++++++----------
mm/migrate.c | 5 ++---
mm/page_io.c | 12 ++++++------
mm/vmscan.c | 7 ++++---
mm/workingset.c | 5 ++---
mm/zswap.c | 11 ++++++-----
9 files changed, 73 insertions(+), 58 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 0782c72a1997..5162145b9322 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -430,6 +430,17 @@ static inline struct mem_cgroup *folio_memcg(struct folio *folio)
return objcg ? obj_cgroup_memcg(objcg) : NULL;
}

+static inline struct mem_cgroup *folio_memcg_begin(struct folio *folio)
+{
+ rcu_read_lock();
+ return folio_memcg(folio);
+}
+
+static inline void folio_memcg_end(void)
+{
+ rcu_read_unlock();
+}
+
/*
* folio_memcg_charged - If a folio is charged to a memory cgroup.
* @folio: Pointer to the folio.
@@ -917,11 +928,10 @@ static inline void mod_memcg_page_state(struct page *page,
if (mem_cgroup_disabled())
return;

- rcu_read_lock();
- memcg = folio_memcg(page_folio(page));
+ memcg = folio_memcg_begin(page_folio(page));
if (memcg)
mod_memcg_state(memcg, idx, val);
- rcu_read_unlock();
+ folio_memcg_end();
}

unsigned long memcg_events(struct mem_cgroup *memcg, int event);
@@ -949,10 +959,9 @@ static inline void count_memcg_folio_events(struct folio *folio,
if (!folio_memcg_charged(folio))
return;

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
count_memcg_events(memcg, idx, nr);
- rcu_read_unlock();
+ folio_memcg_end();
}

static inline void count_memcg_events_mm(struct mm_struct *mm,
@@ -1035,6 +1044,15 @@ static inline struct mem_cgroup *folio_memcg(struct folio *folio)
return NULL;
}

+static inline struct mem_cgroup *folio_memcg_begin(struct folio *folio)
+{
+ return NULL;
+}
+
+static inline void folio_memcg_end(void)
+{
+}
+
static inline bool folio_memcg_charged(struct folio *folio)
{
return false;
@@ -1546,11 +1564,10 @@ static inline void mem_cgroup_track_foreign_dirty(struct folio *folio,
if (!folio_memcg_charged(folio))
return;

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
if (unlikely(&memcg->css != wb->memcg_css))
mem_cgroup_track_foreign_dirty_slowpath(folio, wb);
- rcu_read_unlock();
+ folio_memcg_end();
}

void mem_cgroup_flush_foreign(struct bdi_writeback *wb);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e90d08db219d..1aa20c1dd0c1 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3769,9 +3769,10 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
/* Prevent deferred_split_scan() touching ->_refcount */
dequeue_deferred = folio_test_anon(folio) && old_order > 1;
if (dequeue_deferred) {
- rcu_read_lock();
- l = list_lru_lock(&deferred_split_lru,
- folio_nid(folio), folio_memcg(folio));
+ struct mem_cgroup *memcg;
+
+ memcg = folio_memcg_begin(folio);
+ l = list_lru_lock(&deferred_split_lru, folio_nid(folio), memcg);
}
if (folio_ref_freeze(folio, folio_cache_ref_count(folio) + 1)) {
struct swap_cluster_info *ci = NULL;
@@ -3786,7 +3787,7 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
MTHP_STAT_NR_ANON_PARTIALLY_MAPPED, -1);
}
list_lru_unlock(l);
- rcu_read_unlock();
+ folio_memcg_end();
}

if (mapping) {
@@ -3891,7 +3892,7 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
} else {
if (dequeue_deferred) {
list_lru_unlock(l);
- rcu_read_unlock();
+ folio_memcg_end();
}
return -EAGAIN;
}
@@ -4272,12 +4273,13 @@ bool __folio_unqueue_deferred_split(struct folio *folio)
int nid = folio_nid(folio);
unsigned long flags;
bool unqueued = false;
+ struct mem_cgroup *memcg;

WARN_ON_ONCE(folio_ref_count(folio));
WARN_ON_ONCE(!mem_cgroup_disabled() && !folio_memcg_charged(folio));

- rcu_read_lock();
- l = list_lru_lock_irqsave(&deferred_split_lru, nid, folio_memcg(folio), &flags);
+ memcg = folio_memcg_begin(folio);
+ l = list_lru_lock_irqsave(&deferred_split_lru, nid, memcg, &flags);
if (__list_lru_del(&deferred_split_lru, l, &folio->_deferred_list, nid)) {
if (folio_test_partially_mapped(folio)) {
folio_clear_partially_mapped(folio);
@@ -4287,7 +4289,7 @@ bool __folio_unqueue_deferred_split(struct folio *folio)
unqueued = true;
}
list_lru_unlock_irqrestore(l, &flags);
- rcu_read_unlock();
+ folio_memcg_end();

return unqueued; /* useful for debug warnings */
}
@@ -4322,8 +4324,7 @@ void deferred_split_folio(struct folio *folio, bool partially_mapped)

nid = folio_nid(folio);

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
l = list_lru_lock_irqsave(&deferred_split_lru, nid, memcg, &flags);
if (partially_mapped) {
if (!folio_test_partially_mapped(folio)) {
@@ -4339,7 +4340,7 @@ void deferred_split_folio(struct folio *folio, bool partially_mapped)
}
__list_lru_add(&deferred_split_lru, l, &folio->_deferred_list, nid, memcg);
list_lru_unlock_irqrestore(l, &flags);
- rcu_read_unlock();
+ folio_memcg_end();
}

static unsigned long deferred_split_count(struct shrinker *shrink,
@@ -4445,16 +4446,17 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
* don't add it back to split_queue.
*/
if (!did_split && folio_test_partially_mapped(folio)) {
- rcu_read_lock();
+ struct mem_cgroup *memcg;
+
+ memcg = folio_memcg_begin(folio);
l = list_lru_lock_irqsave(&deferred_split_lru,
- folio_nid(folio),
- folio_memcg(folio),
+ folio_nid(folio), memcg,
&flags);
__list_lru_add(&deferred_split_lru, l,
&folio->_deferred_list,
- folio_nid(folio), folio_memcg(folio));
+ folio_nid(folio), memcg);
list_lru_unlock_irqrestore(l, &flags);
- rcu_read_unlock();
+ folio_memcg_end();
}
folio_put(folio);
}
diff --git a/mm/list_lru.c b/mm/list_lru.c
index 1ccdd45b1d14..638d084bb0f5 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -604,10 +604,9 @@ int folio_memcg_list_lru_alloc(struct folio *folio, struct list_lru *lru,
return 0;

/* Fast path when list_lru heads already exist */
- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
res = memcg_list_lru_allocated(memcg, lru);
- rcu_read_unlock();
+ folio_memcg_end();
if (likely(res))
return 0;

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f381cb6bdff1..14732f1542f2 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -965,18 +965,17 @@ void lruvec_stat_mod_folio(struct folio *folio, enum node_stat_item idx,
pg_data_t *pgdat = folio_pgdat(folio);
struct lruvec *lruvec;

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
/* Untracked pages have no memcg, no lruvec. Update only the node */
if (!memcg) {
- rcu_read_unlock();
+ folio_memcg_end();
mod_node_page_state(pgdat, idx, val);
return;
}

lruvec = mem_cgroup_lruvec(memcg, pgdat);
mod_lruvec_state(lruvec, idx, val);
- rcu_read_unlock();
+ folio_memcg_end();
}
EXPORT_SYMBOL(lruvec_stat_mod_folio);

@@ -1170,11 +1169,10 @@ struct mem_cgroup *get_mem_cgroup_from_folio(struct folio *folio)
if (!folio_memcg_charged(folio))
return root_mem_cgroup;

- rcu_read_lock();
do {
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
} while (unlikely(!css_tryget(&memcg->css)));
- rcu_read_unlock();
+ folio_memcg_end();
return memcg;
}

@@ -5535,8 +5533,7 @@ bool mem_cgroup_swap_full(struct folio *folio)
if (do_memsw_account() || !folio_memcg_charged(folio))
return ret;

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
for (; !mem_cgroup_is_root(memcg); memcg = parent_mem_cgroup(memcg)) {
unsigned long usage = page_counter_read(&memcg->swap);

@@ -5546,7 +5543,7 @@ bool mem_cgroup_swap_full(struct folio *folio)
break;
}
}
- rcu_read_unlock();
+ folio_memcg_end();

return ret;
}
diff --git a/mm/migrate.c b/mm/migrate.c
index fdbb20163f66..a2d542ebf3ed 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -672,8 +672,7 @@ static int __folio_migrate_mapping(struct address_space *mapping,
struct lruvec *old_lruvec, *new_lruvec;
struct mem_cgroup *memcg;

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
old_lruvec = mem_cgroup_lruvec(memcg, oldzone->zone_pgdat);
new_lruvec = mem_cgroup_lruvec(memcg, newzone->zone_pgdat);

@@ -700,7 +699,7 @@ static int __folio_migrate_mapping(struct address_space *mapping,
mod_lruvec_state(new_lruvec, NR_FILE_DIRTY, nr);
__mod_zone_page_state(newzone, NR_ZONE_WRITE_PENDING, nr);
}
- rcu_read_unlock();
+ folio_memcg_end();
}
local_irq_enable();

diff --git a/mm/page_io.c b/mm/page_io.c
index 63b262f4c5a9..862135a65848 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -239,6 +239,7 @@ static void swap_zeromap_folio_clear(struct folio *folio)
*/
int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
{
+ struct mem_cgroup *memcg;
int ret = 0;

if (folio_free_swap(folio))
@@ -277,13 +278,13 @@ int swap_writeout(struct folio *folio, struct swap_iocb **swap_plug)
goto out_unlock;
}

- rcu_read_lock();
- if (!mem_cgroup_zswap_writeback_enabled(folio_memcg(folio))) {
+ memcg = folio_memcg_begin(folio);
+ if (!mem_cgroup_zswap_writeback_enabled(memcg)) {
rcu_read_unlock();
folio_mark_dirty(folio);
return AOP_WRITEPAGE_ACTIVATE;
}
- rcu_read_unlock();
+ folio_memcg_end();

__swap_writepage(folio, swap_plug);
return 0;
@@ -314,11 +315,10 @@ static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio)
if (!folio_memcg_charged(folio))
return;

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
css = cgroup_e_css(memcg->css.cgroup, &io_cgrp_subsys);
bio_associate_blkg_from_css(bio, css);
- rcu_read_unlock();
+ folio_memcg_end();
}
#else
#define bio_associate_blkg_from_page(bio, folio) do { } while (0)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 33287ba4a500..12ad40fa7d60 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3407,6 +3407,7 @@ static struct folio *get_pfn_folio(unsigned long pfn, struct mem_cgroup *memcg,
struct pglist_data *pgdat)
{
struct folio *folio = pfn_folio(pfn);
+ struct mem_cgroup *this_memcg;

if (folio_lru_gen(folio) < 0)
return NULL;
@@ -3414,10 +3415,10 @@ static struct folio *get_pfn_folio(unsigned long pfn, struct mem_cgroup *memcg,
if (folio_nid(folio) != pgdat->node_id)
return NULL;

- rcu_read_lock();
- if (folio_memcg(folio) != memcg)
+ this_memcg = folio_memcg_begin(folio);
+ if (this_memcg != memcg)
folio = NULL;
- rcu_read_unlock();
+ folio_memcg_end();

return folio;
}
diff --git a/mm/workingset.c b/mm/workingset.c
index 07e6836d0502..77bfec58b797 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -251,8 +251,7 @@ static void *lru_gen_eviction(struct folio *folio)
BUILD_BUG_ON(LRU_GEN_WIDTH + LRU_REFS_WIDTH >
BITS_PER_LONG - max(EVICTION_SHIFT, EVICTION_SHIFT_ANON));

- rcu_read_lock();
- memcg = folio_memcg(folio);
+ memcg = folio_memcg_begin(folio);
lruvec = mem_cgroup_lruvec(memcg, pgdat);
lrugen = &lruvec->lrugen;
min_seq = READ_ONCE(lrugen->min_seq[type]);
@@ -261,7 +260,7 @@ static void *lru_gen_eviction(struct folio *folio)
hist = lru_hist_from_seq(min_seq);
atomic_long_add(delta, &lrugen->evicted[hist][type][tier]);
memcg_id = mem_cgroup_private_id(memcg);
- rcu_read_unlock();
+ folio_memcg_end();

return pack_shadow(memcg_id, pgdat, token, workingset, type);
}
diff --git a/mm/zswap.c b/mm/zswap.c
index 4f2e652e8ad3..fb035dd70d8b 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -895,14 +895,15 @@ static bool zswap_compress(struct page *page, struct zswap_entry *entry,
* to the active LRU list in the case.
*/
if (comp_ret || !dlen || dlen >= PAGE_SIZE) {
- rcu_read_lock();
- if (!mem_cgroup_zswap_writeback_enabled(
- folio_memcg(page_folio(page)))) {
- rcu_read_unlock();
+ struct mem_cgroup *memcg;
+
+ memcg = folio_memcg_begin(page_folio(page));
+ if (!mem_cgroup_zswap_writeback_enabled(memcg)) {
+ folio_memcg_end();
comp_ret = comp_ret ? comp_ret : -EINVAL;
goto unlock;
}
- rcu_read_unlock();
+ folio_memcg_end();
comp_ret = 0;
dlen = PAGE_SIZE;
dst = kmap_local_page(page);