* 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] cgroup/cpuset: Remove Chen Ridong as a cpust reviewer for now
From: Ridong Chen @ 2026-06-02 8:13 UTC (permalink / raw)
To: Michal Koutný, Waiman Long
Cc: Tejun Heo, Johannes Weiner, cgroups, linux-kernel
In-Reply-To: <ah6LAfpMsdPLun2_@localhost.localdomain>
On 2026/6/2 15:51, Michal Koutný wrote:
> +Cc: ridong.chen@linux.dev
>
> (This looks like their new address.)
>
Hi all,
Thank you, Michal.
Apologies for the email issue. I'm currently changing my company, The
ridong.chen@linux.dev email is valid.
> On Mon, Jun 01, 2026 at 10:44:22PM -0400, Waiman Long <longman@redhat.com> wrote:
>> Chen Ridong has contributed quite a lot of fixes and cleanups to the
>> cpuset code. Unfortunately, his email address is now no longer valid. So
>> remove him as a cpuset reviewer until he shows up again or someone else
>> volunteers to take his place.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>> MAINTAINERS | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 74c86cf9bc65..c7a7126ea406 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6526,7 +6526,6 @@ F: include/linux/blk-cgroup.h
>>
>> CONTROL GROUP - CPUSET
>> M: Waiman Long <longman@redhat.com>
>> -R: Chen Ridong <chenridong@huaweicloud.com>
>> L: cgroups@vger.kernel.org
>> S: Maintained
>> T: git git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
>> --
>> 2.54.0
>>
--
Best regards,
Ridong
^ permalink raw reply
* Re: [PATCH] cgroup/cpuset: Remove Chen Ridong as a cpust reviewer for now
From: Michal Koutný @ 2026-06-02 7:51 UTC (permalink / raw)
To: Waiman Long
Cc: Ridong Chen, Tejun Heo, Johannes Weiner, cgroups, linux-kernel
In-Reply-To: <20260602024422.249458-1-longman@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 974 bytes --]
+Cc: ridong.chen@linux.dev
(This looks like their new address.)
On Mon, Jun 01, 2026 at 10:44:22PM -0400, Waiman Long <longman@redhat.com> wrote:
> Chen Ridong has contributed quite a lot of fixes and cleanups to the
> cpuset code. Unfortunately, his email address is now no longer valid. So
> remove him as a cpuset reviewer until he shows up again or someone else
> volunteers to take his place.
>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
> MAINTAINERS | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 74c86cf9bc65..c7a7126ea406 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6526,7 +6526,6 @@ F: include/linux/blk-cgroup.h
>
> CONTROL GROUP - CPUSET
> M: Waiman Long <longman@redhat.com>
> -R: Chen Ridong <chenridong@huaweicloud.com>
> L: cgroups@vger.kernel.org
> S: Maintained
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
> --
> 2.54.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ permalink raw reply
* Re: [PATCH v2] cgroup/cpuset: Fix update_prstate() always returning 0 on partition errors
From: Michal Koutný @ 2026-06-02 7:46 UTC (permalink / raw)
To: Tao Cui; +Cc: longman, chenridong, tj, hannes, cgroups, linux-kernel, Tao Cui
In-Reply-To: <20260602045521.2381230-1-cui.tao@linux.dev>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ permalink raw reply
* [PATCH v2] cgroup/cpuset: Fix update_prstate() always returning 0 on partition errors
From: Tao Cui @ 2026-06-02 4:55 UTC (permalink / raw)
To: longman, chenridong, tj, hannes, mkoutny, cgroups; +Cc: linux-kernel, Tao Cui
From: Tao Cui <cuitao@kylinos.cn>
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.
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
kernel/cgroup/cpuset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 591e3aa487fc..8605b4da610e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2965,7 +2965,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
if (force_sd_rebuild)
rebuild_sched_domains_locked();
free_tmpmasks(&tmpmask);
- return 0;
+ return err ? -EINVAL : 0;
}
static struct cpuset *cpuset_attach_old_cs;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] cgroup/cpuset: Fix update_prstate() always returning 0 on partition errors
From: Tao Cui @ 2026-06-02 4:49 UTC (permalink / raw)
To: longman, chenridong, tj, hannes, mkoutny, cgroups
Cc: cui.tao, linux-kernel, Tao Cui, Claude Opus 4 . 7
In-Reply-To: <20260602043652.2380163-1-cui.tao@linux.dev>
Kindly disregard this patch.
The issue is fixed by AI auto-testing without Signed-off-by; the tag will be included in v2.
在 2026/6/2 12:36, Tao Cui 写道:
> From: Tao Cui <cuitao@kylinos.cn>
>
> 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.
>
> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
> ---
> kernel/cgroup/cpuset.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 591e3aa487fc..8605b4da610e 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2965,7 +2965,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
> if (force_sd_rebuild)
> rebuild_sched_domains_locked();
> free_tmpmasks(&tmpmask);
> - return 0;
> + return err ? -EINVAL : 0;
> }
>
> static struct cpuset *cpuset_attach_old_cs;
^ permalink raw reply
* Re: [PATCH 1/1] mm/thp: clear deferred split shrinker bits when queues drain
From: Lance Yang @ 2026-06-02 4:38 UTC (permalink / raw)
To: akpm, hannes
Cc: david, 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: <20260602043453.67597-1-lance.yang@linux.dev>
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.
>
> 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)
^ permalink raw reply
* [PATCH] cgroup/cpuset: Fix update_prstate() always returning 0 on partition errors
From: Tao Cui @ 2026-06-02 4:36 UTC (permalink / raw)
To: longman, chenridong, tj, hannes, mkoutny, cgroups
Cc: linux-kernel, Tao Cui, Claude Opus 4 . 7
From: Tao Cui <cuitao@kylinos.cn>
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.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
kernel/cgroup/cpuset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 591e3aa487fc..8605b4da610e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2965,7 +2965,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
if (force_sd_rebuild)
rebuild_sched_domains_locked();
free_tmpmasks(&tmpmask);
- return 0;
+ return err ? -EINVAL : 0;
}
static struct cpuset *cpuset_attach_old_cs;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/1] mm/thp: clear deferred split shrinker bits when queues drain
From: Lance Yang @ 2026-06-02 4:34 UTC (permalink / raw)
To: akpm
Cc: david, 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, Lance Yang
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>
---
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)
--
2.49.0
^ permalink raw reply related
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Kairui Song @ 2026-06-02 3:24 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=PmwzaJhfjBrho3+kQ8HXFUC0WiegQrsguBc-_pmn5bSA@mail.gmail.com>
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.
> > 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.
> > > >
> > > > 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.
> 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.
^ permalink raw reply
* [PATCH] cgroup/cpuset: Remove Chen Ridong as a cpust reviewer for now
From: Waiman Long @ 2026-06-02 2:44 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný
Cc: cgroups, linux-kernel, Waiman Long
Chen Ridong has contributed quite a lot of fixes and cleanups to the
cpuset code. Unfortunately, his email address is now no longer valid. So
remove him as a cpuset reviewer until he shows up again or someone else
volunteers to take his place.
Signed-off-by: Waiman Long <longman@redhat.com>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 74c86cf9bc65..c7a7126ea406 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6526,7 +6526,6 @@ F: include/linux/blk-cgroup.h
CONTROL GROUP - CPUSET
M: Waiman Long <longman@redhat.com>
-R: Chen Ridong <chenridong@huaweicloud.com>
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 v2] mm/list_lru: drain before clearing xarray entry on reparent
From: Muchun Song @ 2026-06-02 2:37 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, stable
In-Reply-To: <20260601161501.1444829-1-shakeel.butt@linux.dev>
> On Jun 2, 2026, at 00:15, 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.
I see. xas_lock_irqsave() in memcg_list_lru_alloc() functions as an RCU read lock.
Acked-by: Muchun Song <muchun.song@linux.dev>
Thanks.
> */
> - 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
* [PATCH-next v5 6/6] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach()
From: Waiman Long @ 2026-06-02 2:32 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260602023203.248077-1-longman@redhat.com>
With cgroup v2, the cgroup_taskset structure passed into the cgroup
can_attach() and attach() methods can contain task migration data with
multiple destination or source cpusets when the cpuset controller is
enabled or disabled respectively.
Since cpuset is threaded in both v1 and v2, another possible way to
cause many-to-one migration is to move the whole process with multiple
threads in different cpuset enabled threaded cgroups into another cpuset
enabled cgroup.
The current cpuset_can_attach() and cpuset_attach() functions still
expect task migration is from one source cpuset to one destination
cpuset. This has been the case since cpuset was enabled for cgroup v2
in commit 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default
hierarchy").
This problem is less an issue when enabling the cpuset controller as all
the newly created child cpusets will have exactly the same set of CPUs
and memory nodes except when deadline tasks are involved in migration
as the deadline task accounting data can be off.
It can be more problematic when the cpuset controller is disabled as
their set of CPUs and memory nodes may differ from their parent or with
the moving of multi-threaded process from different threaded cgroups.
Fix that by tracking the set of source (old) and destination cpusets
in singly linked lists and iterating them all to properly update the
internal data. Also keep the current cs and oldcs variables up-to-date
with the css and task iterators.
To ensure proper DL tasks accounting, the nr_migrate_dl_tasks in both
the source and destination cpusets are decremented/incremented with
their values added to nr_deadline_tasks when the migration is successful.
Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default hierarchy")
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset-internal.h | 6 +
kernel/cgroup/cpuset.c | 208 ++++++++++++++++++++++++--------
2 files changed, 164 insertions(+), 50 deletions(-)
diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index f7aaf01f7cd5..4c2772a7fd5e 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -161,6 +161,12 @@ struct cpuset {
*/
bool remote_partition;
+ /*
+ * cpuset_can_attach() and cpuset_attach() specific data
+ */
+ bool attach_node_in_llist;
+ struct llist_node attach_node;
+
/*
* number of SCHED_DEADLINE tasks attached to this cpuset, so that we
* know when to rebuild associated root domain bandwidth information.
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 5b5352ec0e69..53a9d3cc8407 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -37,6 +37,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/task_work.h>
+#include <linux/llist.h>
DEFINE_STATIC_KEY_FALSE(cpusets_pre_enable_key);
DEFINE_STATIC_KEY_FALSE(cpusets_enabled_key);
@@ -2983,6 +2984,8 @@ static int update_prstate(struct cpuset *cs, int new_prs)
* cpuset_attach() or cpuset_fork() and used in cpuset_attach_task().
*/
static struct cpuset *cpuset_attach_old_cs;
+static LLIST_HEAD(src_cs_head);
+static LLIST_HEAD(dst_cs_head);
static bool attach_cpus_updated;
static bool attach_mems_updated;
@@ -3005,6 +3008,15 @@ static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
if (!oldcs)
return 0;
+ if (!cs->attach_node_in_llist) {
+ llist_add(&cs->attach_node, &dst_cs_head);
+ cs->attach_node_in_llist = true;
+ }
+ if (!oldcs->attach_node_in_llist) {
+ llist_add(&oldcs->attach_node, &src_cs_head);
+ oldcs->attach_node_in_llist = true;
+ }
+
/*
* Skip rights over task setsched check in v2 when nothing changes,
* migration permission derives from hierarchy ownership in
@@ -3027,33 +3039,101 @@ static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
return 0;
}
-static int cpuset_reserve_dl_bw(struct cpuset *cs)
+/*
+ * If reset_dl_bw is set, reset the previous dl_bw_alloc() call. Otherwise,
+ * update nr_deadline_tasks according to nr_migrate_dl_tasks in both source
+ * and destination cpusets.
+ */
+static void clear_attach_data(bool reset_dl_bw)
{
+ struct cpuset *cs, *next;
+
+ llist_for_each_entry_safe(cs, next, src_cs_head.first, attach_node) {
+ cs->attach_node.next = NULL;
+ cs->attach_node_in_llist = false;
+ if (cs->nr_migrate_dl_tasks && !reset_dl_bw)
+ cs->nr_deadline_tasks += cs->nr_migrate_dl_tasks;
+ cs->nr_migrate_dl_tasks = 0;
+ }
+
+ llist_for_each_entry_safe(cs, next, dst_cs_head.first, attach_node) {
+ cs->attach_node.next = NULL;
+ cs->attach_node_in_llist = false;
+ if (reset_dl_bw && cs->dl_bw_cpu >= 0)
+ dl_bw_free(cs->dl_bw_cpu, cs->sum_migrate_dl_bw);
+ if (cs->nr_migrate_dl_tasks && !reset_dl_bw)
+ cs->nr_deadline_tasks += cs->nr_migrate_dl_tasks;
+ cs->nr_migrate_dl_tasks = 0;
+ cs->sum_migrate_dl_bw = 0;
+ cs->dl_bw_cpu = -1;
+ }
+
+ src_cs_head.first = NULL;
+ dst_cs_head.first = NULL;
+}
+
+static int cpuset_reserve_dl_bw(void)
+{
+ struct cpuset *cs;
int cpu, ret;
- if (!cs->sum_migrate_dl_bw)
- return 0;
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node) {
+ if (!cs->sum_migrate_dl_bw)
+ continue;
- cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
- if (unlikely(cpu >= nr_cpu_ids))
- return -EINVAL;
+ 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;
+ ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
+ if (ret)
+ return ret;
- cs->dl_bw_cpu = cpu;
+ cs->dl_bw_cpu = cpu;
+ }
return 0;
}
-static void reset_migrate_dl_data(struct cpuset *cs)
+static void set_attach_in_progress(void)
{
- cs->nr_migrate_dl_tasks = 0;
- cs->sum_migrate_dl_bw = 0;
- cs->dl_bw_cpu = -1;
+ struct cpuset *cs;
+
+ /*
+ * Mark attach is in progress. This makes validate_change() fail
+ * changes which zero cpus/mems_allowed.
+ */
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node)
+ cs->attach_in_progress++;
+}
+
+static void reset_attach_in_progress(void)
+{
+ struct cpuset *cs;
+
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node)
+ dec_attach_in_progress_locked(cs);
}
-/* Called by cgroups to determine if a cpuset is usable; cpuset_mutex held */
+/*
+ * Called by cgroups to determine if a cpuset is usable; cpuset_mutex held.
+ *
+ * With cgroup v2, enabling of cpuset controller in a cgroup subtree can
+ * cause @tset to contain task migration data from one parent cpuset to multiple
+ * child cpusets. Not much is needed to be done here other than tracking the
+ * number of DL tasks in each cpuset as the CPUs and memory nodes of the child
+ * cpusets are exactly the same as the parent.
+ *
+ * Conversely, disabling of cpuset controller can cause @tset to contain task
+ * migration data from multiple child cpusets to one parent cpuset. Here, the
+ * CPUs and memory nodes of the child cpusets may be different from the parent,
+ * but must be a subset of its parent.
+ *
+ * Another possible many-to-one migration is the moving of the whole
+ * multithreaded process with threads in different cpusets to another cpuset.
+ *
+ * For all other use cases, @tset task migration data should be from one source
+ * cpuset to one destination cpuset.
+ */
static int cpuset_can_attach(struct cgroup_taskset *tset)
{
struct cgroup_subsys_state *css;
@@ -3092,6 +3172,16 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
* selected as cpuset_attach_old_cs.
*/
cgroup_taskset_for_each(task, css, tset) {
+ struct cpuset *newcs = css_cs(css);
+ struct cpuset *new_oldcs = task_cs(task);
+
+ if ((newcs != cs) || (new_oldcs != oldcs)) {
+ cs = newcs;
+ oldcs = new_oldcs;
+ ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
+ if (ret)
+ goto out_unlock;
+ }
ret = task_can_attach(task);
if (ret)
goto out_unlock;
@@ -3113,23 +3203,19 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
* contribute to sum_migrate_dl_bw.
*/
cs->nr_migrate_dl_tasks++;
+ oldcs->nr_migrate_dl_tasks--;
if (dl_task_needs_bw_move(task, cs->effective_cpus))
cs->sum_migrate_dl_bw += task->dl.dl_bw;
}
}
- ret = cpuset_reserve_dl_bw(cs);
+ ret = cpuset_reserve_dl_bw();
out_unlock:
- 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++;
- }
+ if (ret)
+ clear_attach_data(true);
+ else
+ set_attach_in_progress();
mutex_unlock(&cpuset_mutex);
return ret;
@@ -3144,14 +3230,8 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
cs = css_cs(css);
mutex_lock(&cpuset_mutex);
- dec_attach_in_progress_locked(cs);
-
- if (cs->dl_bw_cpu >= 0)
- dl_bw_free(cs->dl_bw_cpu, cs->sum_migrate_dl_bw);
-
- if (cs->nr_migrate_dl_tasks)
- reset_migrate_dl_data(cs);
-
+ reset_attach_in_progress();
+ clear_attach_data(true);
mutex_unlock(&cpuset_mutex);
}
@@ -3224,48 +3304,76 @@ static void cpuset_attach(struct cgroup_taskset *tset)
struct task_struct *task;
struct cgroup_subsys_state *css;
struct cpuset *cs;
- struct cpuset *oldcs = cpuset_attach_old_cs;
+ bool many_cs_to_one = !!src_cs_head.first->next;
cgroup_taskset_first(tset, &css);
- cs = css_cs(css);
lockdep_assert_cpus_held(); /* see cgroup_attach_lock() */
mutex_lock(&cpuset_mutex);
queue_task_work = false;
- attach_cpus_updated = !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus);
- attach_mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
+ /*
+ * attach_cpus_updated/attach_mems_updated can be set to false if
+ * source and destination masks are the same and there is only one
+ * source cpuset. IOW, a many-cs-to-one migration is always treated as
+ * updated as the tasks to old cpuset mapping is lost.
+ */
+ if (many_cs_to_one) {
+ attach_cpus_updated = true;
+ attach_mems_updated = true;
+ } else {
+ /* one_cs_to_one or one_cs_to_many */
+ struct cpuset *oldcs = cpuset_attach_old_cs;
+ attach_cpus_updated = false;
+ attach_mems_updated = false;
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node) {
+ attach_cpus_updated |= !cpumask_equal(cs->effective_cpus,
+ oldcs->effective_cpus);
+ attach_mems_updated |= !nodes_equal(cs->effective_mems,
+ oldcs->effective_mems);
+ }
+ }
+
+ cs = css_cs(css);
/*
* In the default hierarchy, enabling cpuset in the child cgroups
* 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.
+ * iteration and update, but the destination cpuset list is iterated to
+ * set old_mems_sllowed.
*/
if (cpuset_v2()) {
cpuset_attach_nodemask_to = cs->effective_mems;
- if (!attach_cpus_updated && !attach_mems_updated)
+ if (!attach_cpus_updated && !attach_mems_updated) {
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node)
+ cs->old_mems_allowed = cs->effective_mems;
goto out;
+ }
} else {
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
}
- cgroup_taskset_for_each(task, css, tset)
+ cgroup_taskset_for_each(task, css, tset) {
+ struct cpuset *newcs = css_cs(css);
+
+ if (newcs != cs) {
+ cs->old_mems_allowed = cpuset_attach_nodemask_to;
+ cs = newcs;
+ if (cpuset_v2())
+ cpuset_attach_nodemask_to = cs->effective_mems;
+ else
+ guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
+ }
cpuset_attach_task(cs, task);
+ }
-out:
if (queue_task_work)
schedule_flush_migrate_mm();
cs->old_mems_allowed = cpuset_attach_nodemask_to;
-
- if (cs->nr_migrate_dl_tasks) {
- cs->nr_deadline_tasks += cs->nr_migrate_dl_tasks;
- oldcs->nr_deadline_tasks -= cs->nr_migrate_dl_tasks;
- reset_migrate_dl_data(cs);
- }
-
- dec_attach_in_progress_locked(cs);
-
+out:
+ reset_attach_in_progress();
+ clear_attach_data(false);
mutex_unlock(&cpuset_mutex);
}
--
2.54.0
^ permalink raw reply related
* [PATCH-next v5 5/6] cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside cpuset_attach_task()
From: Waiman Long @ 2026-06-02 2:32 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260602023203.248077-1-longman@redhat.com>
The cpuset_attach_task() was introduced in commit 42a11bf5c543
("cgroup/cpuset: Make cpuset_fork() handle CLONE_INTO_CGROUP properly")
to enable the CLONE_INTO_CGROUP flag of clone(2) to behave more like
moving a task from one cpuset into another one. That commits didn't
move the mpol_rebind_mm() and cpuset_migrate_mm() calls for group leader
into cpuset_attach_task().
When the CLONE_INTO_CGROUP flag is used without CLONE_THREAD, the new
task is its own group leader. So it is still not equivalent to moving
task between cpusets in this case. Make CLONE_INTO_CGROUP behaves
more close to cpuset_attach() by moving the mpol_rebind_mm() and
cpuset_migrate_mm() calls inside cpuset_attach_task(). As a result,
the following static variables will have to be updated in cpuset_fork().
- cpuset_attach_old_cs
- attach_cpus_updated
- attach_mems_updated
- queue_task_work
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 103 ++++++++++++++++++++++++-----------------
1 file changed, 60 insertions(+), 43 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 60e8149cc907..5b5352ec0e69 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2978,8 +2978,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
/*
* cpuset_can_attach() and cpuset_attach() specific internal data
* Protected by cpuset_mutex
+ *
+ * The attach_cpus_updated/attach_mems_updated flags are set in either
+ * cpuset_attach() or cpuset_fork() and used in cpuset_attach_task().
*/
static struct cpuset *cpuset_attach_old_cs;
+static bool attach_cpus_updated;
+static bool attach_mems_updated;
/*
* Check to see if a cpuset can accept a new task
@@ -3157,9 +3162,12 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
*/
static cpumask_var_t cpus_attach;
static nodemask_t cpuset_attach_nodemask_to;
+static bool queue_task_work;
static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
{
+ struct mm_struct *mm;
+
lockdep_assert_cpuset_lock_held();
if (cs != &top_cpuset)
@@ -3173,28 +3181,60 @@ static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
*/
WARN_ON_ONCE(set_cpus_allowed_ptr(task, cpus_attach));
+ if (cpuset_v2() && !attach_mems_updated)
+ return;
+
cpuset_change_task_nodemask(task, &cpuset_attach_nodemask_to);
cpuset1_update_task_spread_flags(cs, task);
+
+ if ((task != task->group_leader) ||
+ (!is_memory_migrate(cs) && !attach_mems_updated))
+ return;
+
+ /*
+ * Change mm for threadgroup leader. This is expensive and may
+ * sleep and should be moved outside migration path proper.
+ */
+ mm = get_task_mm(task);
+ if (mm) {
+ struct cpuset *oldcs = cpuset_attach_old_cs;
+
+ mpol_rebind_mm(mm, &cs->effective_mems);
+
+ /*
+ * old_mems_allowed is the same with mems_allowed
+ * here, except if this task is being moved
+ * automatically due to hotplug. In that case
+ * @mems_allowed has been updated and is empty, so
+ * @old_mems_allowed is the right nodesets that we
+ * migrate mm from.
+ */
+ if (is_memory_migrate(cs)) {
+ cpuset_migrate_mm(mm, &oldcs->old_mems_allowed,
+ &cpuset_attach_nodemask_to);
+ queue_task_work = true;
+ } else {
+ mmput(mm);
+ }
+ }
}
static void cpuset_attach(struct cgroup_taskset *tset)
{
struct task_struct *task;
- struct task_struct *leader;
struct cgroup_subsys_state *css;
struct cpuset *cs;
struct cpuset *oldcs = cpuset_attach_old_cs;
- bool cpus_updated, mems_updated;
- bool queue_task_work = false;
cgroup_taskset_first(tset, &css);
cs = css_cs(css);
lockdep_assert_cpus_held(); /* see cgroup_attach_lock() */
mutex_lock(&cpuset_mutex);
- cpus_updated = !cpumask_equal(cs->effective_cpus,
- oldcs->effective_cpus);
- mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
+ queue_task_work = false;
+
+ attach_cpus_updated = !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus);
+ attach_mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
/*
* In the default hierarchy, enabling cpuset in the child cgroups
@@ -3204,7 +3244,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
*/
if (cpuset_v2()) {
cpuset_attach_nodemask_to = cs->effective_mems;
- if (!cpus_updated && !mems_updated)
+ if (!attach_cpus_updated && !attach_mems_updated)
goto out;
} else {
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
@@ -3213,38 +3253,6 @@ static void cpuset_attach(struct cgroup_taskset *tset)
cgroup_taskset_for_each(task, css, tset)
cpuset_attach_task(cs, task);
- /*
- * Change mm for all threadgroup leaders. This is expensive and may
- * sleep and should be moved outside migration path proper. Skip it
- * if there is no change in effective_mems and CS_MEMORY_MIGRATE is
- * not set.
- */
- if (!is_memory_migrate(cs) && !mems_updated)
- goto out;
-
- cgroup_taskset_for_each_leader(leader, css, tset) {
- struct mm_struct *mm = get_task_mm(leader);
-
- if (mm) {
- mpol_rebind_mm(mm, &cs->effective_mems);
-
- /*
- * old_mems_allowed is the same with mems_allowed
- * here, except if this task is being moved
- * automatically due to hotplug. In that case
- * @mems_allowed has been updated and is empty, so
- * @old_mems_allowed is the right nodesets that we
- * migrate mm from.
- */
- if (is_memory_migrate(cs)) {
- cpuset_migrate_mm(mm, &oldcs->old_mems_allowed,
- &cpuset_attach_nodemask_to);
- queue_task_work = true;
- } else
- mmput(mm);
- }
- }
-
out:
if (queue_task_work)
schedule_flush_migrate_mm();
@@ -3678,15 +3686,14 @@ static void cpuset_cancel_fork(struct task_struct *task, struct css_set *cset)
*/
static void cpuset_fork(struct task_struct *task)
{
- struct cpuset *cs;
- bool same_cs;
+ struct cpuset *cs, *oldcs;
rcu_read_lock();
cs = task_cs(task);
- same_cs = (cs == task_cs(current));
+ oldcs = task_cs(current);
rcu_read_unlock();
- if (same_cs) {
+ if (cs == oldcs) {
if (cs == &top_cpuset)
return;
@@ -3698,7 +3705,17 @@ static void cpuset_fork(struct task_struct *task)
/* CLONE_INTO_CGROUP */
mutex_lock(&cpuset_mutex);
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
+ /*
+ * Assume CPUs and memory nodes are updated
+ * A CLONE_INTO_CGROUP operation should have taken the cgroup mutex
+ * and so there shouldn't be a competing cpuset_attach() operation.
+ */
+ attach_cpus_updated = attach_mems_updated = true;
+ queue_task_work = false;
+ cpuset_attach_old_cs = oldcs;
cpuset_attach_task(cs, task);
+ if (queue_task_work)
+ schedule_flush_migrate_mm();
dec_attach_in_progress_locked(cs);
mutex_unlock(&cpuset_mutex);
--
2.54.0
^ permalink raw reply related
* [PATCH-next v5 4/6] cgroup/cpuset: Make cpuset_attach_old_cs track task group leaders
From: Waiman Long @ 2026-06-02 2:32 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long,
Ridong Chen
In-Reply-To: <20260602023203.248077-1-longman@redhat.com>
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
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>
---
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)
--
2.54.0
^ permalink raw reply related
* [PATCH-next v5 3/6] cgroup/cpuset: Expand the scope of cpuset_can_attach_check()
From: Waiman Long @ 2026-06-02 2:32 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260602023203.248077-1-longman@redhat.com>
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.
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>
---
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;
--
2.54.0
^ permalink raw reply related
* [PATCH-next v5 2/6] cgroup/cpuset: Add a cpuset_reserve_dl_bw() helper
From: Waiman Long @ 2026-06-02 2:31 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260602023203.248077-1-longman@redhat.com>
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>
---
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;
}
--
2.54.0
^ permalink raw reply related
* [PATCH-next v5 1/6] cgroup/cpuset: Fix node inconsistencies between cpuset_update_tasks_nodemask() and cpuset_attach()
From: Waiman Long @ 2026-06-02 2:31 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260602023203.248077-1-longman@redhat.com>
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.
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()
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.
*/
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
--
2.54.0
^ permalink raw reply related
* [PATCH-next v5 0/6] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach()
From: Waiman Long @ 2026-06-02 2:31 UTC (permalink / raw)
To: Chen Ridong, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
v5:
- Remove the WARN_ON() call as it can be triggered in a corner case.
- Instead of passing an attach_cpus_updated and attach_mems_updated
flags from cpuset_can_attach() to cpuset_attach(), re-evaluate the
flags at the beginning of cpuset_attach() based on data in the source &
destination cpusets in the singly linked lists to eliminate the
Time-of-Check to Time-of-Use (TOCTOU) race condition & simplify the
code changes.
- Add back the dropped optimization in patch 5.
v4:
- Add a new patch 1 to fix inconsistency in node mask usage in
cpuset_update_tasks_nodemask() and cpuset_attach() and adjust
the subsequent patches accordingly.
- Update patch 3 to set the update flags whenever the CPU or node
mask is updated to address issue reported by Sashiko.
- Update patch 5 to remove unneeded setting of old_mems_allowed as
well as calling schedule_flush_migrate_mm() if queue_task_work is
set.
v3:
- Rebased to the lastest linux-next tree.
- Keep cpuset_attach_old_cs as suggested by Chen Ridong and replace
patch 3 by a new one to make it track task group leader.
Sashiko AI review of another cpuset patch had found that cpuset_attach()
and cpuset_can_attach() can be passed a cgroup_taskset with tasks
migrating from one source cpuset to multiple destination cpusets and
vice versa. Further testing of the cpuset code indicates that this is
indeed the case when the v2 cpuset controller is enabled or disabled.
Unfortunately, cpuset_attach() and cpuset_can_attach() still assume that
there will be one source and one destinaton cpuset which may result in
inocrrect behavior.
This patch series is created to fix this issue.
Patch 1 is to fix an inconsistency in the way node mask update is being
handled in cpuset_update_tasks_nodemask() and cpuset_attach() so that
they match each other.
Patches 2 and 3 are just preparatory patches to make the remaining
patches easier to review.
Patch 4 makes cpuset_attach_old_cs to track group leader for use by
cpuset_migrate_mm().
Patch 5 moves mpol_rebind_mm() and cpuset_migrate_mm() inside
cpuset_attach_task() to make CLONE_INTO_CGROUP flag of clone(2) works
more like moving task from one cpuset to another one, while also make
supporting multiple source and destination cpusets easier.
Patch 6 makes the necessary changes to enable the support of multiple
source and destination cpusets by keeping all the source and destination
cpusets found during task iterations in two singly linked lists for
source and destination cpusets respectively.
Waiman Long (6):
cgroup/cpuset: Fix node inconsistencies between
cpuset_update_tasks_nodemask() and cpuset_attach()
cgroup/cpuset: Add a cpuset_reserve_dl_bw() helper
cgroup/cpuset: Expand the scope of cpuset_can_attach_check()
cgroup/cpuset: Make cpuset_attach_old_cs track task group leaders
cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside
cpuset_attach_task()
cgroup/cpuset: Support multiple source/destination cpusets for
cpuset_*attach()
kernel/cgroup/cpuset-internal.h | 6 +
kernel/cgroup/cpuset.c | 411 +++++++++++++++++++++++---------
2 files changed, 299 insertions(+), 118 deletions(-)
--
2.54.0
^ 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
* 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: [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] 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] 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
* [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
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