* Re: [PATCH] mm/list_lru: drain before clearing xarray entry on reparent
From: Muchun Song @ 2026-06-01 9:54 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Johannes Weiner, Dave Chinner, Roman Gushchin,
Qi Zheng, Kairui Song, Meta kernel team, linux-mm, cgroups,
linux-kernel, Chris Mason
In-Reply-To: <20260601063408.2879011-1-shakeel.butt@linux.dev>
> On Jun 1, 2026, at 14:34, Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> memcg_reparent_list_lrus() clears the dying memcg's xarray entry with
> xas_store(&xas, NULL) before reparenting its per-node lists into the
> parent. This opens a window where a concurrent list_lru_del() arriving
> for the dying memcg sees xa_load() == NULL, walks to the parent in
> lock_list_lru_of_memcg(), takes the parent's per-node lock, and calls
> list_del_init() on an item still physically linked on the dying
> memcg's list.
>
> If another in-flight thread holds the dying memcg's per-node lock at
> the same moment (another list_lru_del, or a list_lru_walk_one running
> an isolate callback), both threads modify ->next/->prev pointers on the
> same physical list under different locks. Adjacent items can corrupt
> each other's links.
>
> Fix it by reversing the order: reparent each per-node list and mark the
> child's list lru dead and then clear the xarray entry. Any concurrent
> list_lru op that finds the still-set xarray entry either takes the dying
> memcg's per-node lock (synchronizing with the drain) or sees LONG_MIN
> and walks to the parent, where the items now live.
>
> Fixes: fb56fdf8b9a2 ("mm/list_lru: split the lock to per-cgroup scope")
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> Reported-by: Chris Mason <clm@fb.com>
> ---
> mm/list_lru.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index dd29bcf8eb5f..ae55a52307db 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -473,26 +473,24 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren
> mutex_lock(&list_lrus_mutex);
> list_for_each_entry(lru, &memcg_list_lrus, list) {
> struct list_lru_memcg *mlru;
> - XA_STATE(xas, &lru->xa, memcg->kmemcg_id);
>
> - /*
> - * Lock the Xarray to ensure no on going list_lru_memcg
> - * allocation and further allocation will see css_is_dying().
> - */
> - xas_lock_irq(&xas);
> - mlru = xas_store(&xas, NULL);
> - xas_unlock_irq(&xas);
> + mlru = xa_load(&lru->xa, memcg->kmemcg_id);
> if (!mlru)
> continue;
Is it possible that concurrent threads running memcg_list_lru_alloc() could
allocate a new mlru after this check passes? This could happen because the
threads haven't noticed css_is_dying() yet. We would consequently miss the
reparent operation for this list. So xas_lock_irq is necessary to serialize
CSS_DYING setting here. Right?
Thanks.
Muchun
>
> /*
> - * With Xarray value set to NULL, holding the lru lock below
> - * prevents list_lru_{add,del,isolate} from touching the lru,
> - * safe to reparent.
> + * Reparent each per-node list and mark the child dead
> + * (LONG_MIN) before clearing xarray entry otherwisw a
> + * concurrent list_lru_del() may corrupt the list if it arrives
> + * after xarray clear but before reparenting as
> + * lock_list_lru_of_memcg will acquire parent's lock while the
> + * item is still on child's list.
> */
> for_each_node(i)
> memcg_reparent_list_lru_one(lru, i, &mlru->node[i], parent);
>
> + xa_erase(&lru->xa, memcg->kmemcg_id);
> +
> /*
> * Here all list_lrus corresponding to the cgroup are guaranteed
> * to remain empty, we can safely free this lru, any further
> --
> 2.52.0
>
^ permalink raw reply
* Re: [PATCH v5 9/9] mm: switch deferred split shrinker to list_lru
From: Lance Yang @ 2026-06-01 10:39 UTC (permalink / raw)
To: hannes
Cc: akpm, david, ljs, shakeel.butt, mhocko, david, roman.gushchin,
muchun.song, qi.zheng, yosry.ahmed, ziy, liam, usama.arif, kas,
vbabka, ryncsn, zaslonko, gor, baolin.wang, baohua, dev.jain,
lance.yang, npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <20260527204757.2544958-10-hannes@cmpxchg.org>
On Wed, May 27, 2026 at 04:45:16PM -0400, Johannes Weiner wrote:
[...]
>diff --git a/mm/memory.c b/mm/memory.c
>index 135f5c0f57bd..f22e61d8c8de 100644
>--- a/mm/memory.c
>+++ b/mm/memory.c
>@@ -5222,6 +5222,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
> folio_put(folio);
> goto next;
> }
>+ if (order > 1 && folio_memcg_alloc_deferred(folio)) {
>+ folio_put(folio);
Missing a MTHP_STAT_ANON_FAULT_FALLBACK bump here?
Since we jump straight to fallback and end up at order-0 :)
>+ goto fallback;
>+ }
> folio_throttle_swaprate(folio, gfp);
> /*
> * When a folio is not zeroed during allocation
[...]
Cheers, Lance
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Hao Jia @ 2026-06-01 11:07 UTC (permalink / raw)
To: Yosry Ahmed
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <aho7nepN5jZtKmef@google.com>
On 2026/5/30 09:24, Yosry Ahmed wrote:
> On Tue, May 26, 2026 at 07:45:58PM +0800, Hao Jia wrote:
>> From: Hao Jia <jiahao1@lixiang.com>
>>
>> The zswap background writeback worker shrink_worker() uses a global
>> cursor zswap_next_shrink, protected by zswap_shrink_lock, to round-robin
>> across the online memcgs under root_mem_cgroup.
>>
>> Proactive writeback also wants a similar per-memcg cursor that is
>> scoped to the specified memcg, so that repeated invocations against
>> the same memcg make forward progress across its descendant memcgs
>> instead of restarting from the first child memcg each time.
>
> Is this a problem in practice?
>
> Is the concern the overhead of scanning memcgs repeatedly, or lack of
> fairness? I wonder if we should just do writeback in batches from all
> memcgs, similar to how reclaim does it, then evaluate at the end if we
> need to start over?
>
Not using a per-cgroup cursor will cause issues for "repeated
small-budget calls" cases. For example, repeatedly triggering a 2MB
writeback might result in only writing back pages from the first few
child memcgs every time. In the worst-case scenario (where the writeback
amount is less than WB_BATCH), it might only ever write back from the
first child memcg.
Similar to how memory reclaim uses mem_cgroup_iter() (via struct
mem_cgroup_reclaim_iter) and the old shrink_worker() used
zswap_next_shrink, we need a shared cursor here.
>>
>> Naturally, group the cursor and its protecting spinlock into a
>> zswap_wb_iter struct, and make it a member of struct mem_cgroup to
>> realize per-memcg cursor management. Accordingly, shrink_worker() now
>> uses the lock and cursor in root_mem_cgroup->zswap_wb_iter.
>
> If we really need to have per-memcg cursors (I am not a big fan), I
> think we can minimize the overhead by making the cursor updates use
> atomic cmpxchg instead of having a per-memcg lock.
>
Because mem_cgroup_iter() always calls css_put(&prev->css), we cannot
simply update zswap_wb_iter.pos via cmpxchg() after calling it. Doing so
could lead to a double css_put() issue on prev->css.
Therefore, if we switch to the cmpxchg() approach, we wouldn't be able
to reuse the existing mem_cgroup_iter() logic. We would have to write a
new function similar to cgroup_iter(), and its implementation might end
up looking a bit obscure/complex.
Currently, this lock is only used in shrink_memcg(), proactive
writeback, and mem_cgroup_css_offline(). Note that shrink_memcg() only
acquires the lock of the root cgroup, and mem_cgroup_css_offline() is
unlikely to be a hot path.
So, should we keep the spin_lock or go with the cmpxchg() approach?
Yosry and Nhat, what are your thoughts on this?
>>
>> Because the cursor is now per-memcg, the offline cleanup must visit
>> every ancestor that could be holding a reference to the dying memcg.
>> Factor out __zswap_memcg_offline_cleanup() and walk from dead_memcg up
>> to the root.
>
> Another reason why I don't like per-memcg cursors. There is too much
> complexity and I wonder if it's warranted. If we stick with per-memcg
> cursors please do the refactoring in separate patches to make the
> patches easier to review.
Sorry about that. I will try to keep each patch as simple as possible in
the next version.
Thanks,
Hao
^ permalink raw reply
* Re: [PATCH v5 9/9] mm: switch deferred split shrinker to list_lru
From: Lance Yang @ 2026-06-01 11:09 UTC (permalink / raw)
To: hannes
Cc: akpm, david, ljs, shakeel.butt, mhocko, david, roman.gushchin,
muchun.song, qi.zheng, yosry.ahmed, ziy, liam, usama.arif, kas,
vbabka, ryncsn, zaslonko, gor, baolin.wang, baohua, dev.jain,
npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <20260601103947.63923-1-lance.yang@linux.dev>
On 2026/6/1 18:39, Lance Yang wrote:
>
> On Wed, May 27, 2026 at 04:45:16PM -0400, Johannes Weiner wrote:
> [...]
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 135f5c0f57bd..f22e61d8c8de 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -5222,6 +5222,10 @@ static struct folio *alloc_anon_folio(struct vm_fault *vmf)
>> folio_put(folio);
>> goto next;
>> }
>> + if (order > 1 && folio_memcg_alloc_deferred(folio)) {
>> + folio_put(folio);
>
> Missing a MTHP_STAT_ANON_FAULT_FALLBACK bump here?
>
> Since we jump straight to fallback and end up at order-0 :)
Sorry for the noise, I missed the earlier discussion. Never mind this
one, please.
>
>> + goto fallback;
>> + }
>> folio_throttle_swaprate(folio, gfp);
>> /*
>> * When a folio is not zeroed during allocation
> [...]
>
> Cheers, Lance
^ permalink raw reply
* Re: [PATCH v5 9/9] mm: switch deferred split shrinker to list_lru
From: Lance Yang @ 2026-06-01 13:21 UTC (permalink / raw)
To: hannes, baolin.wang
Cc: akpm, david, ljs, shakeel.butt, mhocko, david, roman.gushchin,
muchun.song, qi.zheng, yosry.ahmed, ziy, liam, usama.arif, kas,
vbabka, ryncsn, zaslonko, gor, baohua, dev.jain, lance.yang,
npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <20260527204757.2544958-10-hannes@cmpxchg.org>
On Wed, May 27, 2026 at 04:45:16PM -0400, Johannes Weiner wrote:
[...]
>diff --git a/mm/swap_state.c b/mm/swap_state.c
>index 04f5ce992401..9c3a5cf99778 100644
>--- a/mm/swap_state.c
>+++ b/mm/swap_state.c
>@@ -465,6 +465,16 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
> return ERR_PTR(-ENOMEM);
> }
>
Shouldn't this be limited to anon swapin?
e.g. vmf && vma_is_anonymous(vmf->vma)
>+ if (order > 1 && folio_memcg_alloc_deferred(folio)) {
__swap_cache_alloc() is also used by shmem direct swapin, so shmem can
get here too when handling a large swap entry:
shmem_get_folio_gfp()
shmem_swapin_folio()
shmem_swap_alloc_folio()
swapin_sync()
swap_cache_alloc_folio()
__swap_cache_alloc()
folio_memcg_alloc_deferred()
@Baolin please correct me if I got it wrong :)
folio_memcg_alloc_deferred() itself doesn't filter shmem out either; it
only allocates the memcg list_lru metadata for deferred_split_lru:
int folio_memcg_alloc_deferred(struct folio *folio)
{
if (mem_cgroup_disabled())
return 0;
return folio_memcg_list_lru_alloc(folio, &deferred_split_lru, GFP_KERNEL);
}
Since deferred_split_lru only queues anon large folios, doing this for
shmem swapin doesn't buy us anything :)
Maybe I'm missing something, just wanted to raise it.
>+ spin_lock(&ci->lock);
>+ __swap_cache_do_del_folio(ci, folio, entry, shadow);
>+ spin_unlock(&ci->lock);
>+ folio_unlock(folio);
>+ /* nr_pages refs from swap cache, 1 from allocation */
>+ folio_put_refs(folio, nr_pages + 1);
>+ return ERR_PTR(-ENOMEM);
>+ }
>+
> /* memsw uncharges swap when folio is added to swap cache */
> memcg1_swapin(folio);
> if (shadow)
>--
>2.54.0
>
>
^ permalink raw reply
* Re: [PATCH] mm: don't allow empty relative nodemask in mpol_relative_nodemask()
From: David Hildenbrand (Arm) @ 2026-06-01 14:06 UTC (permalink / raw)
To: Yury Norov, Andrew Morton, Zi Yan, Matthew Brost, Joshua Hahn,
Rakie Kim, Byungchul Park, Gregory Price, Ying Huang,
Alistair Popple, linux-mm, linux-kernel
Cc: Farhad Alemi, Waiman Long, Rasmus Villemoes, cgroups
In-Reply-To: <20260528190337.878027-1-ynorov@nvidia.com>
On 5/28/26 21:03, Yury Norov wrote:
> Reassigning nodes relative an empty user-provided nodemask is useless,
> and triggers divide-by-zero in the function.
>
> Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
> Link: https://lore.kernel.org/all/CA+0ovCgxbZkXa+OU8w3s84R3KNPNxxRfmsNR-udh+afQBbGNmw@mail.gmail.com/
Likely this should be a
Closes:
And be accompanied by a Fixes: and Cc stable.
> Signed-off-by: Yury Norov <ynorov@nvidia.com>
> ---
> mm/mempolicy.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 4e4421b22b59..cd961fa1eb33 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -370,8 +370,13 @@ static inline int mpol_store_user_nodemask(const struct mempolicy *pol)
> static void mpol_relative_nodemask(nodemask_t *ret, const nodemask_t *orig,
> const nodemask_t *rel)
> {
Continuing the discussion of the context in the other thread :)
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] mm: don't allow empty relative nodemask in mpol_relative_nodemask()
From: David Hildenbrand (Arm) @ 2026-06-01 14:32 UTC (permalink / raw)
To: Yury Norov, Joshua Hahn
Cc: Andrew Morton, Zi Yan, Matthew Brost, Rakie Kim, Byungchul Park,
Gregory Price, Ying Huang, Alistair Popple, linux-mm,
linux-kernel, Farhad Alemi, Waiman Long, Rasmus Villemoes,
cgroups
In-Reply-To: <ahnRIDBk4bQ3xX2q@yury>
>>
>> Thank you for taking a shot at fixing the bug report, please let me know what
>> you think! Have a great day : -)
>
> Hi Joshua.
>
> Indeed, quick and dirty shot.
>
> The problem is that nodes_fold() can't work with the sz == 0. In
> other words, folding to a 0-bit bitmap is an error. We don't check
> that on bitmaps level because it's an internal helper, and it's a
> caller's responsibility to validate the parameters.
>
> nodes_onto(), or more specifically bitmap_onto(), is a different
> story. In case of empty relmap, the function actually clears all the
> bits in dst and returns.
It's very weird that mpol_new_nodemask() (->create() callback) disallows empty
nodemasks, but mpol_rebind_nodemask() (->rebind() callback) would allow empty
nodemasks.
I guess mpol_set_nodemask() could trigger it after doing the
nodes_and(nsc->mask1, cpuset_current_mems_allowed,
node_states[N_MEMORY]);
And ending with an empty &nsc->mask1.
The later "mpol_ops[pol->mode].create(pol, &nsc->mask2);" would reject it, but
the division by zero could still happen.
>
> I see 2 options to move this forward.
>
> 1. Simply disallow empty relmap in mpol_relative_nodemask(). There's
> no valid cases for it, AFAIK, so the nodes_fold() limitation looks
> reasonable. We can consider it as a new policy.
>
> We've got 2 users for mpol_relative_nodemask(). In mpol_set_nodemask()
> we can simply propagate the error; and in mpol_rebind_nodemask() we
> can throw a warning and do nothing.
Throwing a warning is really bad. We'd still end up with an empty nodemask?
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] mm/list_lru: drain before clearing xarray entry on reparent
From: Shakeel Butt @ 2026-06-01 15:38 UTC (permalink / raw)
To: Muchun Song
Cc: Andrew Morton, Johannes Weiner, Dave Chinner, Roman Gushchin,
Qi Zheng, Kairui Song, Meta kernel team, linux-mm, cgroups,
linux-kernel, Chris Mason
In-Reply-To: <79CD986A-2130-4FB8-804F-A543AF22342B@linux.dev>
Hi Muchun, thanks for taking a look.
On Mon, Jun 01, 2026 at 05:54:01PM +0800, Muchun Song wrote:
>
>
> > On Jun 1, 2026, at 14:34, Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >
> > memcg_reparent_list_lrus() clears the dying memcg's xarray entry with
> > xas_store(&xas, NULL) before reparenting its per-node lists into the
> > parent. This opens a window where a concurrent list_lru_del() arriving
> > for the dying memcg sees xa_load() == NULL, walks to the parent in
> > lock_list_lru_of_memcg(), takes the parent's per-node lock, and calls
> > list_del_init() on an item still physically linked on the dying
> > memcg's list.
> >
> > If another in-flight thread holds the dying memcg's per-node lock at
> > the same moment (another list_lru_del, or a list_lru_walk_one running
> > an isolate callback), both threads modify ->next/->prev pointers on the
> > same physical list under different locks. Adjacent items can corrupt
> > each other's links.
> >
> > Fix it by reversing the order: reparent each per-node list and mark the
> > child's list lru dead and then clear the xarray entry. Any concurrent
> > list_lru op that finds the still-set xarray entry either takes the dying
> > memcg's per-node lock (synchronizing with the drain) or sees LONG_MIN
> > and walks to the parent, where the items now live.
> >
> > Fixes: fb56fdf8b9a2 ("mm/list_lru: split the lock to per-cgroup scope")
> > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> > Reported-by: Chris Mason <clm@fb.com>
> > ---
> > mm/list_lru.c | 20 +++++++++-----------
> > 1 file changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/mm/list_lru.c b/mm/list_lru.c
> > index dd29bcf8eb5f..ae55a52307db 100644
> > --- a/mm/list_lru.c
> > +++ b/mm/list_lru.c
> > @@ -473,26 +473,24 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren
> > mutex_lock(&list_lrus_mutex);
> > list_for_each_entry(lru, &memcg_list_lrus, list) {
> > struct list_lru_memcg *mlru;
> > - XA_STATE(xas, &lru->xa, memcg->kmemcg_id);
> >
> > - /*
> > - * Lock the Xarray to ensure no on going list_lru_memcg
> > - * allocation and further allocation will see css_is_dying().
> > - */
> > - xas_lock_irq(&xas);
> > - mlru = xas_store(&xas, NULL);
> > - xas_unlock_irq(&xas);
> > + mlru = xa_load(&lru->xa, memcg->kmemcg_id);
> > if (!mlru)
> > continue;
>
> Is it possible that concurrent threads running memcg_list_lru_alloc() could
> allocate a new mlru after this check passes? This could happen because the
> threads haven't noticed css_is_dying() yet. We would consequently miss the
> reparent operation for this list. So xas_lock_irq is necessary to serialize
> CSS_DYING setting here. Right?
Good question and it seems like Sashiko [1] raised a similar concern. However
please note that memcg_list_lru_alloc() uses CSS_DYING when it allocate a new
mlru but memcg_reparent_list_lrus() is called from offlice_css() callback and
the given css should already have CSS_DYING before calling offline_css(). There
is a rcu grace period between setting CSS_DYING and calling offline_css().
[1] https://sashiko.dev/#/patchset/20260601063408.2879011-1-shakeel.butt%40linux.dev
>
> Thanks.
> Muchun
>
> >
> > /*
> > - * With Xarray value set to NULL, holding the lru lock below
> > - * prevents list_lru_{add,del,isolate} from touching the lru,
> > - * safe to reparent.
> > + * Reparent each per-node list and mark the child dead
> > + * (LONG_MIN) before clearing xarray entry otherwisw a
> > + * concurrent list_lru_del() may corrupt the list if it arrives
> > + * after xarray clear but before reparenting as
> > + * lock_list_lru_of_memcg will acquire parent's lock while the
> > + * item is still on child's list.
> > */
> > for_each_node(i)
> > memcg_reparent_list_lru_one(lru, i, &mlru->node[i], parent);
> >
> > + xa_erase(&lru->xa, memcg->kmemcg_id);
This one is more tricky. Sashiko said:
" Is it safe to use xa_erase() here instead of xa_erase_irq()?
The list_lru xarray is initialized with XA_FLAGS_LOCK_IRQ, and elements are
added holding the lock via xas_lock_irqsave(), which establishes an IRQ-safe
lock class.
Since xa_erase() internally calls spin_lock() without disabling local
interrupts, an interrupt firing while the lock is held could attempt to
re-acquire the same lock in __memcg_list_lru_alloc(), leading to a deadlock.
This could also trigger a lockdep warning for an inconsistent lock state. "
Initially I though this is a false positive as I couldn't find irq callers for
kmem_cache_alloc_lru() but then claude came up with more concrete scenario which
is below:
"""
For the shadow_nodes lru this lock is also acquired nested under the page
cache i_pages lock, which is irq-safe. Adding a folio holds i_pages and
then allocates an xarray node through the shadow_nodes lru:
__filemap_add_folio()
mapping_set_update(&xas, mapping) // xas->xa_lru = &shadow_nodes
xas_lock_irq(&xas) // holds mapping->i_pages
xas_store() -> xas_alloc()
kmem_cache_alloc_lru(radix_tree_node_cachep, xas->xa_lru, gfp)
memcg_list_lru_alloc(memcg, &shadow_nodes, gfp)
xas_lock_irqsave(&shadow_nodes->xa) // shadow_nodes->xa under i_pages
and i_pages is taken from writeback completion in irq context:
__folio_end_writeback()
xa_lock_irqsave(&mapping->i_pages, flags);
So with xa_erase() taking shadow_nodes->xa with irqs enabled:
CPU0 memcg_reparent_list_lrus() CPU1 __filemap_add_folio()
xa_erase(&shadow_nodes->xa)
xa_lock(&shadow_nodes->xa)
xas_lock_irq(&i_pages) // holds i_pages
... memcg_list_lru_alloc()
xas_lock_irqsave(&shadow_nodes->xa) // waits
<io completion irq on CPU0>
__folio_end_writeback()
xa_lock_irqsave(&i_pages) // waits
Can this deadlock, and should this be xa_erase_irq() to keep the irq-safe
acquisition that the removed xas_lock_irq() had?
"""
This seems more plausible and I think simply using xa_erase_irq() is more safe.
I will send a v2 with this change.
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-01 15:56 UTC (permalink / raw)
To: Kairui Song
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <ahz_iYG4lqWL4g-J@KASONG-MC4>
On Mon, Jun 1, 2026 at 12:34 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Thu, May 28, 2026 at 02:29:24PM +0800, Nhat Pham wrote:
> > Based on: mm-unstable @ 444fc9435e57 + swap-table phase IV v5 [2].
> >
> > I manually adapted Kairui's ghost device implementation (from [4])
> > for my vswap device. I've credited him as Co-developed-by on Patch I
> > since a substantial portion of the dynamic-cluster infrastructure is
> > his (I did propose the idea of using xarray/radix tree for dynamic
> > swap clusters allocation and management though :P).
> >
> > >From here on out, for simplicity, I will refer to swap table phase IV
> > as "P4", and the older v6 virtual swap space implementation as "v6".
> >
>
> ...
>
> >
> > This series reimplements the virtual swap space concept (see [1])
> > on top of Kairui Song's swap table infrastructure, on top of [2]
> > and in accordance with his proposal in [3]. The proposal's idea
> > is interesting, so I decided to give it a shot myself. I'm still not
> > 100% sure that this is bug-proof, but hey, it compiles, and has
> > not crashed in my simple stress testing :)
> >
> > The prototype here is feature-complete relative to the swap-table P4
> > baseline — swapout, swapin, freeing, swapoff, zswap writeback, zswap
> > shrinker, memcg charging, and THP swapin all work for
> > both vswap and direct-physical entries — and satisfies all three
> > requirements above: no backend coupling (zswap/zero entries hold no
> > physical slot), dynamic swap space (clusters allocated on demand via
> > xarray, no static provisioning), and efficient backend transfer
> > (in-place vtable updates, no PTE/rmap walking).
> >
> > II. Design
> >
> > With vswap, pages are assigned virtual swap entries on a ghost device
> > with no backing storage. These entries are backed by zswap, zero pages,
> > or (lazily) physical swap slots. Physical backing is allocated only
> > when needed — on zswap writeback or reclaim writeout, after the rmap
> > step.
> >
> > Compared to the standalone v6 implementation [1], which introduces a
> > 24-byte per-entry swap descriptor and its own cluster allocator, this
> > edition uses swap_table infrastructure, and share a lot of the allocator
> > logic. Per-slot metadata is stored in a tag-encoded virtual_table
> > (atomic_long_t, 8 bytes per slot), and physical clusters store
> > Pointer-tagged rmap entries in the swap_table for reverse lookup back to
> > the virtual cluster.
> >
> > Here are some data layout diagrams:
> >
> > Case 1: vswap entry (virtualized)
> >
> > PTE swap_cluster_info_dynamic
> > vswap_entry +-------------------------+
> > (swp_entry_t) ------>| swap_cluster_info (ci) |
> > | +--------------------+ |
> > | | swap_table | |
> > | | PFN / Shadow | |
> > | | memcg_table | |
> > | | count,flags,order | |
> > | | lock, list | |
> > | +--------------------+ |
> > | |
> > | virtual_table |
> > | +--------------------+ |
> > | | NONE | |
> > | | PHYS | |
> > | | ZERO | |
> > | | ZSWAP(entry*) | |
> > | | FOLIO(folio*) | |
> > | +--------------------+ |
> > +-------------------------+
> > |
> > | PHYS resolves to
> > v
> > PHYSICAL CLUSTER (swap_cluster_info)
> > +--------------------------+
> > | swap_table per-slot: |
> > | NULL - free |
> > | PFN - cached folio |
> > | Shadow - swapped out |
> > | Pointer- vswap rmap |
> > | Bad - unusable |
> > | |
> > | Vswap-backing slot: |
> > | Pointer(C|swp_entry_t) |
> > | rmap back to vswap |
> > +--------------------------+
> >
> > Case 2: direct-mapped physical entry (no vswap)
> >
> > PTE PHYSICAL CLUSTER (swap_cluster_info)
> > phys_entry +--------------------------+
> > (swp_entry_t) ------>| swap_table per-slot: |
> > | NULL - free |
> > | PFN - cached folio |
> > | Shadow - swapped out |
> > | Bad - unusable |
> > +--------------------------+
> >
> > struct swap_cluster_info_dynamic {
> > struct swap_cluster_info ci; /* swap_table, lock, etc. */
> > unsigned int index; /* position in xarray */
> > struct rcu_head rcu; /* kfree_rcu deferred free */
> > atomic_long_t *virtual_table; /* backend info, 8 B/slot */
> > };
> >
> > Each vswap cluster (swap_cluster_info_dynamic) extends the classic
> > swap_cluster_info struct with a virtual_table array that stores the
> > backend information for each virtual swap entry in the cluster. Each
> > entry is tag-encoded in the low 3 bits to indicate backend types:
> >
> > NONE: |----- 0000 ------|000| free / unbacked
> > PHYS: |-- (type:5,off:N)|001| on a physical swapfile (shifted)
> > ZERO: |----- 0000 ------|010| zero-filled page
> > ZSWAP: |--- zswap_entry* |011| compressed in zswap
> > FOLIO: |--- folio* ------|100| in-memory folio
>
> Thanks for trying this approach!
Thanks for the suggestions. I hope going forward we have sth concrete
to tinker with, rather than abstractions :P
>
> For the format part, PHYS don't need that much bits I think,
> so by slightly adjust the format vswap device could be share
> mostly the same format with ordinary device.
>
> For example typical modern system don't have a address space larger
> than 52 bit. (Even with full 64 bits used for addressing, shift it
> by 12 we get 52). Plus 5 for type, you get 57, so you can have a
> marker that should work as long as it shorter than 1000000 for PHYS,
> and shared for all table format since it's not in conflict with
> anything. You have also use a few extra bits so a single swap space
> can be 8 times larger than RAM space, and since we can help
> multiple swap type I think that should be far than enough?
>
> Then you have Shadow back at 001, and zero bit in shadow. The only
> special one is Zswap, which will be 100 now, and that's exactly the
> reserved pointer format in current swap table format, on seeing
> si->flags & VSWAP && is_pointer(swp_tb) you know that's zswap :)
Are you suggesting we merge the virtual table with main swap table?
Man, I'd love to do this. There is a problem though - we have a case
where we occupy both backing physical swap AND swap cache. Do you
think we can fit both the physical swap slot handle and the swap cache
PFN into the same slot in virtual table? Maybe with some expanding...?
Another option is we can be a bit smart about it - if a virtual swap
entry is in swap cache AND occupies physical swap slot, then put the
folio at the physical swap's table, use folio->swap as the rmap.
(I think you recommend this approach somewhere but for the life of me
I can't find the reference - apologies if I'm putting words into your
mouth :))
But this is a bit more complicated - extra care is needed for rmap
handling at the physical swap layer, and swap cache handling at the
virtual swap layer. Maybe a follow-up? :)
>
> Folio / PFN can still be 010 as in the current swap table format.
>
> Then everything seems clean and aligned, no more special handling
> for vswap needed, there are detailed to sort out, but it should work.
>
> > - Pointer-tagged swap_table on physical clusters for rmap (physical
> > -> virtual) lookup.
>
> Or reuse the PHYS format (rename it maybe) since point back to vswap
> is also pointing to a si.
Noted. I'm just doing the simplest thing right now - working
prototype. I mean, we have enough bits :)
>
> > III. Follow-ups:
> >
> > In no particular order (and most of which can be done as follow-up
> > patch series rather than shoving everything in the initial landing):
> >
> > - More thorough stress testing is very much needed.
> >
> > - Performance benchmarks to make sure I don't accidentally regress
> > the vswap-less case, and that the vswap's case performance is
> > good. I suspect I will have to port a lot of the
> > optimizations I implemented in v6 over here - some of the
> > inefficiencies are inherent in any swap virtualization, and
> > would require the same fix (for e.g the MRU cluster caching
> > for faster cluster lookup - see [8] and [9]).
>
> This could be imporved by per-si percpu cluster. Both YoungJun's
> tiering and Baoquan's previous swap ops mentioned this is needed,
> and now vswap also need that. If the vswap is also a si, then it will
> make use of this too.
Yeah I made the same recommendation when I review swap tier last week:
https://lore.kernel.org/all/CAKEwX=N2XcMHN1jatppOk6wnmz-Shab5XMtTtzgYOzRvU_6YFw@mail.gmail.com/
I like it, but yeah it will be complicated. That said, I think not
fixing the fast path for tiering/vswap will seriously restrict their
usefulness. We don't want to go back to the old swap allocator days :)
We can also revive the swap slot cache, but why do it if we can
repurpose your proven design, and just extend it a bit for multiple
tiers/swap devices? :)
>
> YoungJun posted this a few month before:
> https://lore.kernel.org/linux-mm/20260131125454.3187546-5-youngjun.park@lge.com/
>
> The concern is that some locking contention could be heavier, or maybe
> that's just a hypothetical problem though.
I don't think it's hypothetical. At least with vswap, it's very easy
to get into a state where the shared per-cpu cache gets invalidated
constantly if phys swap and vswap allocation alternates (which is
actually very possible under heavy memory pressure), hammering the
slow paths...
>
> >
> > - Runtime enable/disable of the vswap device. To be honest, I don't
> > know if there is a value in this. My preference is vswap can be
> > optimized to the point that any overhead is negligible. Failing that,
> > maybe we can come up with some simple heuristics that automatically
> > decides for users?
> >
> > In this RFC, CONFIG_VSWAP=y means the vswap device is always created at
> > boot, and CONFIG_VSWAP=n means the vswap device is never created. This
> > *might* be enough just on its own.
> >
> > Is a runtime knob (sysfs or sysctl) worth the complexity beyond
> > these heuristics? I'm not sure yet. Maintaining both cases
>
> I checked the code and I think it's not hard to do, patch 1 already
> handling the meta data dynamically, everything will still just work
> even if you remove vswap at runtime. The rest of patches need adaption
> but might not end up being complex, it other comments here
> are considered.
Yeah, it's not terribily hard to do. I'm more wondering if it's worth
the effort, both for the implementer and the user :)
As I said here, if we want vswap, just enable it at boot time and get
a vast (but dynamic) device. We can make it optional per-cgroup
through Youngjun's interface, and that would be good enough?
>
> For patch 2, a few routines like vswap_can_swapin_thp seems not
> needed or should be moved to __swap_cache_alloc? VSWAP_FOLIO is
> same as swap cache folio check, which is already covered. Same for
> zero checking, and VSWAP_NONE which is same as swap count check
> I think. That way we not only save a lot of code, we also no
> longer need to treat vswap specially.
Unfortunately, I think a lot of this complexity is still needed. Vswap
adds a new layer, which means new complications :)
For instance, I think you still need vswap_can_swapin_thp. It
basically enforces that the backend must be something
swap_read_folio() can handle. That means:
1. No zswap.
2. No mixed backend.
3. If it's phys swap, it must be contiguous in the same device.
The vswap entries might look contiguous in the virtual address space,
but completely unsuitable for the backend.... This is a new
complication that does not previously exist for non-virtualized swap,
so it needs more code to handle it...
I use vswap_can_swapin_thp() in two places - first when we try to find
a candidate range of PTEs for swap in (swap_pte_batch). And then
double check after we've added into swap cache, as the backend can
change arbitrarily before swap cache folio is locked.
Similar for some of the other helpers. For instance,
vswap_swapfile_backed() is needed in certain optimizations or for
correctness's sake, etc.
The VSWAP_FOLIO is redundant, I agree. It's just for convenient. It
represents the state of vswap where it's still in swap (swap cache),
but only backed by the swap cache folio, and not any other backends.
Technically, you can represent it with VSWAP_NONE + a check in the
swap cache field to see if there is a folio there.
If it's not too much code in v2 I can try to remove it (while leaving
behind a comment explains the state).
>
> If you keep the format compatible with what we already have
> as the earlier comment mentions, a large portion of this part
> might be unneeded.
>
> > at runtime also has overhead for checking as well, and some of the
> > checks are not cheap :)
>
> I also noticed the new introduced swap_read_folio_phys in patch 3, so
> this actually can be done using Baoquan's swapops idea which is now
> part of Christoph's swap batching:
I'm actually trying to have swap_read_folio() work for both vswap and
phys swpa just with a biiit of if-else. But yeah this might be cleaner
:)
>
> https://lore.kernel.org/linux-mm/20260528124559.2566481-9-hch@lst.de/#r
>
> That series is focusing on batching and better performance but swapops
> was also proposed as a way to solve the virtual layer, makes it possible
> to have vswap as one kind of swapops which is Chris talked a lot about:
>
> https://lore.kernel.org/linux-mm/aZiFvzlBJiYBUDre@MiWiFi-R3L-srv/
>
> Following this, we could have something like:
>
> const struct swap_ops swap_vswap_ops = {
> .submit_write = swap_vswap_submit_write,
> .submit_read = swap_vswap_submit_read,
> };
>
> The move the folio_realloc_swap in swap_vswap_submit_write.
>
> Merge of IO might be moved to lower phyiscal level for vswap.
>
> Another gain is that the memory usage and CPU overhead will be
> lower with only one layer. As I'm recently trying to offload swap
> dataplane off CPU so the CPU won't touch the data at all, the
> overhead will be purely by swap itself, plus some mm overhead.
> Things like that and IO optimization above and could make swap
> subsystem more and more performance sensitive so we have cases
> that needs only one layer.
Yeah I can take a look at this. This prototype purely based on your P4
and with just a bit of hackery to transfer my v6 implementation over.
Not very clean - just a PoC. If everyone is happy I can put more time
in :)
>
> >
> > - Defer per-cluster memcg_table and zeromap allocation on physical
> > clusters. A physical swap cluster backing vswap entries only do
> > not really need their memcg_table, but the current design forces
> > us to allocate it anyway. This is a waste of memory, and is an
> > overhead regression compared to my older design on the zswap-only
> > case, which Johannes has pointed out multiple times (see [6]),
> > and is one of the biggest reasons why I have not been satisfied
> > with this approach thus far. It honestly is a bit of a
> > deal-breaker...
> >
> > That said, I think I might be able to allocate them on demand, i.e
> > only when the first direct-mapped slot is allocated on that cluster.
> > That will give us the best of BOTH worlds, for both the vswap and
> > directly-mapped physical swap cases. No promises, but I will try
> > (if this approach is good enough for all parties).
>
> Zero map is not really a problem when it's just a inlined bit I think.
Yeah no problemo indeed. I saw a zeromap field in struct phys swap
cluster, so I put this in the plan as "to remove later", but I just
took a look and realized it's only for cases where you cant fit the
bit in swap table. I'm gating vswap for 64 bit for now, so should not
be a problem.
> For memcg table allocation, on demand seems a good idea, and actually
> we are not far from there, I tried to generalize the
> alloc-then-retry-sleep-alloc in swap_alloc_table but still not generic
> enough I guess.. Good new is the allocation of the table is already
> kind of ondemand, just need to split the detection of these two kind
> of table.
I have a prototype of this, but I have not tested, so I do not want to
send it out. :)
TLDR is - I still want to record the memcg for vswap (just not charge
it towards the counter). So we still need memcg_table at both level,
generally - just not allocating until needed (basically if a physical
swap slot in the cluster is directly mapped into PTE). You can kinda
tell, since you pass the folio into the allocation path - with some
care you can distinguish between:
1. Virtual swap, or directly maped physical swap -> need memcg_table
2. Physical swap, backing vswap -> does not memcg_table.
Another alternative is you can defer this allocation until the point
where you have to do the charging action. But then you have to be
careful with failure handling, and need to backoff ya di da di da.
Funsies.
I think I did a mixed of these 2 strategies. Anyway, I'll include the
patch in v2 (if folks like this approach).
>
> Mean while I also remember we once discussed about splitting the
> accounting for vswap / physical swap? If we went that approach we
> don't need to treat memcg_table specially.
For the charging behavior, I already have a patch for it actually in
this series (just not the dynamic allocation of the memcg_table field
yet).
Basically:
1. For vswap entry, not backed by phys swap: record swap memcg, hold
reference to pin the memcg, but not charging towards swap.current.
2. For phys swap backing vswap: charging towards swap.current, but
does not record the memcg in its memcg_table, nor does it hold
reference to memcg (its vswap entry holds the reference already)
2. For phys swap directly mapped to PTE: charges, records, and holds reference.
The motivation here is I do not want vswap entry to shares the same
limit as phys swap counter. If we think of it as "infinite" or
"dynamic", it should not be capped at all, but even if it is charged,
it should be something separate.
>
> > - Widen swap_info_struct->max to unsigned long. The vswap device's
> > max is currently clamped to ALIGN_DOWN(UINT_MAX, SWAPFILE_CLUSTER)
> > (~16 TiB) to fit in unsigned int. 16 TiB is small for vswap,
> > especially when we're getting increasingly big machines memory-wise.
>
> This should be very easy to do, just replace unsigned int with
> unsigned long, a lot of place to touch though :)
Agree. I'm just lazy, and this sounds like a simple patch as a
follow-up. This RFC is already 2000 LoCs - I do not want to burden
reviewers with extra useless details :)
^ permalink raw reply
* [PATCH v2] mm/list_lru: drain before clearing xarray entry on reparent
From: Shakeel Butt @ 2026-06-01 16:15 UTC (permalink / raw)
To: Andrew Morton
Cc: Johannes Weiner, Dave Chinner, Roman Gushchin, Muchun Song,
Qi Zheng, Kairui Song, Meta kernel team, linux-mm, cgroups,
linux-kernel, Chris Mason, stable
memcg_reparent_list_lrus() clears the dying memcg's xarray entry with
xas_store(&xas, NULL) before reparenting its per-node lists into the
parent. This opens a window where a concurrent list_lru_del() arriving
for the dying memcg sees xa_load() == NULL, walks to the parent in
lock_list_lru_of_memcg(), takes the parent's per-node lock, and calls
list_del_init() on an item still physically linked on the dying
memcg's list.
If another in-flight thread holds the dying memcg's per-node lock at
the same moment (another list_lru_del, or a list_lru_walk_one running
an isolate callback), both threads modify ->next/->prev pointers on the
same physical list under different locks. Adjacent items can corrupt
each other's links.
Fix it by reversing the order: reparent each per-node list and mark the
child's list lru dead and then clear the xarray entry. Any concurrent
list_lru op that finds the still-set xarray entry either takes the dying
memcg's per-node lock (synchronizing with the drain) or sees LONG_MIN
and walks to the parent, where the items now live.
Fixes: fb56fdf8b9a2 ("mm/list_lru: split the lock to per-cgroup scope")
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
Reported-by: Chris Mason <clm@fb.com>
Cc: stable@vger.kernel.org
---
Changes since v1:
- Use xa_erase_irq() instead of xa_erase() (Sashiko & Claude).
- Added comment on CSS_DYING check in memcg_list_lru_alloc avoiding a new mlru
allocation.
mm/list_lru.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/mm/list_lru.c b/mm/list_lru.c
index dd29bcf8eb5f..d454bce9a78e 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -473,26 +473,29 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren
mutex_lock(&list_lrus_mutex);
list_for_each_entry(lru, &memcg_list_lrus, list) {
struct list_lru_memcg *mlru;
- XA_STATE(xas, &lru->xa, memcg->kmemcg_id);
/*
- * Lock the Xarray to ensure no on going list_lru_memcg
- * allocation and further allocation will see css_is_dying().
+ * css_is_dying() check in memcg_list_lru_alloc() avoids
+ * allocating a new mlru since CSS_DYING is already set for this
+ * memcg a rcu grace period ago.
*/
- xas_lock_irq(&xas);
- mlru = xas_store(&xas, NULL);
- xas_unlock_irq(&xas);
+ mlru = xa_load(&lru->xa, memcg->kmemcg_id);
if (!mlru)
continue;
/*
- * With Xarray value set to NULL, holding the lru lock below
- * prevents list_lru_{add,del,isolate} from touching the lru,
- * safe to reparent.
+ * Reparent each per-node list and mark the child dead
+ * (LONG_MIN) before clearing xarray entry otherwise a
+ * concurrent list_lru_del() may corrupt the list if it arrives
+ * after xarray clear but before reparenting as
+ * lock_list_lru_of_memcg will acquire parent's lock while the
+ * item is still on child's list.
*/
for_each_node(i)
memcg_reparent_list_lru_one(lru, i, &mlru->node[i], parent);
+ xa_erase_irq(&lru->xa, memcg->kmemcg_id);
+
/*
* Here all list_lrus corresponding to the cgroup are guaranteed
* to remain empty, we can safely free this lru, any further
--
2.53.0-Meta
^ permalink raw reply related
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-01 16:22 UTC (permalink / raw)
To: Kairui Song
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAKEwX=PzMwXXgq=ULAkFD9UqMz+ewLqhKt+xdGxkV7OmA2QG6w@mail.gmail.com>
On Mon, Jun 1, 2026 at 8:56 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Mon, Jun 1, 2026 at 12:34 AM Kairui Song <ryncsn@gmail.com> wrote:
> >
> > On Thu, May 28, 2026 at 02:29:24PM +0800, Nhat Pham wrote:
> > > Based on: mm-unstable @ 444fc9435e57 + swap-table phase IV v5 [2].
> > >
> > > I manually adapted Kairui's ghost device implementation (from [4])
> > > for my vswap device. I've credited him as Co-developed-by on Patch I
> > > since a substantial portion of the dynamic-cluster infrastructure is
> > > his (I did propose the idea of using xarray/radix tree for dynamic
> > > swap clusters allocation and management though :P).
> > >
> > > >From here on out, for simplicity, I will refer to swap table phase IV
> > > as "P4", and the older v6 virtual swap space implementation as "v6".
> > >
> >
> > ...
> >
> > >
> > > This series reimplements the virtual swap space concept (see [1])
> > > on top of Kairui Song's swap table infrastructure, on top of [2]
> > > and in accordance with his proposal in [3]. The proposal's idea
> > > is interesting, so I decided to give it a shot myself. I'm still not
> > > 100% sure that this is bug-proof, but hey, it compiles, and has
> > > not crashed in my simple stress testing :)
> > >
> > > The prototype here is feature-complete relative to the swap-table P4
> > > baseline — swapout, swapin, freeing, swapoff, zswap writeback, zswap
> > > shrinker, memcg charging, and THP swapin all work for
> > > both vswap and direct-physical entries — and satisfies all three
> > > requirements above: no backend coupling (zswap/zero entries hold no
> > > physical slot), dynamic swap space (clusters allocated on demand via
> > > xarray, no static provisioning), and efficient backend transfer
> > > (in-place vtable updates, no PTE/rmap walking).
> > >
> > > II. Design
> > >
> > > With vswap, pages are assigned virtual swap entries on a ghost device
> > > with no backing storage. These entries are backed by zswap, zero pages,
> > > or (lazily) physical swap slots. Physical backing is allocated only
> > > when needed — on zswap writeback or reclaim writeout, after the rmap
> > > step.
> > >
> > > Compared to the standalone v6 implementation [1], which introduces a
> > > 24-byte per-entry swap descriptor and its own cluster allocator, this
> > > edition uses swap_table infrastructure, and share a lot of the allocator
> > > logic. Per-slot metadata is stored in a tag-encoded virtual_table
> > > (atomic_long_t, 8 bytes per slot), and physical clusters store
> > > Pointer-tagged rmap entries in the swap_table for reverse lookup back to
> > > the virtual cluster.
> > >
> > > Here are some data layout diagrams:
> > >
> > > Case 1: vswap entry (virtualized)
> > >
> > > PTE swap_cluster_info_dynamic
> > > vswap_entry +-------------------------+
> > > (swp_entry_t) ------>| swap_cluster_info (ci) |
> > > | +--------------------+ |
> > > | | swap_table | |
> > > | | PFN / Shadow | |
> > > | | memcg_table | |
> > > | | count,flags,order | |
> > > | | lock, list | |
> > > | +--------------------+ |
> > > | |
> > > | virtual_table |
> > > | +--------------------+ |
> > > | | NONE | |
> > > | | PHYS | |
> > > | | ZERO | |
> > > | | ZSWAP(entry*) | |
> > > | | FOLIO(folio*) | |
> > > | +--------------------+ |
> > > +-------------------------+
> > > |
> > > | PHYS resolves to
> > > v
> > > PHYSICAL CLUSTER (swap_cluster_info)
> > > +--------------------------+
> > > | swap_table per-slot: |
> > > | NULL - free |
> > > | PFN - cached folio |
> > > | Shadow - swapped out |
> > > | Pointer- vswap rmap |
> > > | Bad - unusable |
> > > | |
> > > | Vswap-backing slot: |
> > > | Pointer(C|swp_entry_t) |
> > > | rmap back to vswap |
> > > +--------------------------+
> > >
> > > Case 2: direct-mapped physical entry (no vswap)
> > >
> > > PTE PHYSICAL CLUSTER (swap_cluster_info)
> > > phys_entry +--------------------------+
> > > (swp_entry_t) ------>| swap_table per-slot: |
> > > | NULL - free |
> > > | PFN - cached folio |
> > > | Shadow - swapped out |
> > > | Bad - unusable |
> > > +--------------------------+
> > >
> > > struct swap_cluster_info_dynamic {
> > > struct swap_cluster_info ci; /* swap_table, lock, etc. */
> > > unsigned int index; /* position in xarray */
> > > struct rcu_head rcu; /* kfree_rcu deferred free */
> > > atomic_long_t *virtual_table; /* backend info, 8 B/slot */
> > > };
> > >
> > > Each vswap cluster (swap_cluster_info_dynamic) extends the classic
> > > swap_cluster_info struct with a virtual_table array that stores the
> > > backend information for each virtual swap entry in the cluster. Each
> > > entry is tag-encoded in the low 3 bits to indicate backend types:
> > >
> > > NONE: |----- 0000 ------|000| free / unbacked
> > > PHYS: |-- (type:5,off:N)|001| on a physical swapfile (shifted)
> > > ZERO: |----- 0000 ------|010| zero-filled page
> > > ZSWAP: |--- zswap_entry* |011| compressed in zswap
> > > FOLIO: |--- folio* ------|100| in-memory folio
> >
> > Thanks for trying this approach!
>
> Thanks for the suggestions. I hope going forward we have sth concrete
> to tinker with, rather than abstractions :P
>
> >
> > For the format part, PHYS don't need that much bits I think,
> > so by slightly adjust the format vswap device could be share
> > mostly the same format with ordinary device.
> >
> > For example typical modern system don't have a address space larger
> > than 52 bit. (Even with full 64 bits used for addressing, shift it
> > by 12 we get 52). Plus 5 for type, you get 57, so you can have a
> > marker that should work as long as it shorter than 1000000 for PHYS,
> > and shared for all table format since it's not in conflict with
> > anything. You have also use a few extra bits so a single swap space
> > can be 8 times larger than RAM space, and since we can help
> > multiple swap type I think that should be far than enough?
> >
> > Then you have Shadow back at 001, and zero bit in shadow. The only
> > special one is Zswap, which will be 100 now, and that's exactly the
> > reserved pointer format in current swap table format, on seeing
> > si->flags & VSWAP && is_pointer(swp_tb) you know that's zswap :)
>
> Are you suggesting we merge the virtual table with main swap table?
>
> Man, I'd love to do this. There is a problem though - we have a case
> where we occupy both backing physical swap AND swap cache. Do you
> think we can fit both the physical swap slot handle and the swap cache
> PFN into the same slot in virtual table? Maybe with some expanding...?
>
> Another option is we can be a bit smart about it - if a virtual swap
> entry is in swap cache AND occupies physical swap slot, then put the
> folio at the physical swap's table, use folio->swap as the rmap.
>
> (I think you recommend this approach somewhere but for the life of me
> I can't find the reference - apologies if I'm putting words into your
> mouth :))
>
> But this is a bit more complicated - extra care is needed for rmap
> handling at the physical swap layer, and swap cache handling at the
> virtual swap layer. Maybe a follow-up? :)
>
> >
> > Folio / PFN can still be 010 as in the current swap table format.
> >
> > Then everything seems clean and aligned, no more special handling
> > for vswap needed, there are detailed to sort out, but it should work.
> >
> > > - Pointer-tagged swap_table on physical clusters for rmap (physical
> > > -> virtual) lookup.
> >
> > Or reuse the PHYS format (rename it maybe) since point back to vswap
> > is also pointing to a si.
>
> Noted. I'm just doing the simplest thing right now - working
> prototype. I mean, we have enough bits :)
>
> >
> > > III. Follow-ups:
> > >
> > > In no particular order (and most of which can be done as follow-up
> > > patch series rather than shoving everything in the initial landing):
> > >
> > > - More thorough stress testing is very much needed.
> > >
> > > - Performance benchmarks to make sure I don't accidentally regress
> > > the vswap-less case, and that the vswap's case performance is
> > > good. I suspect I will have to port a lot of the
> > > optimizations I implemented in v6 over here - some of the
> > > inefficiencies are inherent in any swap virtualization, and
> > > would require the same fix (for e.g the MRU cluster caching
> > > for faster cluster lookup - see [8] and [9]).
> >
> > This could be imporved by per-si percpu cluster. Both YoungJun's
> > tiering and Baoquan's previous swap ops mentioned this is needed,
> > and now vswap also need that. If the vswap is also a si, then it will
> > make use of this too.
Oh and the MRU cluster caching I mentioned here is not the allocation
caching. It's the lookup caching, basically to avoid doing the
xa_load() to look up clusters for consecutive swap operations on the
same vswap cluster (which is the common case with vswap). For v6, it
massively reduces this indirection lookup overhead. Performance-wise
it's an absolute winner, just more complexity (because I need to
handle reference counting carefully).
I also just realized we'll induce the indirection overhead on
allocation here too, even if the cached cluster still have slots for
allocation, because we look up the cluster (which is basically free
for static swap device, but not free for vswap devices). Might need to
take care of that to maintain vswap performance (but it will then
diverge from your existing code...).
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Nhat Pham @ 2026-06-01 16:44 UTC (permalink / raw)
To: Hao Jia
Cc: Yosry Ahmed, akpm, tj, hannes, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <8c0e60e1-5713-69f0-a687-088c87e75764@gmail.com>
On Mon, Jun 1, 2026 at 4:07 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> On 2026/5/30 09:24, Yosry Ahmed wrote:
> > On Tue, May 26, 2026 at 07:45:58PM +0800, Hao Jia wrote:
> >> From: Hao Jia <jiahao1@lixiang.com>
> >>
> >> The zswap background writeback worker shrink_worker() uses a global
> >> cursor zswap_next_shrink, protected by zswap_shrink_lock, to round-robin
> >> across the online memcgs under root_mem_cgroup.
> >>
> >> Proactive writeback also wants a similar per-memcg cursor that is
> >> scoped to the specified memcg, so that repeated invocations against
> >> the same memcg make forward progress across its descendant memcgs
> >> instead of restarting from the first child memcg each time.
> >
> > Is this a problem in practice?
> >
> > Is the concern the overhead of scanning memcgs repeatedly, or lack of
> > fairness? I wonder if we should just do writeback in batches from all
> > memcgs, similar to how reclaim does it, then evaluate at the end if we
> > need to start over?
> >
>
> Not using a per-cgroup cursor will cause issues for "repeated
> small-budget calls" cases. For example, repeatedly triggering a 2MB
> writeback might result in only writing back pages from the first few
> child memcgs every time. In the worst-case scenario (where the writeback
> amount is less than WB_BATCH), it might only ever write back from the
> first child memcg.
>
> Similar to how memory reclaim uses mem_cgroup_iter() (via struct
> mem_cgroup_reclaim_iter) and the old shrink_worker() used
> zswap_next_shrink, we need a shared cursor here.
>
>
> >>
> >> Naturally, group the cursor and its protecting spinlock into a
> >> zswap_wb_iter struct, and make it a member of struct mem_cgroup to
> >> realize per-memcg cursor management. Accordingly, shrink_worker() now
> >> uses the lock and cursor in root_mem_cgroup->zswap_wb_iter.
> >
> > If we really need to have per-memcg cursors (I am not a big fan), I
> > think we can minimize the overhead by making the cursor updates use
> > atomic cmpxchg instead of having a per-memcg lock.
> >
>
> Because mem_cgroup_iter() always calls css_put(&prev->css), we cannot
> simply update zswap_wb_iter.pos via cmpxchg() after calling it. Doing so
> could lead to a double css_put() issue on prev->css.
>
> Therefore, if we switch to the cmpxchg() approach, we wouldn't be able
> to reuse the existing mem_cgroup_iter() logic. We would have to write a
> new function similar to cgroup_iter(), and its implementation might end
> up looking a bit obscure/complex.
>
> Currently, this lock is only used in shrink_memcg(), proactive
> writeback, and mem_cgroup_css_offline(). Note that shrink_memcg() only
> acquires the lock of the root cgroup, and mem_cgroup_css_offline() is
> unlikely to be a hot path.
>
> So, should we keep the spin_lock or go with the cmpxchg() approach?
> Yosry and Nhat, what are your thoughts on this?
TBH, I think the spinlock is simpler at this point if we need to do
all of this explanation to justify correctness of cmpxchg :)
That said, if memcg folks feel like an extra spinlock per cgroup is a
bit much, we can go with the cmpxchg() approach. Please include a FAT
comment explains the compxchg() approach's nuance in the code though.
Speaking from experience, I will forget why it is correct 2 months
after the patch lands :)
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Nhat Pham @ 2026-06-01 16:47 UTC (permalink / raw)
To: Hao Jia
Cc: Yosry Ahmed, akpm, tj, hannes, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <CAKEwX=M5KiWc8ZZTEXCXtxeBrQho3Gs-JnKmBB=YNUkp=WXaKA@mail.gmail.com>
On Mon, Jun 1, 2026 at 9:44 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
>
> TBH, I think the spinlock is simpler at this point if we need to do
> all of this explanation to justify correctness of cmpxchg :)
>
> That said, if memcg folks feel like an extra spinlock per cgroup is a
> bit much, we can go with the cmpxchg() approach. Please include a FAT
> comment explains the compxchg() approach's nuance in the code though.
> Speaking from experience, I will forget why it is correct 2 months
> after the patch lands :)
Another alternative is - can we repurpose any lock here? Locks seem to
be per-lruvec or per-node, unfortunately, and we need something
per-cgroup hmmmm.
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Nhat Pham @ 2026-06-01 17:08 UTC (permalink / raw)
To: Hao Jia
Cc: Yosry Ahmed, akpm, tj, hannes, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <8c0e60e1-5713-69f0-a687-088c87e75764@gmail.com>
On Mon, Jun 1, 2026 at 4:07 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> On 2026/5/30 09:24, Yosry Ahmed wrote:
> > On Tue, May 26, 2026 at 07:45:58PM +0800, Hao Jia wrote:
> >> From: Hao Jia <jiahao1@lixiang.com>
> >>
> >> The zswap background writeback worker shrink_worker() uses a global
> >> cursor zswap_next_shrink, protected by zswap_shrink_lock, to round-robin
> >> across the online memcgs under root_mem_cgroup.
> >>
> >> Proactive writeback also wants a similar per-memcg cursor that is
> >> scoped to the specified memcg, so that repeated invocations against
> >> the same memcg make forward progress across its descendant memcgs
> >> instead of restarting from the first child memcg each time.
> >
> > Is this a problem in practice?
> >
> > Is the concern the overhead of scanning memcgs repeatedly, or lack of
> > fairness? I wonder if we should just do writeback in batches from all
> > memcgs, similar to how reclaim does it, then evaluate at the end if we
> > need to start over?
> >
>
> Not using a per-cgroup cursor will cause issues for "repeated
> small-budget calls" cases. For example, repeatedly triggering a 2MB
> writeback might result in only writing back pages from the first few
> child memcgs every time. In the worst-case scenario (where the writeback
> amount is less than WB_BATCH), it might only ever write back from the
> first child memcg.
>
> Similar to how memory reclaim uses mem_cgroup_iter() (via struct
> mem_cgroup_reclaim_iter) and the old shrink_worker() used
> zswap_next_shrink, we need a shared cursor here.
I think each proactive reclaim invocation just walk the entire subtree
for page reclaim right (see shrink_node_memcgs())? Would that be
acceptable for you?
I also wonder if we can at least make this structure dynamically
allocated... In a system, you only really invoke proactive reclaim
against a few target cgroups, no?
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Kairui Song @ 2026-06-01 17:44 UTC (permalink / raw)
To: Nhat Pham
Cc: Liam.Howlett, akpm, apopple, axelrasmussen, baohua, baolin.wang,
bhe, byungchul, cgroups, chengming.zhou, chrisl, corbet, david,
dev.jain, gourry, hannes, hughd, jannh, joshua.hahnjy, lance.yang,
lenb, linux-doc, linux-kernel, linux-mm, linux-pm,
lorenzo.stoakes, matthew.brost, mhocko, muchun.song, npache,
pavel, peterx, peterz, pfalcato, rafael, rakie.kim,
roman.gushchin, rppt, ryan.roberts, shakeel.butt, shikemeng,
surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed, yuanchu,
zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAKEwX=PzMwXXgq=ULAkFD9UqMz+ewLqhKt+xdGxkV7OmA2QG6w@mail.gmail.com>
On Mon, Jun 1, 2026 at 11:57 PM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Mon, Jun 1, 2026 at 12:34 AM Kairui Song <ryncsn@gmail.com> wrote:
> >
> > For the format part, PHYS don't need that much bits I think,
> > so by slightly adjust the format vswap device could be share
> > mostly the same format with ordinary device.
> >
> > For example typical modern system don't have a address space larger
> > than 52 bit. (Even with full 64 bits used for addressing, shift it
> > by 12 we get 52). Plus 5 for type, you get 57, so you can have a
> > marker that should work as long as it shorter than 1000000 for PHYS,
> > and shared for all table format since it's not in conflict with
> > anything. You have also use a few extra bits so a single swap space
> > can be 8 times larger than RAM space, and since we can help
> > multiple swap type I think that should be far than enough?
> >
> > Then you have Shadow back at 001, and zero bit in shadow. The only
> > special one is Zswap, which will be 100 now, and that's exactly the
> > reserved pointer format in current swap table format, on seeing
> > si->flags & VSWAP && is_pointer(swp_tb) you know that's zswap :)
>
> Are you suggesting we merge the virtual table with main swap table?
>
> Man, I'd love to do this. There is a problem though - we have a case
> where we occupy both backing physical swap AND swap cache. Do you
> think we can fit both the physical swap slot handle and the swap cache
> PFN into the same slot in virtual table? Maybe with some expanding...?
I don't really get why we would need to do that? If you put the PFN
info in the virtual / upper layer, then the count info, locking, and
all swap IO synchronization (via folio lock), dup (current protected
by ci lock / folio lock), and allocation (folio_alloc_swap), are all
handled in this layer.
The physical / lower layer will just hold a reverse entry on
folio_realloc_swap, or no entry at all (no physical layer used, zswap,
or after swap allocation but before IO) right?
Looking up the actual folio from the physical layer will be a bit
slower since it needs to resolve the reverse entry, but the only place
we need to do that is things like migrate, compaction (none of them
exist yet) which seems totally fine?
> > This could be imporved by per-si percpu cluster. Both YoungJun's
> > tiering and Baoquan's previous swap ops mentioned this is needed,
> > and now vswap also need that. If the vswap is also a si, then it will
> > make use of this too.
>
> Yeah I made the same recommendation when I review swap tier last week:
>
> https://lore.kernel.org/all/CAKEwX=N2XcMHN1jatppOk6wnmz-Shab5XMtTtzgYOzRvU_6YFw@mail.gmail.com/
>
> I like it, but yeah it will be complicated. That said, I think not
> fixing the fast path for tiering/vswap will seriously restrict their
> usefulness. We don't want to go back to the old swap allocator days :)
Thanks. Not too complicated, actually our internal kernel
implementation still using si->percpu cluster, and use a counter for
the rotation and each order have a counter :P, it's a bit ugly but
works fine. It still serves pretty well just like the global percpu
cluster, YoungJun's previous per ci percpu cluster also still provides
the fast path, many ways to do that.
>
> >
> > YoungJun posted this a few month before:
> > https://lore.kernel.org/linux-mm/20260131125454.3187546-5-youngjun.park@lge.com/
> >
> > The concern is that some locking contention could be heavier, or maybe
> > that's just a hypothetical problem though.
>
> I don't think it's hypothetical. At least with vswap, it's very easy
> to get into a state where the shared per-cpu cache gets invalidated
> constantly if phys swap and vswap allocation alternates (which is
> actually very possible under heavy memory pressure), hammering the
> slow paths...
I mean if the per-cpu cache is moved to si level, then whoever enters
the allocation path of a si will almost always get a stable percpu
cache to use, even if the last used si changes.
We might better have a more explicit per si fast path for that. The
plist rotation should better be done in a different (will be even
better if lockless) way.
>
> >
> > >
> > > - Runtime enable/disable of the vswap device. To be honest, I don't
> > > know if there is a value in this. My preference is vswap can be
> > > optimized to the point that any overhead is negligible. Failing that,
> > > maybe we can come up with some simple heuristics that automatically
> > > decides for users?
> > >
> > > In this RFC, CONFIG_VSWAP=y means the vswap device is always created at
> > > boot, and CONFIG_VSWAP=n means the vswap device is never created. This
> > > *might* be enough just on its own.
> > >
> > > Is a runtime knob (sysfs or sysctl) worth the complexity beyond
> > > these heuristics? I'm not sure yet. Maintaining both cases
> >
> > I checked the code and I think it's not hard to do, patch 1 already
> > handling the meta data dynamically, everything will still just work
> > even if you remove vswap at runtime. The rest of patches need adaption
> > but might not end up being complex, it other comments here
> > are considered.
>
> Yeah, it's not terribily hard to do. I'm more wondering if it's worth
> the effort, both for the implementer and the user :)
>
> As I said here, if we want vswap, just enable it at boot time and get
> a vast (but dynamic) device. We can make it optional per-cgroup
> through Youngjun's interface, and that would be good enough?
>
> >
> > For patch 2, a few routines like vswap_can_swapin_thp seems not
> > needed or should be moved to __swap_cache_alloc? VSWAP_FOLIO is
> > same as swap cache folio check, which is already covered. Same for
> > zero checking, and VSWAP_NONE which is same as swap count check
> > I think. That way we not only save a lot of code, we also no
> > longer need to treat vswap specially.
>
> Unfortunately, I think a lot of this complexity is still needed. Vswap
> adds a new layer, which means new complications :)
>
> For instance, I think you still need vswap_can_swapin_thp. It
> basically enforces that the backend must be something
> swap_read_folio() can handle. That means:
>
> 1. No zswap.
>
> 2. No mixed backend.
If mixed backend means phys vs zero vs zswap, then we already have
part of that covered with the current swap cache except for the phys
part (zswap part seems very doable with fujunjie's work).
swap_cache_alloc_folio will ensure there is no mixed zerobit, it can
be easily extended to ensure there is no mixed zswap as well
(according to what I've learned from fujunjie's code). Similar logic
for phys detection I think.
> > For memcg table allocation, on demand seems a good idea, and actually
> > we are not far from there, I tried to generalize the
> > alloc-then-retry-sleep-alloc in swap_alloc_table but still not generic
> > enough I guess.. Good new is the allocation of the table is already
> > kind of ondemand, just need to split the detection of these two kind
> > of table.
>
> I have a prototype of this, but I have not tested, so I do not want to
> send it out. :)
>
> TLDR is - I still want to record the memcg for vswap (just not charge
> it towards the counter). So we still need memcg_table at both level,
> generally - just not allocating until needed (basically if a physical
> swap slot in the cluster is directly mapped into PTE). You can kinda
> tell, since you pass the folio into the allocation path - with some
> care you can distinguish between:
>
> 1. Virtual swap, or directly maped physical swap -> need memcg_table
>
> 2. Physical swap, backing vswap -> does not memcg_table.
>
> Another alternative is you can defer this allocation until the point
> where you have to do the charging action. But then you have to be
> careful with failure handling, and need to backoff ya di da di da.
> Funsies.
>
> I think I did a mixed of these 2 strategies. Anyway, I'll include the
> patch in v2 (if folks like this approach).
>
> >
> > Mean while I also remember we once discussed about splitting the
> > accounting for vswap / physical swap? If we went that approach we
> > don't need to treat memcg_table specially.
>
> For the charging behavior, I already have a patch for it actually in
> this series (just not the dynamic allocation of the memcg_table field
> yet).
>
> Basically:
>
> 1. For vswap entry, not backed by phys swap: record swap memcg, hold
> reference to pin the memcg, but not charging towards swap.current.
Maybe you don't need to record memcg here since folio->memcg already
have that info?
I previously had a patch:
https://lore.kernel.org/linux-mm/20260220-swap-table-p4-v1-7-104795d19815@tencent.com/
The defers the recording of memcg, the behavior is almost identical to
before, but charging & recording should be cleaner and you don't need
to record memcg at allocation time hence maybe reduce the possibility
of pinning a memcg. I didn't include that in P4 just to reduce LOC,
maybe can be resent or included.
> 2. For phys swap backing vswap: charging towards swap.current, but
> does not record the memcg in its memcg_table, nor does it hold
> reference to memcg (its vswap entry holds the reference already)
>
> 2. For phys swap directly mapped to PTE: charges, records, and holds reference.
>
> The motivation here is I do not want vswap entry to shares the same
> limit as phys swap counter. If we think of it as "infinite" or
> "dynamic", it should not be capped at all, but even if it is charged,
> it should be something separate.
Good to know, it's not too messy to make everything dynamic, but if
our ultimate goal is to account for them separately, maybe we can save
some effort here.
>
> >
> > > - Widen swap_info_struct->max to unsigned long. The vswap device's
> > > max is currently clamped to ALIGN_DOWN(UINT_MAX, SWAPFILE_CLUSTER)
> > > (~16 TiB) to fit in unsigned int. 16 TiB is small for vswap,
> > > especially when we're getting increasingly big machines memory-wise.
> >
> > This should be very easy to do, just replace unsigned int with
> > unsigned long, a lot of place to touch though :)
>
> Agree. I'm just lazy, and this sounds like a simple patch as a
> follow-up. This RFC is already 2000 LoCs - I do not want to burden
> reviewers with extra useless details :)
No problem, good to see progress here :)
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Kairui Song @ 2026-06-01 17:49 UTC (permalink / raw)
To: Nhat Pham
Cc: Liam.Howlett, akpm, apopple, axelrasmussen, baohua, baolin.wang,
bhe, byungchul, cgroups, chengming.zhou, chrisl, corbet, david,
dev.jain, gourry, hannes, hughd, jannh, joshua.hahnjy, lance.yang,
lenb, linux-doc, linux-kernel, linux-mm, linux-pm,
lorenzo.stoakes, matthew.brost, mhocko, muchun.song, npache,
pavel, peterx, peterz, pfalcato, rafael, rakie.kim,
roman.gushchin, rppt, ryan.roberts, shakeel.butt, shikemeng,
surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed, yuanchu,
zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAKEwX=NNNf0KCZC0ph7VRW0gjnbXd4W5NKEaHM4XzPdN03Ek3A@mail.gmail.com>
On Tue, Jun 2, 2026 at 12:22 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Mon, Jun 1, 2026 at 8:56 AM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > On Mon, Jun 1, 2026 at 12:34 AM Kairui Song <ryncsn@gmail.com> wrote:
> > >
> > > On Thu, May 28, 2026 at 02:29:24PM +0800, Nhat Pham wrote:
> > > > III. Follow-ups:
> > > >
> > > > In no particular order (and most of which can be done as follow-up
> > > > patch series rather than shoving everything in the initial landing):
> > > >
> > > > - More thorough stress testing is very much needed.
> > > >
> > > > - Performance benchmarks to make sure I don't accidentally regress
> > > > the vswap-less case, and that the vswap's case performance is
> > > > good. I suspect I will have to port a lot of the
> > > > optimizations I implemented in v6 over here - some of the
> > > > inefficiencies are inherent in any swap virtualization, and
> > > > would require the same fix (for e.g the MRU cluster caching
> > > > for faster cluster lookup - see [8] and [9]).
> > >
> > > This could be imporved by per-si percpu cluster. Both YoungJun's
> > > tiering and Baoquan's previous swap ops mentioned this is needed,
> > > and now vswap also need that. If the vswap is also a si, then it will
> > > make use of this too.
>
> Oh and the MRU cluster caching I mentioned here is not the allocation
> caching. It's the lookup caching, basically to avoid doing the
> xa_load() to look up clusters for consecutive swap operations on the
> same vswap cluster (which is the common case with vswap). For v6, it
> massively reduces this indirection lookup overhead. Performance-wise
> it's an absolute winner, just more complexity (because I need to
> handle reference counting carefully).
Ah alright, that's interesting. And I think we can keep things simple
to start, since sensitive users is stil able tol use plain device this
way.
BTW maintaining MRU is also an overhead, I'm not sure if the lookup
pattern always follows that?
> I also just realized we'll induce the indirection overhead on
> allocation here too, even if the cached cluster still have slots for
> allocation, because we look up the cluster (which is basically free
> for static swap device, but not free for vswap devices). Might need to
> take care of that to maintain vswap performance (but it will then
> diverge from your existing code...).
That part should be indeed coverable by the si->percpu cluster though, I think.
^ permalink raw reply
* Re: [PATCH v2] mm/list_lru: drain before clearing xarray entry on reparent
From: Kairui Song @ 2026-06-01 17:50 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Johannes Weiner, Dave Chinner, Roman Gushchin,
Muchun Song, Qi Zheng, Meta kernel team, linux-mm, cgroups,
linux-kernel, Chris Mason, stable
In-Reply-To: <20260601161501.1444829-1-shakeel.butt@linux.dev>
On Tue, Jun 2, 2026 at 12:28 AM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> memcg_reparent_list_lrus() clears the dying memcg's xarray entry with
> xas_store(&xas, NULL) before reparenting its per-node lists into the
> parent. This opens a window where a concurrent list_lru_del() arriving
> for the dying memcg sees xa_load() == NULL, walks to the parent in
> lock_list_lru_of_memcg(), takes the parent's per-node lock, and calls
> list_del_init() on an item still physically linked on the dying
> memcg's list.
>
> If another in-flight thread holds the dying memcg's per-node lock at
> the same moment (another list_lru_del, or a list_lru_walk_one running
> an isolate callback), both threads modify ->next/->prev pointers on the
> same physical list under different locks. Adjacent items can corrupt
> each other's links.
>
> Fix it by reversing the order: reparent each per-node list and mark the
> child's list lru dead and then clear the xarray entry. Any concurrent
> list_lru op that finds the still-set xarray entry either takes the dying
> memcg's per-node lock (synchronizing with the drain) or sees LONG_MIN
> and walks to the parent, where the items now live.
>
> Fixes: fb56fdf8b9a2 ("mm/list_lru: split the lock to per-cgroup scope")
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> Reported-by: Chris Mason <clm@fb.com>
> Cc: stable@vger.kernel.org
> ---
> Changes since v1:
> - Use xa_erase_irq() instead of xa_erase() (Sashiko & Claude).
> - Added comment on CSS_DYING check in memcg_list_lru_alloc avoiding a new mlru
> allocation.
>
> mm/list_lru.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index dd29bcf8eb5f..d454bce9a78e 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -473,26 +473,29 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren
> mutex_lock(&list_lrus_mutex);
> list_for_each_entry(lru, &memcg_list_lrus, list) {
> struct list_lru_memcg *mlru;
> - XA_STATE(xas, &lru->xa, memcg->kmemcg_id);
>
> /*
> - * Lock the Xarray to ensure no on going list_lru_memcg
> - * allocation and further allocation will see css_is_dying().
> + * css_is_dying() check in memcg_list_lru_alloc() avoids
> + * allocating a new mlru since CSS_DYING is already set for this
> + * memcg a rcu grace period ago.
> */
> - xas_lock_irq(&xas);
> - mlru = xas_store(&xas, NULL);
> - xas_unlock_irq(&xas);
> + mlru = xa_load(&lru->xa, memcg->kmemcg_id);
> if (!mlru)
> continue;
>
> /*
> - * With Xarray value set to NULL, holding the lru lock below
> - * prevents list_lru_{add,del,isolate} from touching the lru,
> - * safe to reparent.
> + * Reparent each per-node list and mark the child dead
> + * (LONG_MIN) before clearing xarray entry otherwise a
> + * concurrent list_lru_del() may corrupt the list if it arrives
> + * after xarray clear but before reparenting as
> + * lock_list_lru_of_memcg will acquire parent's lock while the
> + * item is still on child's list.
> */
> for_each_node(i)
> memcg_reparent_list_lru_one(lru, i, &mlru->node[i], parent);
>
> + xa_erase_irq(&lru->xa, memcg->kmemcg_id);
> +
> /*
> * Here all list_lrus corresponding to the cgroup are guaranteed
> * to remain empty, we can safely free this lru, any further
> --
> 2.53.0-Meta
>
>
Nice catch! Thanks a lot!
Reviewed-by: Kairui Song <kasong@tencent.com>
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-01 18:06 UTC (permalink / raw)
To: Kairui Song
Cc: Liam.Howlett, akpm, apopple, axelrasmussen, baohua, baolin.wang,
bhe, byungchul, cgroups, chengming.zhou, chrisl, corbet, david,
dev.jain, gourry, hannes, hughd, jannh, joshua.hahnjy, lance.yang,
lenb, linux-doc, linux-kernel, linux-mm, linux-pm,
lorenzo.stoakes, matthew.brost, mhocko, muchun.song, npache,
pavel, peterx, peterz, pfalcato, rafael, rakie.kim,
roman.gushchin, rppt, ryan.roberts, shakeel.butt, shikemeng,
surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed, yuanchu,
zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAMgjq7BhOn48xEyC=2j837R7qddfjeBVHMiRqdx8no4ZEBpBLg@mail.gmail.com>
On Mon, Jun 1, 2026 at 10:45 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Mon, Jun 1, 2026 at 11:57 PM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > On Mon, Jun 1, 2026 at 12:34 AM Kairui Song <ryncsn@gmail.com> wrote:
> > >
> > > For the format part, PHYS don't need that much bits I think,
> > > so by slightly adjust the format vswap device could be share
> > > mostly the same format with ordinary device.
> > >
> > > For example typical modern system don't have a address space larger
> > > than 52 bit. (Even with full 64 bits used for addressing, shift it
> > > by 12 we get 52). Plus 5 for type, you get 57, so you can have a
> > > marker that should work as long as it shorter than 1000000 for PHYS,
> > > and shared for all table format since it's not in conflict with
> > > anything. You have also use a few extra bits so a single swap space
> > > can be 8 times larger than RAM space, and since we can help
> > > multiple swap type I think that should be far than enough?
> > >
> > > Then you have Shadow back at 001, and zero bit in shadow. The only
> > > special one is Zswap, which will be 100 now, and that's exactly the
> > > reserved pointer format in current swap table format, on seeing
> > > si->flags & VSWAP && is_pointer(swp_tb) you know that's zswap :)
> >
> > Are you suggesting we merge the virtual table with main swap table?
> >
> > Man, I'd love to do this. There is a problem though - we have a case
> > where we occupy both backing physical swap AND swap cache. Do you
> > think we can fit both the physical swap slot handle and the swap cache
> > PFN into the same slot in virtual table? Maybe with some expanding...?
>
> I don't really get why we would need to do that? If you put the PFN
> info in the virtual / upper layer, then the count info, locking, and
> all swap IO synchronization (via folio lock), dup (current protected
> by ci lock / folio lock), and allocation (folio_alloc_swap), are all
> handled in this layer.
>
> The physical / lower layer will just hold a reverse entry on
> folio_realloc_swap, or no entry at all (no physical layer used, zswap,
> or after swap allocation but before IO) right?
>
> Looking up the actual folio from the physical layer will be a bit
> slower since it needs to resolve the reverse entry, but the only place
> we need to do that is things like migrate, compaction (none of them
> exist yet) which seems totally fine?
All of this is correct, but consider swaping in a vswap entry backed
by pswap. There are cases where you still want to maintain the pswap
slots around backing vswap entry, while having the swap cache folio as
well.
For e.g, at swap in time, we add the folio into the swap cache. First
of all, we need to hold on to the physical swap slot for IO step. But
even after IO succeeds, there are cases where you would still like to
keep physical swap slots around (for e.g, to avoid swapping out again
if the folio is only speculatively fetched).
So you have to make sure we have space for both the physical swap
slot, and the swap cache folio's PFN at the same time for each vswap
entry. So we still need the vtable extension (well maybe the other
approach I mentioned could work, but I'm not 100% sure).
>
> > > This could be imporved by per-si percpu cluster. Both YoungJun's
> > > tiering and Baoquan's previous swap ops mentioned this is needed,
> > > and now vswap also need that. If the vswap is also a si, then it will
> > > make use of this too.
> >
> > Yeah I made the same recommendation when I review swap tier last week:
> >
> > https://lore.kernel.org/all/CAKEwX=N2XcMHN1jatppOk6wnmz-Shab5XMtTtzgYOzRvU_6YFw@mail.gmail.com/
> >
> > I like it, but yeah it will be complicated. That said, I think not
> > fixing the fast path for tiering/vswap will seriously restrict their
> > usefulness. We don't want to go back to the old swap allocator days :)
>
> Thanks. Not too complicated, actually our internal kernel
> implementation still using si->percpu cluster, and use a counter for
> the rotation and each order have a counter :P, it's a bit ugly but
> works fine. It still serves pretty well just like the global percpu
> cluster, YoungJun's previous per ci percpu cluster also still provides
> the fast path, many ways to do that.
Sounds like something that should be upstreamed? ;)
I was concerned that you might deem the overhead too high (so I came
up with the per-tier percpu caching), but honestly I like it a lot.
>
> >
> > >
> > > YoungJun posted this a few month before:
> > > https://lore.kernel.org/linux-mm/20260131125454.3187546-5-youngjun.park@lge.com/
> > >
> > > The concern is that some locking contention could be heavier, or maybe
> > > that's just a hypothetical problem though.
> >
> > I don't think it's hypothetical. At least with vswap, it's very easy
> > to get into a state where the shared per-cpu cache gets invalidated
> > constantly if phys swap and vswap allocation alternates (which is
> > actually very possible under heavy memory pressure), hammering the
> > slow paths...
>
> I mean if the per-cpu cache is moved to si level, then whoever enters
> the allocation path of a si will almost always get a stable percpu
> cache to use, even if the last used si changes.
>
> We might better have a more explicit per si fast path for that. The
> plist rotation should better be done in a different (will be even
> better if lockless) way.
Exactly! That's what I'm thinking too. Basically I'm special-casing
for vswap here, but if you're happy with generalizing this for every
si I'm happy with it.
>
> >
> > >
> > > >
> > > > - Runtime enable/disable of the vswap device. To be honest, I don't
> > > > know if there is a value in this. My preference is vswap can be
> > > > optimized to the point that any overhead is negligible. Failing that,
> > > > maybe we can come up with some simple heuristics that automatically
> > > > decides for users?
> > > >
> > > > In this RFC, CONFIG_VSWAP=y means the vswap device is always created at
> > > > boot, and CONFIG_VSWAP=n means the vswap device is never created. This
> > > > *might* be enough just on its own.
> > > >
> > > > Is a runtime knob (sysfs or sysctl) worth the complexity beyond
> > > > these heuristics? I'm not sure yet. Maintaining both cases
> > >
> > > I checked the code and I think it's not hard to do, patch 1 already
> > > handling the meta data dynamically, everything will still just work
> > > even if you remove vswap at runtime. The rest of patches need adaption
> > > but might not end up being complex, it other comments here
> > > are considered.
> >
> > Yeah, it's not terribily hard to do. I'm more wondering if it's worth
> > the effort, both for the implementer and the user :)
> >
> > As I said here, if we want vswap, just enable it at boot time and get
> > a vast (but dynamic) device. We can make it optional per-cgroup
> > through Youngjun's interface, and that would be good enough?
> >
> > >
> > > For patch 2, a few routines like vswap_can_swapin_thp seems not
> > > needed or should be moved to __swap_cache_alloc? VSWAP_FOLIO is
> > > same as swap cache folio check, which is already covered. Same for
> > > zero checking, and VSWAP_NONE which is same as swap count check
> > > I think. That way we not only save a lot of code, we also no
> > > longer need to treat vswap specially.
> >
> > Unfortunately, I think a lot of this complexity is still needed. Vswap
> > adds a new layer, which means new complications :)
> >
> > For instance, I think you still need vswap_can_swapin_thp. It
> > basically enforces that the backend must be something
> > swap_read_folio() can handle. That means:
> >
> > 1. No zswap.
> >
> > 2. No mixed backend.
>
> If mixed backend means phys vs zero vs zswap, then we already have
> part of that covered with the current swap cache except for the phys
> part (zswap part seems very doable with fujunjie's work).
> swap_cache_alloc_folio will ensure there is no mixed zerobit, it can
> be easily extended to ensure there is no mixed zswap as well
> (according to what I've learned from fujunjie's code). Similar logic
> for phys detection I think.
Yeah it's basically generalizing that check, and handle the case where
we can have indirection.
I mean I can open-code it, but it has to be there :) And I figure it
might be useful to check this opportunistically (at swap_pte_batch,
even if it's not guaranteed to be correct down the line) before we
even attempt to allocate a large folio etc. to avoid large folio
allocation.
>
> > > For memcg table allocation, on demand seems a good idea, and actually
> > > we are not far from there, I tried to generalize the
> > > alloc-then-retry-sleep-alloc in swap_alloc_table but still not generic
> > > enough I guess.. Good new is the allocation of the table is already
> > > kind of ondemand, just need to split the detection of these two kind
> > > of table.
> >
> > I have a prototype of this, but I have not tested, so I do not want to
> > send it out. :)
> >
> > TLDR is - I still want to record the memcg for vswap (just not charge
> > it towards the counter). So we still need memcg_table at both level,
> > generally - just not allocating until needed (basically if a physical
> > swap slot in the cluster is directly mapped into PTE). You can kinda
> > tell, since you pass the folio into the allocation path - with some
> > care you can distinguish between:
> >
> > 1. Virtual swap, or directly maped physical swap -> need memcg_table
> >
> > 2. Physical swap, backing vswap -> does not memcg_table.
> >
> > Another alternative is you can defer this allocation until the point
> > where you have to do the charging action. But then you have to be
> > careful with failure handling, and need to backoff ya di da di da.
> > Funsies.
> >
> > I think I did a mixed of these 2 strategies. Anyway, I'll include the
> > patch in v2 (if folks like this approach).
> >
> > >
> > > Mean while I also remember we once discussed about splitting the
> > > accounting for vswap / physical swap? If we went that approach we
> > > don't need to treat memcg_table specially.
> >
> > For the charging behavior, I already have a patch for it actually in
> > this series (just not the dynamic allocation of the memcg_table field
> > yet).
> >
> > Basically:
> >
> > 1. For vswap entry, not backed by phys swap: record swap memcg, hold
> > reference to pin the memcg, but not charging towards swap.current.
>
> Maybe you don't need to record memcg here since folio->memcg already
> have that info?
>
> I previously had a patch:
> https://lore.kernel.org/linux-mm/20260220-swap-table-p4-v1-7-104795d19815@tencent.com/
>
> The defers the recording of memcg, the behavior is almost identical to
> before, but charging & recording should be cleaner and you don't need
> to record memcg at allocation time hence maybe reduce the possibility
> of pinning a memcg. I didn't include that in P4 just to reduce LOC,
> maybe can be resent or included.
That works-ish when the folio is sitll in swap cache, but say if it's
vswap backed by zswap (and the swap cache folio has been reclaimed),
you need a place to store the memcg, no?
Just seems cleaner to centralize this info at vswap layer when it is
presented, for now anyway, rather than juggling this on a per-backend
basis.
>
> > 2. For phys swap backing vswap: charging towards swap.current, but
> > does not record the memcg in its memcg_table, nor does it hold
> > reference to memcg (its vswap entry holds the reference already)
> >
> > 2. For phys swap directly mapped to PTE: charges, records, and holds reference.
> >
> > The motivation here is I do not want vswap entry to shares the same
> > limit as phys swap counter. If we think of it as "infinite" or
> > "dynamic", it should not be capped at all, but even if it is charged,
> > it should be something separate.
>
> Good to know, it's not too messy to make everything dynamic, but if
> our ultimate goal is to account for them separately, maybe we can save
> some effort here.
It's not terrible, TBH.
^ permalink raw reply
* Re: [PATCH v5 9/9] mm: switch deferred split shrinker to list_lru
From: Johannes Weiner @ 2026-06-01 18:17 UTC (permalink / raw)
To: Lance Yang
Cc: baolin.wang, akpm, david, ljs, shakeel.butt, mhocko, david,
roman.gushchin, muchun.song, qi.zheng, yosry.ahmed, ziy, liam,
usama.arif, kas, vbabka, ryncsn, zaslonko, gor, baohua, dev.jain,
npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <20260601132135.14272-1-lance.yang@linux.dev>
On Mon, Jun 01, 2026 at 09:21:35PM +0800, Lance Yang wrote:
>
> On Wed, May 27, 2026 at 04:45:16PM -0400, Johannes Weiner wrote:
> [...]
> >diff --git a/mm/swap_state.c b/mm/swap_state.c
> >index 04f5ce992401..9c3a5cf99778 100644
> >--- a/mm/swap_state.c
> >+++ b/mm/swap_state.c
> >@@ -465,6 +465,16 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
> > return ERR_PTR(-ENOMEM);
> > }
> >
>
> Shouldn't this be limited to anon swapin?
>
> e.g. vmf && vma_is_anonymous(vmf->vma)
>
> >+ if (order > 1 && folio_memcg_alloc_deferred(folio)) {
>
> __swap_cache_alloc() is also used by shmem direct swapin, so shmem can
> get here too when handling a large swap entry:
>
> shmem_get_folio_gfp()
> shmem_swapin_folio()
> shmem_swap_alloc_folio()
> swapin_sync()
> swap_cache_alloc_folio()
> __swap_cache_alloc()
> folio_memcg_alloc_deferred()
Good catch, I think you're right. I shouldn't have dismissed that
branch due to "/* Direct swapin skipping swap cache & readahead */"
> @Baolin please correct me if I got it wrong :)
>
> folio_memcg_alloc_deferred() itself doesn't filter shmem out either; it
> only allocates the memcg list_lru metadata for deferred_split_lru:
>
> int folio_memcg_alloc_deferred(struct folio *folio)
> {
> if (mem_cgroup_disabled())
> return 0;
> return folio_memcg_list_lru_alloc(folio, &deferred_split_lru, GFP_KERNEL);
> }
>
> Since deferred_split_lru only queues anon large folios, doing this for
> shmem swapin doesn't buy us anything :)
Yes, agreed. I don't think it's a big deal / show stopper in terms of
user-visible effect, but of course still worth fixing.
I'll send a follow-up patch.
^ permalink raw reply
* [PATCH] cgroup: Migrate tasks to the root css when a controller is rebound
From: Tejun Heo @ 2026-06-01 19:02 UTC (permalink / raw)
To: cgroups, linux-kernel
Cc: Mark Brown, Bert Karwatzki, Johannes Weiner, Michal Koutný,
Sebastian Andrzej Siewior, Petr Malat, kernel test robot,
Martin Pitt, Aishwarya.TCV, Tejun Heo
In-Reply-To: <a9f6c0bcd262e764453b95eb7397871825e11559.camel@web.de>
cgroup_apply_control_disable() defers kill_css_finish() while a css is
still populated, relying on css_update_populated() to fire the deferred
kill once the populated count reaches zero.
This deadlocks when a controller is rebound out of a hierarchy. Mounting
an implicit_on_dfl controller such as perf_event as a v1 hierarchy steals
it off the default hierarchy, and rebind_subsystems() kills its
per-cgroup csses while they are still populated. The migration run in the
same step keeps the old css for a controller no longer in the hierarchy's
mask, so no task is migrated off the dying csses. Their populated count
never reaches zero, the deferred kill_css_finish() never fires, and the
next cgroup_lock_and_drain_offline() hangs forever under cgroup_mutex.
That migration is already a no-op pass over the rebound subtree. Add
cgroup_rebind_ss_mask so find_existing_css_set() resolves the leaving
controllers to the root css. Their tasks are migrated there, the
per-cgroup csses depopulate, and cgroup_apply_control_disable() kills
them synchronously. The deferral stays correct for the rmdir and
controller-disable paths it was meant for.
Fixes: 1dffd95575eb ("cgroup: Defer kill_css_finish() in cgroup_apply_control_disable()")
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/all/41cd159c-54e5-45e0-81df-eaf36a6c028e@sirena.org.uk/
Reported-by: Bert Karwatzki <spasswolf@web.de>
Closes: https://lore.kernel.org/all/4e986b4ed7e16547805d54b6e67d09120bc4d2f2.camel@web.de/
Signed-off-by: Tejun Heo <tj@kernel.org>
---
Hello, and thanks a lot for all the reproduction information. It made this
much easier to track down.
Bert, Mark, would you mind giving this a try on your setups?
kernel/cgroup/cgroup.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index bdc8deedb4f7..7f4861109e48 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -197,6 +197,14 @@ static u32 cgrp_dfl_implicit_ss_mask;
/* some controllers can be threaded on the default hierarchy */
static u32 cgrp_dfl_threaded_ss_mask;
+/*
+ * Set across rebind_subsystems() to the controllers leaving a hierarchy.
+ * Guarded by cgroup_mutex. Makes find_existing_css_set() resolve them to the
+ * root css so the affected tasks are migrated there before
+ * cgroup_apply_control_disable() kills the per-cgroup csses.
+ */
+static u32 cgroup_rebind_ss_mask;
+
/* The list of hierarchy roots */
LIST_HEAD(cgroup_roots);
static int cgroup_root_count;
@@ -1083,7 +1091,15 @@ static struct css_set *find_existing_css_set(struct css_set *old_cset,
* won't change, so no need for locking.
*/
for_each_subsys(ss, i) {
- if (root->subsys_mask & (1UL << i)) {
+ if (unlikely(cgroup_rebind_ss_mask & (1UL << i))) {
+ /*
+ * @ss is leaving this hierarchy and its per-cgroup
+ * csses are about to be killed. Resolve to the
+ * surviving root css so the tasks are migrated there.
+ */
+ template[i] = cgroup_css(&root->cgrp, ss);
+ WARN_ON_ONCE(!template[i]);
+ } else if (root->subsys_mask & (1UL << i)) {
/*
* @ss is in this hierarchy, so we want the
* effective css from @cgrp.
@@ -1853,11 +1869,17 @@ int rebind_subsystems(struct cgroup_root *dst_root, u32 ss_mask)
struct cgroup *scgrp = &cgrp_dfl_root.cgrp;
/*
- * Controllers from default hierarchy that need to be rebound
- * are all disabled together in one go.
+ * Controllers leaving the default hierarchy are disabled
+ * together. cgroup_rebind_ss_mask makes cgroup_apply_control()
+ * migrate their tasks to the root css, so the per-cgroup csses
+ * are unpopulated when cgroup_finalize_control() kills them.
+ * Clear it before cgroup_finalize_control(), which does no
+ * css_set lookup.
*/
cgrp_dfl_root.subsys_mask &= ~dfl_disable_ss_mask;
+ cgroup_rebind_ss_mask = dfl_disable_ss_mask;
WARN_ON(cgroup_apply_control(scgrp));
+ cgroup_rebind_ss_mask = 0;
cgroup_finalize_control(scgrp, 0);
}
@@ -1871,9 +1893,14 @@ int rebind_subsystems(struct cgroup_root *dst_root, u32 ss_mask)
WARN_ON(!css || cgroup_css(dcgrp, ss));
if (src_root != &cgrp_dfl_root) {
- /* disable from the source */
+ /*
+ * Disable from the source, migrating its tasks to the
+ * root css first (see cgroup_rebind_ss_mask).
+ */
src_root->subsys_mask &= ~(1 << ssid);
+ cgroup_rebind_ss_mask = 1 << ssid;
WARN_ON(cgroup_apply_control(scgrp));
+ cgroup_rebind_ss_mask = 0;
cgroup_finalize_control(scgrp, 0);
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] cgroup: Migrate tasks to the root css when a controller is rebound
From: Bert Karwatzki @ 2026-06-01 19:07 UTC (permalink / raw)
To: Tejun Heo, cgroups, linux-kernel
Cc: Mark Brown, spasswolf, Johannes Weiner, Michal Koutný,
Sebastian Andrzej Siewior, Petr Malat, kernel test robot,
Martin Pitt, Aishwarya.TCV
In-Reply-To: <20260601190256.1815778-1-tj@kernel.org>
Am Montag, dem 01.06.2026 um 09:02 -1000 schrieb Tejun Heo:
> cgroup_apply_control_disable() defers kill_css_finish() while a css is
> still populated, relying on css_update_populated() to fire the deferred
> kill once the populated count reaches zero.
>
> This deadlocks when a controller is rebound out of a hierarchy. Mounting
> an implicit_on_dfl controller such as perf_event as a v1 hierarchy steals
> it off the default hierarchy, and rebind_subsystems() kills its
> per-cgroup csses while they are still populated. The migration run in the
> same step keeps the old css for a controller no longer in the hierarchy's
> mask, so no task is migrated off the dying csses. Their populated count
> never reaches zero, the deferred kill_css_finish() never fires, and the
> next cgroup_lock_and_drain_offline() hangs forever under cgroup_mutex.
>
> That migration is already a no-op pass over the rebound subtree. Add
> cgroup_rebind_ss_mask so find_existing_css_set() resolves the leaving
> controllers to the root css. Their tasks are migrated there, the
> per-cgroup csses depopulate, and cgroup_apply_control_disable() kills
> them synchronously. The deferral stays correct for the rmdir and
> controller-disable paths it was meant for.
>
> Fixes: 1dffd95575eb ("cgroup: Defer kill_css_finish() in cgroup_apply_control_disable()")
> Reported-by: Mark Brown <broonie@kernel.org>
> Closes: https://lore.kernel.org/all/41cd159c-54e5-45e0-81df-eaf36a6c028e@sirena.org.uk/
> Reported-by: Bert Karwatzki <spasswolf@web.de>
> Closes: https://lore.kernel.org/all/4e986b4ed7e16547805d54b6e67d09120bc4d2f2.camel@web.de/
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
> Hello, and thanks a lot for all the reproduction information. It made this
> much easier to track down.
>
> Bert, Mark, would you mind giving this a try on your setups?
>
> kernel/cgroup/cgroup.c | 35 +++++++++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index bdc8deedb4f7..7f4861109e48 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -197,6 +197,14 @@ static u32 cgrp_dfl_implicit_ss_mask;
> /* some controllers can be threaded on the default hierarchy */
> static u32 cgrp_dfl_threaded_ss_mask;
>
> +/*
> + * Set across rebind_subsystems() to the controllers leaving a hierarchy.
> + * Guarded by cgroup_mutex. Makes find_existing_css_set() resolve them to the
> + * root css so the affected tasks are migrated there before
> + * cgroup_apply_control_disable() kills the per-cgroup csses.
> + */
> +static u32 cgroup_rebind_ss_mask;
> +
> /* The list of hierarchy roots */
> LIST_HEAD(cgroup_roots);
> static int cgroup_root_count;
> @@ -1083,7 +1091,15 @@ static struct css_set *find_existing_css_set(struct css_set *old_cset,
> * won't change, so no need for locking.
> */
> for_each_subsys(ss, i) {
> - if (root->subsys_mask & (1UL << i)) {
> + if (unlikely(cgroup_rebind_ss_mask & (1UL << i))) {
> + /*
> + * @ss is leaving this hierarchy and its per-cgroup
> + * csses are about to be killed. Resolve to the
> + * surviving root css so the tasks are migrated there.
> + */
> + template[i] = cgroup_css(&root->cgrp, ss);
> + WARN_ON_ONCE(!template[i]);
> + } else if (root->subsys_mask & (1UL << i)) {
> /*
> * @ss is in this hierarchy, so we want the
> * effective css from @cgrp.
> @@ -1853,11 +1869,17 @@ int rebind_subsystems(struct cgroup_root *dst_root, u32 ss_mask)
> struct cgroup *scgrp = &cgrp_dfl_root.cgrp;
>
> /*
> - * Controllers from default hierarchy that need to be rebound
> - * are all disabled together in one go.
> + * Controllers leaving the default hierarchy are disabled
> + * together. cgroup_rebind_ss_mask makes cgroup_apply_control()
> + * migrate their tasks to the root css, so the per-cgroup csses
> + * are unpopulated when cgroup_finalize_control() kills them.
> + * Clear it before cgroup_finalize_control(), which does no
> + * css_set lookup.
> */
> cgrp_dfl_root.subsys_mask &= ~dfl_disable_ss_mask;
> + cgroup_rebind_ss_mask = dfl_disable_ss_mask;
> WARN_ON(cgroup_apply_control(scgrp));
> + cgroup_rebind_ss_mask = 0;
> cgroup_finalize_control(scgrp, 0);
> }
>
> @@ -1871,9 +1893,14 @@ int rebind_subsystems(struct cgroup_root *dst_root, u32 ss_mask)
> WARN_ON(!css || cgroup_css(dcgrp, ss));
>
> if (src_root != &cgrp_dfl_root) {
> - /* disable from the source */
> + /*
> + * Disable from the source, migrating its tasks to the
> + * root css first (see cgroup_rebind_ss_mask).
> + */
> src_root->subsys_mask &= ~(1 << ssid);
> + cgroup_rebind_ss_mask = 1 << ssid;
> WARN_ON(cgroup_apply_control(scgrp));
> + cgroup_rebind_ss_mask = 0;
> cgroup_finalize_control(scgrp, 0);
> }
>
I'll try this right away, but I found out another thing. My real problem seems
to be the perf_event test, the test after perf_events hangs, no matter what
test I run:
cgroup_fj_function_perf_event: pass (0.206s)
cgroup_core01: HANG
Bert Karwatzki
^ permalink raw reply
* Re: [PATCH] cgroup: Migrate tasks to the root css when a controller is rebound
From: Bert Karwatzki @ 2026-06-01 19:50 UTC (permalink / raw)
To: Tejun Heo, cgroups, linux-kernel
Cc: Mark Brown, spasswolf, Johannes Weiner, Michal Koutný,
Sebastian Andrzej Siewior, Petr Malat, kernel test robot,
Martin Pitt, Aishwarya.TCV
In-Reply-To: <a2602616eec07521be1f76508dfc2632c8c571de.camel@web.de>
Am Montag, dem 01.06.2026 um 21:07 +0200 schrieb Bert Karwatzki:
> Am Montag, dem 01.06.2026 um 09:02 -1000 schrieb Tejun Heo:
> > cgroup_apply_control_disable() defers kill_css_finish() while a css is
> > still populated, relying on css_update_populated() to fire the deferred
> > kill once the populated count reaches zero.
> >
> > This deadlocks when a controller is rebound out of a hierarchy. Mounting
> > an implicit_on_dfl controller such as perf_event as a v1 hierarchy steals
> > it off the default hierarchy, and rebind_subsystems() kills its
> > per-cgroup csses while they are still populated. The migration run in the
> > same step keeps the old css for a controller no longer in the hierarchy's
> > mask, so no task is migrated off the dying csses. Their populated count
> > never reaches zero, the deferred kill_css_finish() never fires, and the
> > next cgroup_lock_and_drain_offline() hangs forever under cgroup_mutex.
> >
> > That migration is already a no-op pass over the rebound subtree. Add
> > cgroup_rebind_ss_mask so find_existing_css_set() resolves the leaving
> > controllers to the root css. Their tasks are migrated there, the
> > per-cgroup csses depopulate, and cgroup_apply_control_disable() kills
> > them synchronously. The deferral stays correct for the rmdir and
> > controller-disable paths it was meant for.
> >
> > Fixes: 1dffd95575eb ("cgroup: Defer kill_css_finish() in cgroup_apply_control_disable()")
> > Reported-by: Mark Brown <broonie@kernel.org>
> > Closes: https://lore.kernel.org/all/41cd159c-54e5-45e0-81df-eaf36a6c028e@sirena.org.uk/
> > Reported-by: Bert Karwatzki <spasswolf@web.de>
> > Closes: https://lore.kernel.org/all/4e986b4ed7e16547805d54b6e67d09120bc4d2f2.camel@web.de/
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > ---
> > Hello, and thanks a lot for all the reproduction information. It made this
> > much easier to track down.
> >
> > Bert, Mark, would you mind giving this a try on your setups?
> >
> > kernel/cgroup/cgroup.c | 35 +++++++++++++++++++++++++++++++----
> > 1 file changed, 31 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> > index bdc8deedb4f7..7f4861109e48 100644
> > --- a/kernel/cgroup/cgroup.c
> > +++ b/kernel/cgroup/cgroup.c
> > @@ -197,6 +197,14 @@ static u32 cgrp_dfl_implicit_ss_mask;
> > /* some controllers can be threaded on the default hierarchy */
> > static u32 cgrp_dfl_threaded_ss_mask;
> >
> > +/*
> > + * Set across rebind_subsystems() to the controllers leaving a hierarchy.
> > + * Guarded by cgroup_mutex. Makes find_existing_css_set() resolve them to the
> > + * root css so the affected tasks are migrated there before
> > + * cgroup_apply_control_disable() kills the per-cgroup csses.
> > + */
> > +static u32 cgroup_rebind_ss_mask;
> > +
> > /* The list of hierarchy roots */
> > LIST_HEAD(cgroup_roots);
> > static int cgroup_root_count;
> > @@ -1083,7 +1091,15 @@ static struct css_set *find_existing_css_set(struct css_set *old_cset,
> > * won't change, so no need for locking.
> > */
> > for_each_subsys(ss, i) {
> > - if (root->subsys_mask & (1UL << i)) {
> > + if (unlikely(cgroup_rebind_ss_mask & (1UL << i))) {
> > + /*
> > + * @ss is leaving this hierarchy and its per-cgroup
> > + * csses are about to be killed. Resolve to the
> > + * surviving root css so the tasks are migrated there.
> > + */
> > + template[i] = cgroup_css(&root->cgrp, ss);
> > + WARN_ON_ONCE(!template[i]);
> > + } else if (root->subsys_mask & (1UL << i)) {
> > /*
> > * @ss is in this hierarchy, so we want the
> > * effective css from @cgrp.
> > @@ -1853,11 +1869,17 @@ int rebind_subsystems(struct cgroup_root *dst_root, u32 ss_mask)
> > struct cgroup *scgrp = &cgrp_dfl_root.cgrp;
> >
> > /*
> > - * Controllers from default hierarchy that need to be rebound
> > - * are all disabled together in one go.
> > + * Controllers leaving the default hierarchy are disabled
> > + * together. cgroup_rebind_ss_mask makes cgroup_apply_control()
> > + * migrate their tasks to the root css, so the per-cgroup csses
> > + * are unpopulated when cgroup_finalize_control() kills them.
> > + * Clear it before cgroup_finalize_control(), which does no
> > + * css_set lookup.
> > */
> > cgrp_dfl_root.subsys_mask &= ~dfl_disable_ss_mask;
> > + cgroup_rebind_ss_mask = dfl_disable_ss_mask;
> > WARN_ON(cgroup_apply_control(scgrp));
> > + cgroup_rebind_ss_mask = 0;
> > cgroup_finalize_control(scgrp, 0);
> > }
> >
> > @@ -1871,9 +1893,14 @@ int rebind_subsystems(struct cgroup_root *dst_root, u32 ss_mask)
> > WARN_ON(!css || cgroup_css(dcgrp, ss));
> >
> > if (src_root != &cgrp_dfl_root) {
> > - /* disable from the source */
> > + /*
> > + * Disable from the source, migrating its tasks to the
> > + * root css first (see cgroup_rebind_ss_mask).
> > + */
> > src_root->subsys_mask &= ~(1 << ssid);
> > + cgroup_rebind_ss_mask = 1 << ssid;
> > WARN_ON(cgroup_apply_control(scgrp));
> > + cgroup_rebind_ss_mask = 0;
> > cgroup_finalize_control(scgrp, 0);
> > }
> >
>
>
> Bert Karwatzki
Your fix works for me. No more hangs after cgroup_fj_function_perf_event is run.
Let's hope this solves Mark's problems, too.
Tested-By: Bert Karwatzki <spasswolf@web.de>
Bert Karwatzki
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Yosry Ahmed @ 2026-06-02 0:31 UTC (permalink / raw)
To: Hao Jia
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <8c0e60e1-5713-69f0-a687-088c87e75764@gmail.com>
On Mon, Jun 01, 2026 at 07:07:45PM +0800, Hao Jia wrote:
>
>
> On 2026/5/30 09:24, Yosry Ahmed wrote:
> > On Tue, May 26, 2026 at 07:45:58PM +0800, Hao Jia wrote:
> > > From: Hao Jia <jiahao1@lixiang.com>
> > >
> > > The zswap background writeback worker shrink_worker() uses a global
> > > cursor zswap_next_shrink, protected by zswap_shrink_lock, to round-robin
> > > across the online memcgs under root_mem_cgroup.
> > >
> > > Proactive writeback also wants a similar per-memcg cursor that is
> > > scoped to the specified memcg, so that repeated invocations against
> > > the same memcg make forward progress across its descendant memcgs
> > > instead of restarting from the first child memcg each time.
> >
> > Is this a problem in practice?
> >
> > Is the concern the overhead of scanning memcgs repeatedly, or lack of
> > fairness? I wonder if we should just do writeback in batches from all
> > memcgs, similar to how reclaim does it, then evaluate at the end if we
> > need to start over?
> >
>
> Not using a per-cgroup cursor will cause issues for "repeated small-budget
> calls" cases. For example, repeatedly triggering a 2MB writeback might
> result in only writing back pages from the first few child memcgs every
> time. In the worst-case scenario (where the writeback amount is less than
> WB_BATCH), it might only ever write back from the first child memcg.
Right, so a fairness concern?
I wonder if we should just reclaim a batch from each memcg, then check
if we reached the goal, otherwise start over. If the batch size is small
enough that should work?
>
> Similar to how memory reclaim uses mem_cgroup_iter() (via struct
> mem_cgroup_reclaim_iter) and the old shrink_worker() used zswap_next_shrink,
> we need a shared cursor here.
Right, I understand that in theory we need a cursor. I am just wondering
if the complexity is justified in practice. Reclaim is a much larger
beast than zswap writeback. I wonder if we can just get away with
scanning a batch from each child memcg -- for per-memcg reclaim, not
global.
We can always improve it later with a cursor if there's an actual need.
>
>
> > >
> > > Naturally, group the cursor and its protecting spinlock into a
> > > zswap_wb_iter struct, and make it a member of struct mem_cgroup to
> > > realize per-memcg cursor management. Accordingly, shrink_worker() now
> > > uses the lock and cursor in root_mem_cgroup->zswap_wb_iter.
> >
> > If we really need to have per-memcg cursors (I am not a big fan), I
> > think we can minimize the overhead by making the cursor updates use
> > atomic cmpxchg instead of having a per-memcg lock.
> >
>
> Because mem_cgroup_iter() always calls css_put(&prev->css), we cannot simply
> update zswap_wb_iter.pos via cmpxchg() after calling it. Doing so could lead
> to a double css_put() issue on prev->css.
>
> Therefore, if we switch to the cmpxchg() approach, we wouldn't be able to
> reuse the existing mem_cgroup_iter() logic. We would have to write a new
> function similar to cgroup_iter(), and its implementation might end up
> looking a bit obscure/complex.
What if we do something like this (for the global cursor):
do {
memcg = xchg(zswap_next_shrink, NULL);
memcg = mem_cgroup_iter(NULL, memcg, NULL);
/* If the cursor was advanced from under us, try again */
if (!try_cmpxchg(zswap_next_shrink, NULL, memcg))
continue;
} while (..);
There is a window where a racing shrinker will see the cursor as NULL
and start over, but that should be fine. We can generalize this for the
per-memcg cursor.
That being said..
>
> Currently, this lock is only used in shrink_memcg(), proactive writeback,
> and mem_cgroup_css_offline(). Note that shrink_memcg() only acquires the
> lock of the root cgroup, and mem_cgroup_css_offline() is unlikely to be a
> hot path.
..this made me realize it's probably fine to just use a global lock for
now?
IIUC the only additional contention to the existing lock will be from
userspace proactive writeback, and that shouldn't be a big deal
especially with the critical section being short?
>
> So, should we keep the spin_lock or go with the cmpxchg() approach?
> Yosry and Nhat, what are your thoughts on this?
I think we should experiment with the global lock first. See if you
observe any regressions with workloads that put a lot of pressure on the
lock (a lot of threads in reclaim doing writeback + a few userspace
threads doing proactive writeback). See if the userspace threads
actually cause a meaningful regression.
>
>
>
> > >
> > > Because the cursor is now per-memcg, the offline cleanup must visit
> > > every ancestor that could be holding a reference to the dying memcg.
> > > Factor out __zswap_memcg_offline_cleanup() and walk from dead_memcg up
> > > to the root.
> >
> > Another reason why I don't like per-memcg cursors. There is too much
> > complexity and I wonder if it's warranted. If we stick with per-memcg
> > cursors please do the refactoring in separate patches to make the
> > patches easier to review.
>
>
> Sorry about that. I will try to keep each patch as simple as possible in the
> next version.
No worries, thanks!
>
>
> Thanks,
> Hao
>
>
^ permalink raw reply
* Re: [PATCH v6 01/22] mm/swap: decouple swap cache from physical swap infrastructure
From: Yosry Ahmed @ 2026-06-02 0:56 UTC (permalink / raw)
To: Nhat Pham
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAKEwX=PZnKqfriUsPV2whZyqxfCRNy67z7gyrHObEvztDF0_zg@mail.gmail.com>
On Thu, May 28, 2026 at 02:42:19PM -0700, Nhat Pham wrote:
> On Mon, May 11, 2026 at 3:46 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Tue, May 05, 2026 at 08:38:30AM -0700, Nhat Pham wrote:
> > > When we virtualize the swap space, we will manage swap cache at the
> > > virtual swap layer. To prepare for this, decouple swap cache from
> > > physical swap infrastructure.
> > >
> > > We will also remove all the swap cache related helpers of swap table. We
> > > will keep the rest of the swap table infrastructure, which will be
> > > repurposed to serve as the rmap (physical -> virtual swap mapping)
> > > later.
> >
> > I didn't look through the entire series, but let me ask the same
> > high-level question I asked before. Instead of moving things out of the
> > swap table, why not reuse the swap table as the representation of the
> > virtual swap space? Seems like most/all metadata is already moved there
> > in a nice concise format.
>
> The honest answer is I wasn't sure it would work, so I was hacking
> quietly a prototype on my own time :)
>
> I finally got something that survives stress-ng and constant
> memory.reclaim thrown at it though. I figured I should send it out to
> get feedback before digging myself deeper into that hole:
>
> https://lore.kernel.org/all/20260528212955.1912856-1-nphamcs@gmail.com/
>
> There is still a small problem left (the metadata duplication issue
> that Johannes brought up). It is potentially fixable, but I haven't
> actually tried it out yet, so I don't want to overstate here. But take
> a look at it and let me know how you feel about this alternative
> approach!
Thank you! I will take a look!
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Balbir Singh @ 2026-06-02 2:16 UTC (permalink / raw)
To: Gregory Price
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, apopple, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <ahOqzpzAua96HVkn@gourry-fedora-PF4VCD3F>
On Sun, May 24, 2026 at 09:50:06PM -0400, Gregory Price wrote:
> On Thu, May 21, 2026 at 04:23:28PM +1000, Balbir Singh wrote:
> > On Sun, Feb 22, 2026 at 03:48:15AM -0500, Gregory Price wrote:
> > > Topic type: MM
> > >
> > > Presenter: Gregory Price <gourry@gourry.net>
> > >
> > > This series introduces N_MEMORY_PRIVATE, a NUMA node state for memory
> > > managed by the buddy allocator but excluded from normal allocations.
> > >
> > > I present it with an end-to-end Compressed RAM service (mm/cram.c)
> > > that would otherwise not be possible (or would be considerably more
> > > difficult, be device-specific, and add to the ZONE_DEVICE boondoggle).
> > >
> >
> > Do we have updates/notes from the meeting?
> >
>
> I have been on leave since LSF, but I do have some notes posted:
>
> https://lore.kernel.org/linux-mm/af9i7dkNvGGxPHzu@gourry-fedora-PF4VCD3F/
> https://lore.kernel.org/linux-mm/agYJcRgOHho8upVv@gourry-fedora-PF4VCD3F/
>
> I will be trying to post an updated set stripped down without the GFP
> flag as a first pass w/o RFC tags and no UAPI implications so that
> device folks can play with this upstream.
>
> I'm debating on whether to include OPS_MEMPOLICY in the initial version
> if only because it's not intuitive how it interacts with pagecache. That
> needs more time to bake.
>
It makes sense to look at it and then decide if it makes sense.
> > >
> > > page = alloc_pages_node(nid, __GFP_PRIVATE, 0);
> >
> > Do we want to provide kernel level control over allocation of private
> > pages, I assumed that only user space applications? I would assume
> > node affinity would be the way to do so, unless we have multiple
> >
>
> alloc_pages_node() is the kernel interface
I was think we wouldn't need explicit flags and that allocations would
happen from user space using __GFP_THISNODE to the node or via a nodemask
based on nodes of interest. Is there a reason to add this flag, a system
might have more than one source of N_MEMORY_PRIVATE?
>
> > >
> > > /* Ok but I want to do something useful with it */
> > > static const struct node_private_ops ops = {
> > > .migrate_to = my_migrate_to,
> > > .folio_migrate = my_folio_migrate,
> > > .flags = NP_OPS_MIGRATION | NP_OPS_MEMPOLICY,
> > > };
> > > node_private_set_ops(nid, &ops);
> > >
> >
> > Could you explain this further? Why does OPS_MIGRATION
> > and OPS_MEMPOLICY needs to be set explictly?
> >
>
> Both of these have been removed from the upcoming version, but in this
> RFC version i was testing OPS_MIGRATION as an explicit flag that meant
> "migrate.c can touch the folios" while OPS_MEMPOLICY meant "mempolicy.c
> can touch the folios".
>
> As it turns out, OPS_MIGRATION is not a useful filter, as it doesn't
> actually filter anything (anything using OPS_MIGRATION would also need
> its own filter flag, so better to just drop it and do per-server
> opt-ins).
>
Thanks,
Balbir
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox