* Re: [PATCH v2] cgroup/cpuset: Fix update_prstate() always returning 0 on partition errors
From: Tao Cui @ 2026-06-02 8:19 UTC (permalink / raw)
To: Michal Koutný
Cc: cui.tao, longman, chenridong, tj, hannes, cgroups, linux-kernel,
Tao Cui
In-Reply-To: <ah6JpNvdO7vaBmjS@localhost.localdomain>
Hi Michal,
You are absolutely right. Looking at this again, this patch is total
suckage.
Returning 0 here is the correct declarative UAPI behavior. The invalid
state records intent and can recover later when resources become
available. Returning -EINVAL would only make things worse since the
state has already been mutated.
Sorry for the noise. I'll drop this patch.
--
Tao
在 2026/6/2 15:46, Michal Koutný 写道:
> Hi.
>
> On Tue, Jun 02, 2026 at 12:55:21PM +0800, Tao Cui <cui.tao@linux.dev> wrote:
>> update_prstate() stores the error code in cs->prs_err and transitions
>> the partition to an invalid state, but always returns 0. The caller
>> cpuset_partition_write() uses "return retval ?: nbytes", so the write
>> syscall always appears to succeed from userspace even when the partition
>> became invalid.
>> Return -EINVAL when err is set so userspace can detect
>> the failure immediately.
>
> This is quite a visible UAPI change (a write can succeed to invalidate a
> partition) and users are meant to watch for cpuset.cpus.partition state
> anyway for asynchronous changes.
>
> I'd not change this gratuitously.
>
> Michal
>
^ permalink raw reply
* Re: [PATCH] mm: don't allow empty relative nodemask in mpol_relative_nodemask()
From: Gregory Price @ 2026-06-02 8:44 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Yury Norov, Joshua Hahn, Andrew Morton, Zi Yan, Matthew Brost,
Rakie Kim, Byungchul Park, Ying Huang, Alistair Popple, linux-mm,
linux-kernel, Farhad Alemi, Waiman Long, Rasmus Villemoes,
cgroups
In-Reply-To: <fe33c767-ea11-43e2-8732-f752c9c1205c@kernel.org>
On Mon, Jun 01, 2026 at 04:32:25PM +0200, David Hildenbrand (Arm) wrote:
> >>
> >> 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.
>
Was this actually observed?
mpol_rebind_nodemask() happens when cgroup.cpuset changes, and
cgroup.cpuset cannot be empty.
cpuset only changes with sysfs twiddles or offlining. In either case,
cpuset *guarantees* that cpuset.mems will never be empty.
So... is this an observed bug or just a statically discovered
"bug" that can't actually be reached?
~Gregory
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-06-02 8:57 UTC (permalink / raw)
To: Balbir Singh
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: <ah47NNhuiClgGCdn@parvat>
On Tue, Jun 02, 2026 at 12:16:50PM +1000, Balbir Singh wrote:
> On Sun, May 24, 2026 at 09:50:06PM -0400, Gregory Price wrote:
> >
> > 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.
>
I am thinking i will ship without any OPS flags at all for now and the
have the introduction of ops as a separate series.
> > 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?
>
There's a few things to unpack here. I discussed this many times on
list and at LSF, but to reiterate.
1) __GFP_THISNODE is insufficient to enforce isolation and otherwise
not particularly useful. Additionally, from userland, it's not
something you can actually set.
for node in possible_nodes:
alloc_pages_node(private_node, __GFP_THISNODE)
In fact it's the opposite semantic of what we want.
THISNODE says: "Do not fallback back to OTHER nodes".
The semantic we want is "Do not allow allocations from private
nodes UNLESS we specifically request" (__GFP_PRIVATE).
__GFP_THISNODE does not actually buy you anything here, AND it's
worse, in the scenario where a private node makes its way into the
preferred slot (via possible_nodes or some other nodemask), the
allocator cannot fall back to a node it can access.
__GFP_THISNODE cannot be overloaded to do anything useful here.
2) We're trying not to expose *ANY* userland APIs for this, at all.
The ultimate goal here should be one of two things:
1) fd = open(/dev/xxx, ...);
mem = mmap(fd, ...);
mem[0] = 0xDEADBEEF; /* Fault device page into page table */
In this case, the driver is responsible for doing the
alloc_pages_node() call.
or
2) mem = mmap(NULL, ..., ANON);
mbind(mem, ..., private_node);
mem[0] = 0xDEADBEEF; /* Fault device page into page table */
in this case mempolicy.c is responsible for doing the
alloc_pages_node() call via the _mpol() alloc variants.
Addition OPT flags (reclaim, compaction, whatever), would
(optionally) allow mm/ to operate on the device memory with, for
example, mmu_notifier callbacks to tell the device to invalidate
whatever it's caching about that page.
This would all be relatively transparent the userland, all userland
"knows" is that it's getting memory from a device (/dev/xxx) or a
node it's otherwise aware of hosting device memory somehow.
~Gregory
^ permalink raw reply
* [PATCH] cgroup/cpuset: change ridong's email
From: Ridong Chen @ 2026-06-02 9:10 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný
Cc: cgroups, linux-kernel, ridong.chen
The chenridong@huaweicloud.com is no longer a valid email,
replace it with the personal email ridong.chen@linux.dev
Signed-off-by: Ridong Chen <ridong.chen@linux.dev>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ec290e38b44..e035a3be797c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6535,7 +6535,7 @@ F: include/linux/blk-cgroup.h
CONTROL GROUP - CPUSET
M: Waiman Long <longman@redhat.com>
-R: Chen Ridong <chenridong@huaweicloud.com>
+R: Ridong Chen <ridong.chen@linux.dev>
L: cgroups@vger.kernel.org
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] mm: don't allow empty relative nodemask in mpol_relative_nodemask()
From: David Hildenbrand (Arm) @ 2026-06-02 9:19 UTC (permalink / raw)
To: Gregory Price
Cc: Yury Norov, Joshua Hahn, Andrew Morton, Zi Yan, Matthew Brost,
Rakie Kim, Byungchul Park, Ying Huang, Alistair Popple, linux-mm,
linux-kernel, Farhad Alemi, Waiman Long, Rasmus Villemoes,
cgroups
In-Reply-To: <ah6X-RtVX75YP7VX@gourry-fedora-PF4VCD3F>
On 6/2/26 10:44, Gregory Price wrote:
> On Mon, Jun 01, 2026 at 04:32:25PM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> 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.
>>
>
> Was this actually observed?
>
> mpol_rebind_nodemask() happens when cgroup.cpuset changes, and
> cgroup.cpuset cannot be empty.
>
> cpuset only changes with sysfs twiddles or offlining. In either case,
> cpuset *guarantees* that cpuset.mems will never be empty.
>
> So... is this an observed bug or just a statically discovered
> "bug" that can't actually be reached?
According to the report [1] syzkaller can trigger it. There is no reproducer,
though.
[1]
https://lore.kernel.org/all/CA+0ovCgxbZkXa+OU8w3s84R3KNPNxxRfmsNR-udh+afQBbGNmw@mail.gmail.com/
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] cgroup/cpuset: Free sched domains on rebuild guard failure
From: Ridong Chen @ 2026-06-02 9:27 UTC (permalink / raw)
To: Guopeng Zhang, Waiman Long, Tejun Heo, Johannes Weiner,
Michal Koutný
Cc: Chen Ridong, cgroups, linux-kernel, Guopeng Zhang
In-Reply-To: <20260528093742.1792456-1-guopeng.zhang@linux.dev>
On 2026/5/28 17:37, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> generate_sched_domains() returns sched-domain masks and optional
> attributes that are normally handed to partition_sched_domains(), which
> takes ownership of them.
>
> rebuild_sched_domains_locked() has a WARN guard after
> generate_sched_domains() and before partition_sched_domains() to avoid
> passing offline CPUs into the scheduler domain rebuild path. If that
> guard fires, the function currently returns directly without freeing
> the generated doms and attr.
>
> Free the generated sched-domain masks and attributes before returning
> from the guard failure path.
>
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> kernel/cgroup/cpuset.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 51327333980a..c5fdebc205d8 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1004,8 +1004,11 @@ void rebuild_sched_domains_locked(void)
> * prevent the panic.
> */
> for (i = 0; doms && i < ndoms; i++) {
> - if (WARN_ON_ONCE(!cpumask_subset(doms[i], cpu_active_mask)))
> + if (WARN_ON_ONCE(!cpumask_subset(doms[i], cpu_active_mask))) {
> + free_sched_domains(doms, ndoms);
> + kfree(attr);
> return;
> + }
> }
>
> /* Have scheduler rebuild the domains */
Thank you. This helps prevent a memory leak in this abnormal scenario. I
should have included it when I originally added this check.
--
Best regards,
Ridong
^ permalink raw reply
* Re: [PATCH] mm: don't allow empty relative nodemask in mpol_relative_nodemask()
From: Gregory Price @ 2026-06-02 9:54 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Yury Norov, Joshua Hahn, Andrew Morton, Zi Yan, Matthew Brost,
Rakie Kim, Byungchul Park, Ying Huang, Alistair Popple, linux-mm,
linux-kernel, Farhad Alemi, Waiman Long, Rasmus Villemoes,
cgroups
In-Reply-To: <c98eb14d-b878-4eeb-91f0-d2b1d4407e1e@kernel.org>
On Tue, Jun 02, 2026 at 11:19:49AM +0200, David Hildenbrand (Arm) wrote:
>
> According to the report [1] syzkaller can trigger it. There is no reproducer,
> though.
>
> [1]
> https://lore.kernel.org/all/CA+0ovCgxbZkXa+OU8w3s84R3KNPNxxRfmsNR-udh+afQBbGNmw@mail.gmail.com/
>
The actual implication of this report is that there is a bug in cpuset,
not mempolicy.
mpol_rebind_mm+0x3ab/0x680 mm/mempolicy.c:569
^^^ should never receive a 0-node nodemask ^^^
...snip...
cpuset_update_tasks_nodemask+0x22e/0x340 kernel/cgroup/cpuset.c:2777
^^^ calls guarantee_online_mems ^^^
...snip...
hotplug_update_tasks kernel/cgroup/cpuset.c:3882 [inline]
cpuset_hotplug_update_tasks kernel/cgroup/cpuset.c:3985 [inline]
Relevant code:
void cpuset_update_tasks_nodemask(struct cpuset *cs)
{
... snip ...
guarantee_online_mems(cs, &newmems); <<< critical call
... snip ...
while ((task = css_task_iter_next(&it))) {
... snip ...
mpol_rebind_mm(mm, &cs->mems_allowed);
Seems like maybe mpol_rebind_mm should be called with newmems, not
cs->mems_allowed, though cs->mems_allowed should never be allowed to be
empty, because that makes no sense.
Just eyeballing it, I can't say whether calling with newmems is the
right thing, or if mems_allowed should not be allowed to be empty, would
have to dig in a little further.
~Gregory
^ permalink raw reply
* Re: [PATCH 1/1] mm/thp: clear deferred split shrinker bits when queues drain
From: Usama Arif @ 2026-06-02 10:30 UTC (permalink / raw)
To: Lance Yang, akpm
Cc: david, ljs, shakeel.butt, mhocko, david, roman.gushchin,
muchun.song, qi.zheng, yosry.ahmed, ziy, liam, kas, vbabka,
ryncsn, zaslonko, gor, wangkefeng.wang, baolin.wang, baohua,
dev.jain, npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <20260602043453.67597-1-lance.yang@linux.dev>
On 02/06/2026 05:34, Lance Yang wrote:
> From: Lance Yang <lance.yang@linux.dev>
>
> deferred_split_count() returns the raw list_lru count. When the per-memcg,
> per-node list is empty, that count is 0.
>
> That skips scanning, but it does not tell memcg reclaim that the shrinker
> is empty. shrink_slab_memcg() only clears the memcg shrinker bit when the
> count callback reports SHRINK_EMPTY.
>
> Return SHRINK_EMPTY for an empty deferred split list, so the bit can be
> cleared once the queue has drained.
>
> Signed-off-by: Lance Yang <lance.yang@linux.dev>
Same as slab, workingset and others.
Acked-by: Usama Arif <usama.arif@linux.dev>
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Hao Jia @ 2026-06-02 11:32 UTC (permalink / raw)
To: Nhat Pham
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=NoQNXOMDD0uTSOPWHQX-CMNU1dw=zEuFj=eLcS3fB-ow@mail.gmail.com>
On 2026/6/2 01:08, Nhat Pham wrote:
> 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?
Our current approach is very similar to how proactive memory reclaim
works in shrink_node_memcgs().
shrink_node_memcgs() first calls memcg = mem_cgroup_iter(target_memcg,
NULL, partial);. By doing this, it uses
target_memcg->nodeinfo[nid]->iter->position to retrieve the child memcg
where the last reclaim left off, and then resumes the iteration.
The catch is that zswap can't just reuse
memcg->nodeinfo[nid]->iter->position, as that would mess up the cursor
used by the memory reclaim.
>
> 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?
It is possible to allocate it dynamically, but I am concerned that it
might introduce a slight performance overhead. We would need to add a
**check** like if (READ_ONCE(memcg->zswap_wb_iter)) every time
zswap_mem_cgroup_iter() is called. Furthermore, to handle concurrent
allocations, we might also need to introduce cmpxchg() to resolve race
conditions.
The additional code would look something like this:
static struct zswap_wb_iter *get_zswap_wb_iter(struct mem_cgroup *memcg)
{
struct zswap_wb_iter *iter, *new_iter;
iter = READ_ONCE(memcg->zswap_wb_iter);
if (likely(iter))
return iter;
new_iter = kzalloc(sizeof(*new_iter), GFP_KERNEL);
if (!new_iter)
return NULL;
spin_lock_init(&new_iter->lock);
if (cmpxchg(&memcg->zswap_wb_iter, NULL, new_iter) != NULL) {
/* Lost the race, someone else installed first. */
kfree(new_iter);
}
return READ_ONCE(memcg->zswap_wb_iter);
}
Thanks,
Hao
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Hao Jia @ 2026-06-02 11:33 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: <ah4ZZGl7GYJf54Wz@google.com>
On 2026/6/2 08:31, Yosry Ahmed wrote:
> 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?
Even with a small batch size, for small writeback requests triggered by
user-space (e.g., 2MB, which is batch size * N), it might still
repeatedly write back from only the first N child memcgs. This could
cause the user-space agent to prematurely give up on zswap writeback.
>
>>
>> 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 (..);
>
>
Regarding the code above, IIRC, both the global and per-cgroup cursors
suffer from race conditions. This race can cause mem_cgroup_iter(NULL,
NULL, NULL) to return the root memcg or its descendants, leading zswap
to write back pages from the wrong memcg.
Additionally, since mem_cgroup_iter() puts the prev memcg ref and gets
the next memcg ref, a try_cmpxchg() failure on CPU1 might also lead to a
ref leak for memcg1.
CPU1 CPU2
memcg1 = xchg(pos, NULL)
memcg2 = xchg(pos, NULL) memcg2 = NULL;
memcg1 = mem_cgroup_iter()
mem_cgroup_iter(NULL, **NULL**, NULL) error memcg
try_cmpxchg(pos，NULL，memcg2） succeed
try_cmpxchg(pos，NULL，memcg1） **fail**
I took a stab at implementing a cmpxchg()-based zswap_mem_cgroup_iter()
modeled after mem_cgroup_iter(), and it actually doesn't look that
complex after all :)
Of course, as Nhat mentioned, we definitely need to add plenty of
comments for this function.
static struct mem_cgroup *zswap_mem_cgroup_iter(struct mem_cgroup *root)
{
struct cgroup_subsys_state *css;
struct mem_cgroup *pos, *next;
if (mem_cgroup_disabled())
return NULL;
if (!root)
root = root_mem_cgroup;
rcu_read_lock();
restart:
pos = READ_ONCE(root->zswap_wb_iter.pos);
css = pos ? &pos->css : NULL;
next = NULL;
while ((css = css_next_descendant_pre(css, &root->css))) {
if (css_tryget_online(css))
break;
}
next = css ? mem_cgroup_from_css(css) : NULL;
if (cmpxchg(&root->zswap_wb_iter.pos, pos, next) != pos) {
if (next)
css_put(&next->css);
goto restart;
}
rcu_read_unlock();
return next;
}
> 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?
>
In the current patch implementation, this lock protects the cgroup's own
cursor variable. During each writeback, we only acquire the spin_lock of
the target cgroup itself; we do not attempt to **spin on any child
cgroup's lock while iterating through the descendants**.
Specifically:
- shrink_memcg() will only attempt to acquire the root cgroup's lock
throughout the entire process.
- Proactive writeback will only acquire the lock of the target cgroup
**itself**.
- Only mem_cgroup_css_offline() might attempt to hold locks of other
cgroups, but normally, this shouldn't be a hot path.
Therefore, even if proactive writebacks are triggered concurrently on a
parent cgroup and its child cgroup, there will be **no** lock contention
at all (specifically referring to zswap_wb_iter.lock).
Lock contention would only occur if user-space **concurrently** triggers
proactive writeback on the exact **same** cgroup. And IIRC, in such a
scenario, the bottleneck is more likely to be on other locks anyway.
>>
>> 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.
Sorry, it seems there are some implementation issues with the global
lock approach.
In practice, our user-space agent mostly operates in the following two
scenarios:
- Triggering proactive writeback on the same cgroup at different times
(sequentially).
- Triggering proactive writeback on different cgroups at the same time
(concurrently).
In both cases, there is no lock contention. So, the current lock works
perfectly fine for us.
However, if we really hate zswap_wb_iter.lock, I can try replacing it
with the cmpxchg() approach.
Thanks,
Hao
^ permalink raw reply
* Re: [PATCH 1/1] mm/thp: clear deferred split shrinker bits when queues drain
From: David Hildenbrand (Arm) @ 2026-06-02 12:11 UTC (permalink / raw)
To: Lance Yang, akpm, hannes
Cc: ljs, shakeel.butt, mhocko, david, roman.gushchin, muchun.song,
qi.zheng, yosry.ahmed, ziy, liam, usama.arif, kas, vbabka, ryncsn,
zaslonko, gor, wangkefeng.wang, baolin.wang, baohua, dev.jain,
npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <1dce1561-b42a-4322-a99f-89eba1e7c227@linux.dev>
On 6/2/26 06:38, Lance Yang wrote:
> Sorry, I missed Johannes in Cc ...
>
> On 2026/6/2 12:34, Lance Yang wrote:
>> From: Lance Yang <lance.yang@linux.dev>
>>
>> deferred_split_count() returns the raw list_lru count. When the per-memcg,
>> per-node list is empty, that count is 0.
>>
>> That skips scanning, but it does not tell memcg reclaim that the shrinker
>> is empty. shrink_slab_memcg() only clears the memcg shrinker bit when the
>> count callback reports SHRINK_EMPTY.
What's the effect of that? Would we consider it a fix that we'd want to backport?
>>
>> Return SHRINK_EMPTY for an empty deferred split list, so the bit can be
>> cleared once the queue has drained.
>>
>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
>> ---
>> mm/huge_memory.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 72f6caf0fec6..62d598290c3b 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -4397,7 +4397,10 @@ void deferred_split_folio(struct folio *folio, bool
>> partially_mapped)
>> static unsigned long deferred_split_count(struct shrinker *shrink,
>> struct shrink_control *sc)
>> {
>> - return list_lru_shrink_count(&deferred_split_lru, sc);
>> + unsigned long count;
>> +
>> + count = list_lru_shrink_count(&deferred_split_lru, sc);
>> + return count ?: SHRINK_EMPTY;
>> }
>> static bool thp_underused(struct folio *folio)
>
This is against Johannes' work, right?
If this is a fix, likely it would be fixing 87eaceb3faa5 ("mm: thp: make
deferred split shrinker memcg aware"), right?
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH 1/1] mm/thp: clear deferred split shrinker bits when queues drain
From: Lance Yang @ 2026-06-02 12:47 UTC (permalink / raw)
To: David Hildenbrand (Arm), akpm, hannes
Cc: ljs, shakeel.butt, mhocko, david, roman.gushchin, muchun.song,
qi.zheng, yosry.ahmed, ziy, liam, usama.arif, kas, vbabka, ryncsn,
zaslonko, gor, wangkefeng.wang, baolin.wang, baohua, dev.jain,
npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <716c7351-2641-4317-8675-e07a16a1efe2@kernel.org>
On 2026/6/2 20:11, David Hildenbrand (Arm) wrote:
> On 6/2/26 06:38, Lance Yang wrote:
>> Sorry, I missed Johannes in Cc ...
>>
>> On 2026/6/2 12:34, Lance Yang wrote:
>>> From: Lance Yang <lance.yang@linux.dev>
>>>
>>> deferred_split_count() returns the raw list_lru count. When the per-memcg,
>>> per-node list is empty, that count is 0.
>>>
>>> That skips scanning, but it does not tell memcg reclaim that the shrinker
>>> is empty. shrink_slab_memcg() only clears the memcg shrinker bit when the
>>> count callback reports SHRINK_EMPTY.
>
> What's the effect of that? Would we consider it a fix that we'd want to backport?
Just a stale memcg shrinker bit :) I'd treat this patch as a small
cleanup.
Once the queue is empty, count_objects() returns 0. That skips the scan,
but shrink_slab_memcg() only clears the bit on SHRINK_EMPTY, not 0.
So memcg reclaim can keep calling the shrinker even though there is
nothing on that queue.
>>>
>>> Return SHRINK_EMPTY for an empty deferred split list, so the bit can be
>>> cleared once the queue has drained.
>>>
>>> Signed-off-by: Lance Yang <lance.yang@linux.dev>
>>> ---
>>> mm/huge_memory.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 72f6caf0fec6..62d598290c3b 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -4397,7 +4397,10 @@ void deferred_split_folio(struct folio *folio, bool
>>> partially_mapped)
>>> static unsigned long deferred_split_count(struct shrinker *shrink,
>>> struct shrink_control *sc)
>>> {
>>> - return list_lru_shrink_count(&deferred_split_lru, sc);
>>> + unsigned long count;
>>> +
>>> + count = list_lru_shrink_count(&deferred_split_lru, sc);
>>> + return count ?: SHRINK_EMPTY;
>>> }
>>> static bool thp_underused(struct folio *folio)
>>
>
> This is against Johannes' work, right?
Yep, I noticed it there, but the behavior is older.
> If this is a fix, likely it would be fixing 87eaceb3faa5 ("mm: thp: make
> deferred split shrinker memcg aware"), right?
No missed reclaim, just some extra reclaim work :)
Cheers, Lance
^ permalink raw reply
* Re: [PATCH-next v5 1/6] cgroup/cpuset: Fix node inconsistencies between cpuset_update_tasks_nodemask() and cpuset_attach()
From: Ridong Chen @ 2026-06-02 13:37 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra, ridong.chen
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang
In-Reply-To: <20260602023203.248077-2-longman@redhat.com>
On 2026/6/2 10:31, Waiman Long wrote:
> Whenever memory node mask is changed, there are 4 places where the node
> mask has to be updated or used.
> 1) task's node mask via cpuset_change_task_nodemask()
> 2) memory policy binding via mpol_rebind_mm()
> 3) if memory migration is enabled, migrate from old_mems_allowed to
> the new node mask via cpuset_migrate_mm().
> 4) setting old_mems_allowed
>
> These memory actions are done in cpuset_update_tasks_nodemask() and
> cpuset_attach(). However there are inconsistencies in what node masks
> are being used in these 2 functions.
>
> In cpuset_update_tasks_nodemask(),
> - cpuset_change_task_nodemask(): guarantee_online_mems()
> - mpol_rebind_mm(): mems_allowed
> - cpuset_migrate_mm(): guarantee_online_mems()
> - old_mems_allowed: guarantee_online_mems()
>
> In cpuset_attach(),
> - cpuset_change_task_nodemask(): guarantee_online_mems()
> - mpol_rebind_mm(): effective_mems
> - cpuset_migrate_mm(): effective_mems
> - old_mems_allowed: effective_mems
>
> These inconsistencies dates back to quite a long time ago and it is
> hard to say what should be the correct values.
>
> The guarantee_online_mems() function returns a node mask from current or
> an ancestor cpuset that is a subset of node_states[N_MEMORY]. Nodes in
> node_states[N_MEMORY] are all online, i.e. in node_states[N_ONLINE].
> However, node in node_states[N_ONLINE] may not have memory. So
> node_states[N_MEMORY] should be a subset of node_states[N_ONLINE].
>
> The guarantee_online_mems() function should only be useful for v1 where
> mems_allowed is the same as effective_mems. With v2, the memory nodes
> in effective_mems should always be a subset of node_states[N_MEMORY].
> The only time that may not be true is when a memory hot-unplug operation
> is in progress and a memory node is removed from node_states[N_MEMORY]
> but not yet reflected in effective_mems as cpuset_handle_hotplug()
> has not yet been called from cpuset_track_online_nodes(). When
> cpuset_handle_hotplug() is called later, the memory node setting
> of the relevant cpusets and tasks will be updated. So replacing the
> guarantee_online_mems() call by just using cs->effective_mems should
> be fine.
>
I noticed this pattern in several places:
```
if (cpuset_v2())
newmems = cs->effective_mems;
else
guarantee_online_mems(cs, &newmems);
```
Would it be simpler to move the v2 logic into guarantee_online_mems?
```
static void guarantee_online_mems(struct cpuset *cs, nodemask_t *pmask)
{
if (cpuset_v2()) {
*pmask = cs->effective_mems;
return;
}
while (!nodes_and(*pmask, cs->effective_mems, node_states[N_MEMORY]))
cs = parent_cs(cs);
}
```
> Let use the following setup for both of them and make them consistent.
> - cpuset_change_task_nodemask(): guarantee_online_mems()
> - mpol_rebind_mm(): effective_mems
> - cpuset_migrate_mm(): guarantee_online_mems()
> - old_mems_allowed: guarantee_online_mems()
>
> So for v2, it is effectively all effective_mems. For v1, mpol_rebind_mm()
> uses cpus_allowed which may differ from what guarantee_online_mems()
^
mems_allowed?
> returns.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> kernel/cgroup/cpuset.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 6bdb68689c24..987456b6d879 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2616,6 +2616,13 @@ static void *cpuset_being_rebound;
> * Iterate through each task of @cs updating its mems_allowed to the
> * effective cpuset's. As this function is called with cpuset_mutex held,
> * cpuset membership stays stable.
> + *
> + * - cpuset_change_task_nodemask(): guarantee_online_mems()
> + * - mpol_rebind_mm(): effective_mems
> + * - cpuset_migrate_mm(): guarantee_online_mems()
> + * - old_mems_allowed: guarantee_online_mems()
> + *
> + * For v2, guarantee_online_mems() should just return effective_mems.
I agree, but the implementation is not as simple as what I mentioned above.
> */
> void cpuset_update_tasks_nodemask(struct cpuset *cs)
> {
> @@ -2625,7 +2632,10 @@ void cpuset_update_tasks_nodemask(struct cpuset *cs)
>
> cpuset_being_rebound = cs; /* causes mpol_dup() rebind */
>
> - guarantee_online_mems(cs, &newmems);
> + if (cpuset_v2())
> + newmems = cs->effective_mems;
> + else
> + guarantee_online_mems(cs, &newmems);
>
> /*
> * The mpol_rebind_mm() call takes mmap_lock, which we couldn't
> @@ -2650,7 +2660,7 @@ void cpuset_update_tasks_nodemask(struct cpuset *cs)
>
> migrate = is_memory_migrate(cs);
>
> - mpol_rebind_mm(mm, &cs->mems_allowed);
> + mpol_rebind_mm(mm, &cs->effective_mems);
> if (migrate)
> cpuset_migrate_mm(mm, &cs->old_mems_allowed, &newmems);
> else
> @@ -3148,17 +3158,18 @@ static void cpuset_attach(struct cgroup_taskset *tset)
>
> /*
> * In the default hierarchy, enabling cpuset in the child cgroups
> - * will trigger a number of cpuset_attach() calls with no change
> - * in effective cpus and mems. In that case, we can optimize out
> - * by skipping the task iteration and update.
> + * will trigger a cpuset_attach() call with no change in effective cpus
> + * and mems. In that case, we can optimize out by skipping the task
> + * iteration and update.
> */
> - if (cpuset_v2() && !cpus_updated && !mems_updated) {
> + if (cpuset_v2()) {
> cpuset_attach_nodemask_to = cs->effective_mems;
> - goto out;
> + if (!cpus_updated && !mems_updated)
> + goto out;
> + } else {
> + guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
> }
>
> - guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
> -
> cgroup_taskset_for_each(task, css, tset)
> cpuset_attach_task(cs, task);
>
> @@ -3168,7 +3179,6 @@ static void cpuset_attach(struct cgroup_taskset *tset)
> * if there is no change in effective_mems and CS_MEMORY_MIGRATE is
> * not set.
> */
> - cpuset_attach_nodemask_to = cs->effective_mems;
> if (!is_memory_migrate(cs) && !mems_updated)
> goto out;
>
> @@ -3176,7 +3186,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
> struct mm_struct *mm = get_task_mm(leader);
>
> if (mm) {
> - mpol_rebind_mm(mm, &cpuset_attach_nodemask_to);
> + mpol_rebind_mm(mm, &cs->effective_mems);
>
> /*
> * old_mems_allowed is the same with mems_allowed
--
Best regards,
Ridong
^ permalink raw reply
* Re: [PATCH-next v5 2/6] cgroup/cpuset: Add a cpuset_reserve_dl_bw() helper
From: Ridong Chen @ 2026-06-02 13:40 UTC (permalink / raw)
To: Waiman Long, Chen Ridong, Tejun Heo, Johannes Weiner,
Michal Koutný, Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang
In-Reply-To: <20260602023203.248077-3-longman@redhat.com>
On 2026/6/2 10:31, Waiman Long wrote:
> Extract the DL bandwidth allocation code in cpuset_attach() to a new
> cpuset_reserve_dl_bw() helper to simplify code.
>
> No functional change is expected.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
LGTM.
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
> ---
> kernel/cgroup/cpuset.c | 53 ++++++++++++++++++++++++------------------
> 1 file changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 987456b6d879..5c1f3ee48d5d 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2991,6 +2991,25 @@ static int cpuset_can_attach_check(struct cpuset *cs)
> return 0;
> }
>
> +static int cpuset_reserve_dl_bw(struct cpuset *cs)
> +{
> + int cpu, ret;
> +
> + if (!cs->sum_migrate_dl_bw)
> + return 0;
> +
> + cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
> + if (unlikely(cpu >= nr_cpu_ids))
> + return -EINVAL;
> +
> + ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
> + if (ret)
> + return ret;
> +
> + cs->dl_bw_cpu = cpu;
> + return 0;
> +}
> +
> static void reset_migrate_dl_data(struct cpuset *cs)
> {
> cs->nr_migrate_dl_tasks = 0;
> @@ -3005,7 +3024,7 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> struct cpuset *cs, *oldcs;
> struct task_struct *task;
> bool setsched_check;
> - int cpu, ret;
> + int ret;
>
> /* used later by cpuset_attach() */
> cpuset_attach_old_cs = task_cs(cgroup_taskset_first(tset, &css));
> @@ -3061,31 +3080,19 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> }
> }
>
> - if (!cs->sum_migrate_dl_bw)
> - goto out_success;
> -
> - cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
> - if (unlikely(cpu >= nr_cpu_ids)) {
> - ret = -EINVAL;
> - goto out_unlock;
> - }
> -
> - ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
> - if (ret)
> - goto out_unlock;
> -
> - cs->dl_bw_cpu = cpu;
> -
> -out_success:
> - /*
> - * Mark attach is in progress. This makes validate_change() fail
> - * changes which zero cpus/mems_allowed.
> - */
> - cs->attach_in_progress++;
> + ret = cpuset_reserve_dl_bw(cs);
>
> out_unlock:
> - if (ret)
> + if (ret) {
> reset_migrate_dl_data(cs);
> + } else {
> + /*
> + * Mark attach is in progress. This makes validate_change() fail
> + * changes which zero cpus/mems_allowed.
> + */
> + cs->attach_in_progress++;
> + }
> +
> mutex_unlock(&cpuset_mutex);
> return ret;
> }
--
Best regards,
Ridong
^ permalink raw reply
* Re: [PATCH-next v5 3/6] cgroup/cpuset: Expand the scope of cpuset_can_attach_check()
From: Ridong Chen @ 2026-06-02 13:51 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang
In-Reply-To: <20260602023203.248077-4-longman@redhat.com>
On 2026/6/2 10:32, Waiman Long wrote:
> Expand the scope of cpuset_can_attach_check() by including the setting
> of setsched flag inside cpuset_can_attach_check() with the new @oldcs
> and @psetsched argument. As cpuset_can_attach_check() is also called
> from cpuset_can_fork(), set the new arguments to NULL from that caller.
>
Hi Waiman,
The code change itself looks good to me. However, the commit message
has two paragraphs that don't match this patch:
> While at it, expose the source and destination cpuset cpu/memory check
> results in the new attach_cpus_updated and attach_mems_updated static
> flags so that these flags can be used directly from cpuset_attach()
> without the need to do the same computations again.
>
> Two new global attach related flags are added (attach_cpus_updated &
> attach_mems_updated) which are set to indicate that CPUs or memory nodes
> are updated. These 2 flags are set in cpuset_can_attach() and are used
> in cpuset_attach() for optimization. Since cpuset_mutex will be released
> between the 2 calls, it is possible that an intervening cpuset action
> may change the CPU or node mask of the relevant cpusets, so check is
> added to set these flags if the effective_cpus or effective_mems of
> those cpusets is changed.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
Other than that:
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
> ---
> kernel/cgroup/cpuset.c | 52 ++++++++++++++++++++++++------------------
> 1 file changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 5c1f3ee48d5d..5c777b1237a8 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2982,12 +2982,39 @@ static struct cpuset *cpuset_attach_old_cs;
> * For v1, cpus_allowed and mems_allowed can't be empty.
> * For v2, effective_cpus can't be empty.
> * Note that in v1, effective_cpus = cpus_allowed.
> + *
> + * Also set the boolean flag passed in by @psetsched depending on if
> + * security_task_setscheduler() call is needed and @oldcs is not NULL.
> */
> -static int cpuset_can_attach_check(struct cpuset *cs)
> +static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
> + bool *psetsched)
> {
> if (cpumask_empty(cs->effective_cpus) ||
> (!is_in_v2_mode() && nodes_empty(cs->mems_allowed)))
> return -ENOSPC;
> +
> + if (!oldcs)
> + return 0;
> +
> + /*
> + * Skip rights over task setsched check in v2 when nothing changes,
> + * migration permission derives from hierarchy ownership in
> + * cgroup_procs_write_permission()).
> + */
> + *psetsched = !cpuset_v2() ||
> + !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
> + !nodes_equal(cs->effective_mems, oldcs->effective_mems);
> +
> + /*
> + * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
> + * brings the last online CPU offline as users are not allowed to empty
> + * cpuset.cpus when there are active tasks inside. When that happens,
> + * we should allow tasks to migrate out without security check to make
> + * sure they will be able to run after migration.
> + */
> + if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
> + *psetsched = false;
> +
> return 0;
> }
>
> @@ -3034,29 +3061,10 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> mutex_lock(&cpuset_mutex);
>
> /* Check to see if task is allowed in the cpuset */
> - ret = cpuset_can_attach_check(cs);
> + ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
> if (ret)
> goto out_unlock;
>
> - /*
> - * Skip rights over task setsched check in v2 when nothing changes,
> - * migration permission derives from hierarchy ownership in
> - * cgroup_procs_write_permission()).
> - */
> - setsched_check = !cpuset_v2() ||
> - !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
> - !nodes_equal(cs->effective_mems, oldcs->effective_mems);
> -
> - /*
> - * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
> - * brings the last online CPU offline as users are not allowed to empty
> - * cpuset.cpus when there are active tasks inside. When that happens,
> - * we should allow tasks to migrate out without security check to make
> - * sure they will be able to run after migration.
> - */
> - if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
> - setsched_check = false;
> -
> cgroup_taskset_for_each(task, css, tset) {
> ret = task_can_attach(task);
> if (ret)
> @@ -3601,7 +3609,7 @@ static int cpuset_can_fork(struct task_struct *task, struct css_set *cset)
> mutex_lock(&cpuset_mutex);
>
> /* Check to see if task is allowed in the cpuset */
> - ret = cpuset_can_attach_check(cs);
> + ret = cpuset_can_attach_check(cs, NULL, NULL);
> if (ret)
> goto out_unlock;
>
--
Best regards,
Ridong
^ permalink raw reply
* Re: [PATCH-next v5 4/6] cgroup/cpuset: Make cpuset_attach_old_cs track task group leaders
From: Ridong Chen @ 2026-06-02 13:58 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra, ridong.chen
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang
In-Reply-To: <20260602023203.248077-5-longman@redhat.com>
On 2026/6/2 10:32, Waiman Long wrote:
> There are two possible ways that migration of tasks from multiple source
> cpusets to a target cpuset can happen. Either a multithread application
> with threads in different cpusets is wholely moved to a new cpuset
^
wholly
> or disabling of v2 cpuset controller will move all the tasks in child
> cpusets to the parent cpuset.
>
> In the former case, it is the mm setting of the group leader that really
> matters. So cpuset_attach_old_cs should track the oldcs of the thread
> leader. In the latter case, effective_mems of child cpusets must always
> be a subset of the parent. So no real page migration will be necessary
> no matter which child cpuset is selected as cpuset_attach_old_cs.
>
> IOW, cpuset_attach_old_cs should be updated to match the latest task
> group leader in cpuset_can_attach(), but fall back to that of the first
> task if there is no group leader in the taskset.
>
> Suggested-by: Ridong Chen <ridong.chen@linux.dev>
> Signed-off-by: Waiman Long <longman@redhat.com>
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
> ---
> kernel/cgroup/cpuset.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 5c777b1237a8..60e8149cc907 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2975,6 +2975,10 @@ static int update_prstate(struct cpuset *cs, int new_prs)
> return 0;
> }
>
> +/*
> + * cpuset_can_attach() and cpuset_attach() specific internal data
> + * Protected by cpuset_mutex
> + */
> static struct cpuset *cpuset_attach_old_cs;
>
> /*
> @@ -3065,11 +3069,32 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> if (ret)
> goto out_unlock;
>
> + /*
> + * The cpuset_attach_old_cs is used mainly by cpuset_migrate_mm() to get
> + * the old_mems_allowed value. There are two ways that many-to-one
> + * cpuset migration can happen:
> + * 1) A multithread application with threads in different cpusets is
> + * wholely migrated to a new cpuset.
> + * 2) Disabling v2 cpuset controller will move all the tasks in child
> + * cpusets to the parent cpuset.
> + *
> + * In the former case, it is the mm setting of the group leader that
> + * really matters. So cpuset_attach_old_cs should track the oldcs of the
> + * group leader. It falls back to the oldcs of the first task if there
> + * is no group leader in the taskset. In the latter case, effective_mems
> + * of child cpusets must always be a subset of the parent. So no real
> + * page migration will be necessary no matter which child cpuset is
> + * selected as cpuset_attach_old_cs.
> + */
> cgroup_taskset_for_each(task, css, tset) {
> ret = task_can_attach(task);
> if (ret)
> goto out_unlock;
>
> + /* Update cpuset_attach_old_cs to the latest group leader */
> + if (task == task->group_leader)
> + cpuset_attach_old_cs = task_cs(task);
> +
> if (setsched_check) {
> ret = security_task_setscheduler(task);
> if (ret)
--
Best regards,
Ridong
^ permalink raw reply
* [PATCH v2] cgroup/cpuset: Change email address of Chen Ridong
From: Waiman Long @ 2026-06-02 14:08 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Ridong Chen
Cc: cgroups, linux-kernel, Waiman Long
Chen Ridong has contributed quite a lot of fixes and cleanups to
the cpuset code. Recently, his email address has been changed to
ridong.chen@linux.dev. Update that in the MAINTAINERS file.
Signed-off-by: Waiman Long <longman@redhat.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 74c86cf9bc65..634eb67acd06 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6526,7 +6526,7 @@ F: include/linux/blk-cgroup.h
CONTROL GROUP - CPUSET
M: Waiman Long <longman@redhat.com>
-R: Chen Ridong <chenridong@huaweicloud.com>
+R: Ridong Chen <ridong.chen@linux.dev>
L: cgroups@vger.kernel.org
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] mm: don't allow empty relative nodemask in mpol_relative_nodemask()
From: Farhad Alemi @ 2026-06-02 15:01 UTC (permalink / raw)
To: Gregory Price
Cc: falemi, David Hildenbrand (Arm), Yury Norov, Joshua Hahn,
Andrew Morton, Zi Yan, Matthew Brost, Rakie Kim, Byungchul Park,
Ying Huang, Alistair Popple, linux-mm, linux-kernel, Waiman Long,
Rasmus Villemoes, cgroups
In-Reply-To: <ah6oS7wiGB4u4-eR@gourry-fedora-PF4VCD3F>
[-- Attachment #1: Type: text/plain, Size: 3495 bytes --]
Confirmed, with a standalone reproducer (attached); it panics linus/master
at e8c2f9fdadee. cs->mems_allowed can legitimately be empty
on v2 -- a freshly created cpuset child that never had cpuset.mems
written keeps mems_allowed empty (never initialized) while effective_mems
is inherited non-empty in cpuset_css_online(), and v2 allows attaching
tasks to it (the empty-mems guard in cpuset_can_attach_check() is gated
on !is_in_v2_mode()). So the non-empty guarantee holds for effective_mems,
not for the configured cs->mems_allowed; forbidding empty cpuset.mems
would break v2's inherit-from-parent semantics.
The reproducer enables +cpuset, mkdirs a child without writing
cpuset.mems, moves a task in, mbind()s a VMA with
MPOL_BIND | MPOL_F_RELATIVE_NODES, and offlines a CPU; the hotplug walk
then calls mpol_rebind_mm(mm, &cs->mems_allowed) with the empty mask and
folds modulo nodes_weight(*rel) == 0 (console logs attached).
The newmems instinct looks right: it's the effective, online mask the
task is actually allowed to use, guarantee_online_mems() keeps it
non-empty, and it matches cpuset_attach(), which already rebinds against
cs->effective_mems. The fix this implies:
- mpol_rebind_mm(mm, &cs->mems_allowed);
+ mpol_rebind_mm(mm, &newmems);
I built the current base (e8c2f9fdadee) with and without this one-liner:
the unpatched kernel panics on the first cpu1 offline, while the patched
kernel runs the reproducer's 8 offline/online cycles cleanly, with no
divide error.
This regressed in ae1c802382f7 ("cpuset: apply cs->effective_{cpus,mems}",
v3.17), which moved cpuset_attach() to the effective mask but left this
rebind on cs->mems_allowed.
Happy to send this as a proper patch (Fixes: ae1c802382f7, Cc: stable,
reproducer) if you agree the cpuset side is right, or to test a
mempolicy-side fix if not.
Thanks,
Farhad Alemi
PhD Student
SEFCOM Lab @ ASU
On Tue, Jun 2, 2026 at 2:54 AM Gregory Price <gourry@gourry.net> wrote:
>
> On Tue, Jun 02, 2026 at 11:19:49AM +0200, David Hildenbrand (Arm) wrote:
> >
> > According to the report [1] syzkaller can trigger it. There is no reproducer,
> > though.
> >
> > [1]
> > https://lore.kernel.org/all/CA+0ovCgxbZkXa+OU8w3s84R3KNPNxxRfmsNR-udh+afQBbGNmw@mail.gmail.com/
> >
>
> The actual implication of this report is that there is a bug in cpuset,
> not mempolicy.
>
> mpol_rebind_mm+0x3ab/0x680 mm/mempolicy.c:569
> ^^^ should never receive a 0-node nodemask ^^^
> ...snip...
> cpuset_update_tasks_nodemask+0x22e/0x340 kernel/cgroup/cpuset.c:2777
> ^^^ calls guarantee_online_mems ^^^
> ...snip...
> hotplug_update_tasks kernel/cgroup/cpuset.c:3882 [inline]
> cpuset_hotplug_update_tasks kernel/cgroup/cpuset.c:3985 [inline]
>
> Relevant code:
>
> void cpuset_update_tasks_nodemask(struct cpuset *cs)
> {
> ... snip ...
> guarantee_online_mems(cs, &newmems); <<< critical call
> ... snip ...
> while ((task = css_task_iter_next(&it))) {
> ... snip ...
> mpol_rebind_mm(mm, &cs->mems_allowed);
>
> Seems like maybe mpol_rebind_mm should be called with newmems, not
> cs->mems_allowed, though cs->mems_allowed should never be allowed to be
> empty, because that makes no sense.
>
> Just eyeballing it, I can't say whether calling with newmems is the
> right thing, or if mems_allowed should not be allowed to be empty, would
> have to dig in a little further.
>
> ~Gregory
[-- Attachment #2: reproducer.c --]
[-- Type: application/octet-stream, Size: 5840 bytes --]
// Reproducer for: divide error in bitmap_fold
// (cpuset hotplug rebind of a relative-nodes mempolicy with an empty
// cpuset.mems_allowed)
//
// Lore report:
// https://lore.kernel.org/all/CA+0ovCgxbZkXa+OU8w3s84R3KNPNxxRfmsNR-udh+afQBbGNmw@mail.gmail.com/
//
// Crash signature (PID is a cpuhp/N kthread):
// Oops: divide error: 0000 [#1] ... RIP: bitmap_fold+0x5e/0xb0 lib/bitmap.c:728
// __nodes_fold include/linux/nodemask.h
// mpol_relative_nodemask mm/mempolicy.c:374
// mpol_rebind_nodemask mm/mempolicy.c:511
// mpol_rebind_policy mm/mempolicy.c:545
// mpol_rebind_mm mm/mempolicy.c:572
// cpuset_update_tasks_nodemask kernel/cgroup/cpuset.c:2652
// hotplug_update_tasks / cpuset_hotplug_update_tasks / cpuset_handle_hotplug
// cpuset_cpu_active|inactive -> sched_cpu_activate|deactivate (CPU hotplug)
//
// Mechanism:
// 1. A freshly created cgroup-v2 cpuset child has cpuset.mems_allowed == {}
// (never written) while its effective_mems is inherited from the parent
// and is non-empty. On the legacy (v1) hierarchy, changing a populated
// cpuset's non-empty cpuset.mems to empty is rejected (-ENOSPC) by the
// empty-mems check in cpuset1_validate_change(); on the default (v2)
// hierarchy there is no such check, and a fresh child simply never
// writes mems_allowed at all.
// 2. A task in that child owns a VMA mempolicy created with
// MPOL_F_RELATIVE_NODES and a non-empty user nodemask.
// 3. A CPU hot{,un}plug event makes cpuset_handle_hotplug() walk every
// descendant (the walk is gated on the active CPU/mem set actually
// changing, which a cpu on/offline satisfies via cpus_updated). For the
// child, new effective mems == old, but the v2 hotplug path still calls
// cpuset_update_tasks_nodemask(), which rebinds VMA policies with
// &cs->mems_allowed -- the *configured* (empty) mask, NOT the effective
// one.
// 4. mpol_rebind_nodemask() sees MPOL_F_RELATIVE_NODES and calls
// mpol_relative_nodemask(tmp, user_nodemask, cs->mems_allowed={}), i.e.
// nodes_fold(tmp, user_nodemask, nodes_weight({})==0) -> bitmap_fold()
// with sz==0 -> `oldbit % 0` -> #DE.
//
// Run as root inside the test VM (kernel CONFIG_HOTPLUG_CPU, CONFIG_CPUSETS,
// CONFIG_NUMA). The VM in the report has -smp 2, so cpu1 is offlined.
//
// gcc -O2 -static -o reproducer reproducer.c && ./reproducer
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/mount.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <unistd.h>
#define MPOL_BIND 2
#define MPOL_F_RELATIVE_NODES (1 << 14)
static int write_file(const char *path, const char *val)
{
int fd = open(path, O_WRONLY);
if (fd < 0) {
fprintf(stderr, "[-] open(%s): %s\n", path, strerror(errno));
return -1;
}
int n = write(fd, val, strlen(val));
if (n < 0)
fprintf(stderr, "[-] write(%s, \"%s\"): %s\n", path, val,
strerror(errno));
close(fd);
return n < 0 ? -1 : 0;
}
// Find a writable cgroup2 root, mounting a fresh view if needed.
static const char *cgroup2_root(void)
{
static char root[256];
// A fresh mount of cgroup2 is just another view of the single unified
// hierarchy, so its root subtree_control is the system one.
mkdir("/tmp/cg2", 0755);
if (mount("none", "/tmp/cg2", "cgroup2", 0, NULL) == 0 ||
errno == EBUSY) {
strcpy(root, "/tmp/cg2");
if (access("/tmp/cg2/cgroup.subtree_control", F_OK) == 0)
return root;
}
// Fall back to the conventional location.
if (access("/sys/fs/cgroup/cgroup.subtree_control", F_OK) == 0) {
strcpy(root, "/sys/fs/cgroup");
return root;
}
return NULL;
}
int main(void)
{
char path[320];
const char *root = cgroup2_root();
if (!root) {
fprintf(stderr, "[-] no cgroup2 hierarchy available\n");
return 1;
}
printf("[+] cgroup2 root: %s\n", root);
// 1. Enable the cpuset controller for children of the root.
snprintf(path, sizeof(path), "%s/cgroup.subtree_control", root);
write_file(path, "+cpuset");
// 2. Create a child cpuset. Crucially, we NEVER write its cpuset.mems,
// so mems_allowed stays empty while effective_mems inherits {0}.
snprintf(path, sizeof(path), "%s/koops", root);
mkdir(path, 0755);
// 3. Move ourselves into the child (allowed in v2 even with empty mems).
snprintf(path, sizeof(path), "%s/koops/cgroup.procs", root);
char pid[32];
snprintf(pid, sizeof(pid), "%d", getpid());
if (write_file(path, pid) < 0)
fprintf(stderr, "[-] could not join child cgroup\n");
// 4. Install a VMA mempolicy with MPOL_F_RELATIVE_NODES and a non-empty
// user nodemask (node 0). This is the policy whose later rebind with
// an empty mask folds modulo zero.
void *area = mmap(NULL, 0x4000, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (area == MAP_FAILED) {
fprintf(stderr, "[-] mmap: %s\n", strerror(errno));
return 1;
}
unsigned long nodemask[16] = { 0 };
nodemask[0] = 1UL; // node 0
long r = syscall(__NR_mbind, area, 0x4000,
MPOL_BIND | MPOL_F_RELATIVE_NODES, nodemask,
sizeof(nodemask) * 8, 0);
if (r != 0)
fprintf(stderr, "[-] mbind: %s\n", strerror(errno));
else
printf("[+] installed MPOL_F_RELATIVE_NODES VMA policy\n");
// 5. Trigger CPU hotplug. cpuset_handle_hotplug() then walks descendants
// and rebinds our VMA policy with the empty mems_allowed -> #DE in the
// cpuhp/N kthread. Loop a few times to cover online/offline timing.
printf("[+] toggling cpu1 online state to trigger hotplug rebind...\n");
for (int i = 0; i < 8; i++) {
write_file("/sys/devices/system/cpu/cpu1/online", "0");
write_file("/sys/devices/system/cpu/cpu1/online", "1");
}
printf("[+] done (if the kernel did not crash, check NUMA/cpuset config)\n");
return 0;
}
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-02 15:28 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: <CAMgjq7D4XsAD4NGDL7FC2kaYAQAP8PDJdn4bpzGZwXYtjEpJ6w@mail.gmail.com>
On Mon, Jun 1, 2026 at 8:25 PM Kairui Song <ryncsn@gmail.com> wrote:
>
> On Tue, Jun 2, 2026 at 2:06 AM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > 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:
> > > >
> > > > 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).
>
> A reverse entry is enough to hold the physical swap, just like how the
> current hibernation works with a fake shadow, you don't need a PFN
> just for holding that.
>
> >
> > 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).
>
> Right, vtable extension is fine, there is no redundant data. I just
> mean you don't need to set the PFN twice (for vswap & pswap). So
> simply reusing the PFN format in the vswap layer and solving
> everything there should be enough.
Ah yeah, then I might have misunderstood you here. I thought you were
proposing a way to remove vtable :)
"don't need to set the PFN twice" completely agree. I'm pretty sure I
did not here, but do let me know if I accidentally set it twice. I'm
be sure to check this myself for the next version.
>
> > > 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'd love to :), there is a lot of work going on as you can see and
> people seem to have many different proposals about this so I didn't
> prioritize it. I'll try as things settle down.
Yeah understandable. It's a very volatile codebase, with a lot of
folks trying to improve different aspects.
Hopefully we're close to a unified design :)
I'll keep my dedicated vswap per-cpu alloc caching for now, but I'll
get rid of it whenever the per-CPU per-si cache is ready.
>
> > > > >
> > > > > 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.
>
> Right, but swap_cache_alloc_folio with orders=<large order> won't
> attempt a large allocation if the batch check fails, so that's fine.
>
> > > > 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?
>
> "Backed by zswap" means the actual swapout already happened, which is
> the case where we always have to record the memcg info because the
> folio is gone, seems still fit in the model.
Hmmm I might have misunderstood you in my last response here.
So what you are doing in that patch:
1. Charge towards folio->memcg when we allocate swap slots, but do not
record or take reference yet.
2. Once we reclaimed the folio after swap out, then we record and
acquire reference to pin.
You know what - this would simplify my usecase. For vswap entries not
backing by pswap, it *basically* just means I skip step 1 for vswap
backend. Step 2 is shared for all cases. Donezo.
You're right. This is simpler :) Let me brew on it a bit longer in
case there might be something we're missing. but it does seem like
this will reduce complexity (and with the added benefits of me not
having to come up with names for helpers).
>
> > 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.
>
> Zswap charge could be merged with vswap I think but pswap we just
> discussed that we might want to charge it differently? And actually
> vswap charge is still quite different from zswap charge if you want to
> make vswap infinitely large? I think we can figure out this part as we
> progress; it's not a major problem at this point.
That was because I misunderstood your suggestions. My bad :)
Anyway, please keep the suggestions and recommendations coming :) I'm
playing with some of your suggestions right now, and waiting for other
folks' inputs as well. Will send out the next version at some point.
If there is no fundamental design flaws, I will un-RFC once I've
addressed all the main issues.
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-02 15:54 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: <CAMgjq7CT0ccCnzmpRGjTGPnNEn4eK==5A-OFbr3+p465dQMH4A@mail.gmail.com>
On Mon, Jun 1, 2026 at 10:49 AM Kairui Song <ryncsn@gmail.com> wrote:
>
> 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.
Of course. I'm hoping vswap-on-zswap will not be too terrible at a
start. We can then optimize for the swapfile backend case later.
>
> BTW maintaining MRU is also an overhead, I'm not sure if the lookup
> pattern always follows that?
Yeah I had to be a bit careful in v6 to make sure the cache (and cache
invalidation) happens at the right time. I've had this idea for awhile
- there's a reason why I waited until v6 to implement it :)
For instance, when physical swap allocator runs out of slot for a
cluster, we try to reclaim the swap-cache-only slots. That involves
taking the rmap back to vswap layer, to check swap cache and swap
count. This is a very random pattern, so it does not benefit from this
lookup cache, and in fact invalidates the cache :) So I had to add
some hint to avoid going back to the vswap layer to check for
swap-cache-only state.
>
> > 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.
Yeah agree - we just need to be a bit craftier with it. The
fundamental problem is in the current model, we're only storing offset
and si, then look up cluster based on that. But for dynamic vswap,
that look up takes the xa_load().
Once we move to per-si per-cpu cluster, then I think it becomes ok to
store the cluster pointer directly, correct?
The reference counting needs to be carefully handled though. I think
in my old vss design I did something fairly silly - just hold a
reference to it while it's in cache, then add CPU offlining handler to
clean up. Not the end of the world I suppose, but maybe there's a
smarter scheme.
^ permalink raw reply
* Re: [PATCH] cgroup: Migrate tasks to the root css when a controller is rebound
From: Mark Brown @ 2026-06-02 16:28 UTC (permalink / raw)
To: Tejun Heo
Cc: cgroups, linux-kernel, Bert Karwatzki, 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>
[-- Attachment #1: Type: text/plain, Size: 343 bytes --]
On Mon, Jun 01, 2026 at 09:02:56AM -1000, Tejun Heo wrote:
> 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 seems to fix things for me, thanks both!
Tested-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Kairui Song @ 2026-06-02 16:43 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=M3WAkSY=Zd35dEuQ6V3ZiNR02bKAN_DnCgVr69w9=0sQ@mail.gmail.com>
On Tue, Jun 2, 2026 at 11:54 PM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Mon, Jun 1, 2026 at 10:49 AM Kairui Song <ryncsn@gmail.com> wrote:
> >
> >
> > That part should be indeed coverable by the si->percpu cluster though, I think.
>
> Yeah agree - we just need to be a bit craftier with it. The
> fundamental problem is in the current model, we're only storing offset
> and si, then look up cluster based on that. But for dynamic vswap,
> that look up takes the xa_load().
>
> Once we move to per-si per-cpu cluster, then I think it becomes ok to
> store the cluster pointer directly, correct?
>
> The reference counting needs to be carefully handled though. I think
> in my old vss design I did something fairly silly - just hold a
> reference to it while it's in cache, then add CPU offlining handler to
> clean up. Not the end of the world I suppose, but maybe there's a
> smarter scheme.
Yeah... I'm not entirely sure about this at this point, maybe it can
be sorted out as we process. Maybe we can also avoid the xa_load with
other techniques too.
^ permalink raw reply
* Re: [PATCH 1/1] mm/thp: clear deferred split shrinker bits when queues drain
From: David Hildenbrand (Arm) @ 2026-06-02 17:33 UTC (permalink / raw)
To: Lance Yang, akpm, hannes
Cc: ljs, shakeel.butt, mhocko, david, roman.gushchin, muchun.song,
qi.zheng, yosry.ahmed, ziy, liam, usama.arif, kas, vbabka, ryncsn,
zaslonko, gor, wangkefeng.wang, baolin.wang, baohua, dev.jain,
npache, ryan.roberts, cgroups, linux-mm, linux-kernel
In-Reply-To: <63797977-1c18-4885-8099-f5c21c80da39@linux.dev>
On 6/2/26 14:47, Lance Yang wrote:
>
>
> On 2026/6/2 20:11, David Hildenbrand (Arm) wrote:
>> On 6/2/26 06:38, Lance Yang wrote:
>>> Sorry, I missed Johannes in Cc ...
>>>
>>
>> What's the effect of that? Would we consider it a fix that we'd want to backport?
>
> Just a stale memcg shrinker bit :) I'd treat this patch as a small
> cleanup.
>
> Once the queue is empty, count_objects() returns 0. That skips the scan,
> but shrink_slab_memcg() only clears the bit on SHRINK_EMPTY, not 0.
>
> So memcg reclaim can keep calling the shrinker even though there is
> nothing on that queue.
>
>>>
>>
>> This is against Johannes' work, right?
>
> Yep, I noticed it there, but the behavior is older.
>
>> If this is a fix, likely it would be fixing 87eaceb3faa5 ("mm: thp: make
>> deferred split shrinker memcg aware"), right?
>
> No missed reclaim, just some extra reclaim work :)
Thanks for clarifying! :)
Reviewed-by: David Hildenbrand (Arm) <david@kernel.org>
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH] cgroup/cpuset: change ridong's email
From: Waiman Long @ 2026-06-02 18:23 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný
Cc: cgroups, linux-kernel
In-Reply-To: <20260602091038.26901-1-ridong.chen@linux.dev>
On 6/2/26 5:10 AM, Ridong Chen wrote:
> The chenridong@huaweicloud.com is no longer a valid email,
> replace it with the personal email ridong.chen@linux.dev
>
> Signed-off-by: Ridong Chen <ridong.chen@linux.dev>
> ---
> MAINTAINERS | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9ec290e38b44..e035a3be797c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6535,7 +6535,7 @@ F: include/linux/blk-cgroup.h
>
> CONTROL GROUP - CPUSET
> M: Waiman Long <longman@redhat.com>
> -R: Chen Ridong <chenridong@huaweicloud.com>
> +R: Ridong Chen <ridong.chen@linux.dev>
> L: cgroups@vger.kernel.org
> S: Maintained
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
Acked-by: Waiman Long <longman@redhat.com>
^ permalink raw reply
* Re: [PATCH] cgroup: Migrate tasks to the root css when a controller is rebound
From: Tejun Heo @ 2026-06-02 18:34 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
In-Reply-To: <20260601190256.1815778-1-tj@kernel.org>
Applied to cgroup/for-7.2.
Thanks Mark and Bert for the reports and the testing.
--
tejun
^ 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