* Re: [RFC PATCH v5 20/29] sched/deadline: Allow deeper hierarchies of RT cgroups
From: luca abeni @ 2026-05-07 16:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, Yuri Andriaccio, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, linux-kernel, Yuri Andriaccio,
hannes, mkoutny, cgroups
In-Reply-To: <20260507105331.GQ1026330@noisy.programming.kicks-ass.net>
Hi,
On Thu, 7 May 2026 12:53:31 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
[...]
> > - This has the same problem with cgroup1's rt cgroup sched support
> > where there is no way to have a permissive default configuration,
> > which means that users who don't really care about distributing rt
> > shares hierarchically would get blocked from running rt processes
> > by default, which basically forces distros to disable rt cgroup
> > sched support. This is not new but it'd be a shame to put in all
> > the work and the end result is that most people don't even have
> > access to the feature.
>
> Right, but cgroup-v2 allows enabling/disabling specific controllers
> for a (sub)-hierarchy, right? So if the controller is not enabled (by
> default), it will fall back to putting the tasks in whatever parent
> does have it on, and by default the root group would have and would
> accept tasks.
If I understand well, this is similar to what I was thinking about:
having a default that allows creating FIFO/RR tasks (and execute them
without runtime control - so, without being served by a dl server)
> Additionally, I think we want a flag to allow non-priv tasks to use RT
> inside the controller -- after all, these tasks would be subject to
> strict bandwidth controls and cannot burn the system like
> unbounded/root FIFO tasks can.
This is something Yuri and I wanted to propose as a follow-up patch,
once there is an agreement on the patchset (should be a pretty simple
change :)
Luca
^ permalink raw reply
* Re: [RFC PATCH v5 20/29] sched/deadline: Allow deeper hierarchies of RT cgroups
From: luca abeni @ 2026-05-07 16:39 UTC (permalink / raw)
To: Juri Lelli
Cc: Peter Zijlstra, Tejun Heo, Yuri Andriaccio, Ingo Molnar,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, linux-kernel, Yuri Andriaccio,
hannes, mkoutny, cgroups
In-Reply-To: <afypzfyH0M7Rcge2@jlelli-thinkpadt14gen4.remote.csb>
Hi,
On Thu, 7 May 2026 17:03:41 +0200
Juri Lelli <juri.lelli@redhat.com> wrote:
> On 07/05/26 12:53, Peter Zijlstra wrote:
> > On Tue, May 05, 2026 at 09:56:58AM -1000, Tejun Heo wrote:
>
> ...
>
> > > - However, the cpu controller is a threaded controller which
> > > means that it can have threaded sub-hierarchy where the
> > > no-internal-process rule doesn't apply. This was created
> > > explicitly for cpu controller. The proposed change blocks it
> > > effectively forcing cpu controller into regular domain controller
> > > behavior subject to no-internal-process rule. Note these are
> > > enforced at controller granularity and this means that users who
> > > use the threaded mode will be forced to pick between the two.
> >
> > Right... this then means we need two controls, one to do
> > hierarchical bandwidth distribution, and one to assign bandwidth to
> > the internal group -- which is then subject to its own bandwidth
> > distribution constraint.
> >
> > This might be a little confusing, but there is no way around that
> > AFAICT.
>
> Just to check if I'm following, you are thinking something like below?
>
> groupA/
> cpu.rt.max = "50 50 100" <- 0.5 from root
> cpu.rt.internal = "20 20 100" <- 0.2 from groupA for threads at
> this level
> + threadA <
> + threadB <
> +- group1/
> cpu.rt.max = "30 30 100" <- 0.3 from groupA
> + threadC
>
> And we still keep it flat, so 2 dl-entities (per CPU), one handles
> threads at groupA level and the other threads inside group1?
An alternative idea I was thinking about: we create 2 dl entities (one
for "groupA" and one for "group1"); we set cpu.rt.max for groupA, and
we subtract group1's utilization from it (so, if groupA's cpu.rt.max is
"50 100" and group1's cpu.rt.max is "30 100", groupA is served by a dl
entity (50-30,100)=(20,100) while group1 is served by a dl entity
(30,100)).
Basically, with this idea the "internal" reservation is automatically
computed based on rt.max and on the children cgroups. A possible issue
is that if the children consume all the groupA's utilization the groupA
RT tasks remain with 0 runtime (and never execute).
Luca
^ permalink raw reply
* Re: [PATCH v2 2/2] cgroup/cpuset: align DL bandwidth reservation with attach target mask
From: Waiman Long @ 2026-05-07 15:52 UTC (permalink / raw)
To: Guopeng Zhang, Tejun Heo, Michal Koutný, Ingo Molnar,
Peter Zijlstra, Juri Lelli
Cc: Chen Ridong, Johannes Weiner, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Gabriele Monaco, Will Deacon, linux-kernel,
cgroups
In-Reply-To: <20260507103310.35849-3-zhangguopeng@kylinos.cn>
On 5/7/26 6:33 AM, Guopeng Zhang wrote:
> cpuset_can_attach() preallocates destination SCHED_DEADLINE bandwidth
> before the attach commit point, while set_cpus_allowed_dl() later
> subtracts bandwidth from the source root domain when the task affinity is
> actually updated.
>
> Those two decisions must be made with the same CPU mask.
> cpuset_can_attach() used the destination cpuset effective mask directly,
> but cpuset_attach_task() first builds a per-task target mask which is
> constrained by task_cpu_possible_mask() and, if needed, by walking up the
> cpuset hierarchy. On asymmetric systems, the actual target mask can
> therefore be a strict subset of cs->effective_cpus.
The task_cpu_possible_mask() is there for a special class of arm64 CPUs
where only some of the cores are able to run legacy 32-bit applications
on 64-bit arm CPUs. We can argue how likely that a DL task can be a
legacy 32 bit application that is inherently slower than the same
application compiled into native 64-bit code. Perhaps we can just
disallow such a legacy 32-bit application from moving to a DL scheduling
class in the first place.
I am not in favor of the idea of making the cpuset code more complex to
support such a corner case which may never be utilized. Could you strip
out the task_possible_cpu_mask() part from this patch? We can revisit
this with another patch if such a special use case can be useful to
support in the future.
Cheers,
Longman
>
> If the source root domain intersects cs->effective_cpus only on CPUs
> outside the task's possible mask, can_attach() can skip the destination
> reservation even though set_cpus_allowed_dl() later sees a real
> root-domain move and subtracts from the source domain.
>
> Extract the root-domain bandwidth-move test used by
> set_cpus_allowed_dl() into dl_task_needs_bw_move(), and make
> cpuset_can_attach() compute the same per-task target mask that
> cpuset_attach_task() applies.
>
> Keep nr_migrate_dl_tasks counting all migrating deadline tasks for
> cpuset DL task accounting. Restrict sum_migrate_dl_bw to the subset of
> tasks that need destination root-domain bandwidth reservation, because a
> deadline task can move between cpusets without moving bandwidth between
> root domains.
>
> This keeps the existing per-attach aggregate reservation model; it only
> changes the per-task mask used to decide which tasks contribute to that
> aggregate. The broader can_attach()/attach() transaction window is left
> unchanged.
>
> Fixes: 431c69fac05b ("cpuset: Honour task_cpu_possible_mask() in guarantee_online_cpus()")
> Fixes: 2ef269ef1ac0 ("cgroup/cpuset: Free DL BW in case can_attach() fails")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> include/linux/sched/deadline.h | 9 +++
> kernel/cgroup/cpuset-internal.h | 1 +
> kernel/cgroup/cpuset.c | 97 ++++++++++++++++++++++-----------
> kernel/sched/deadline.c | 13 ++++-
> 4 files changed, 86 insertions(+), 34 deletions(-)
>
> diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h
> index 1198138cb839..ddfd5216f3fc 100644
> --- a/include/linux/sched/deadline.h
> +++ b/include/linux/sched/deadline.h
> @@ -33,6 +33,15 @@ struct root_domain;
> extern void dl_add_task_root_domain(struct task_struct *p);
> extern void dl_clear_root_domain(struct root_domain *rd);
> extern void dl_clear_root_domain_cpu(int cpu);
> +/*
> + * Return whether moving DL task @p to @new_mask requires moving DL
> + * bandwidth accounting between root domains. This helper is specific to
> + * DL bandwidth move accounting semantics and is shared by
> + * cpuset_can_attach() and set_cpus_allowed_dl() so both paths use the
> + * same source root-domain test.
> + */
> +bool dl_task_needs_bw_move(struct task_struct *p,
> + const struct cpumask *new_mask);
>
> extern u64 dl_cookie;
> extern bool dl_bw_visited(int cpu, u64 cookie);
> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> index bb4e692bea30..f7aaf01f7cd5 100644
> --- a/kernel/cgroup/cpuset-internal.h
> +++ b/kernel/cgroup/cpuset-internal.h
> @@ -167,6 +167,7 @@ struct cpuset {
> */
> int nr_deadline_tasks;
> int nr_migrate_dl_tasks;
> + /* DL bandwidth that needs destination reservation for this attach. */
> u64 sum_migrate_dl_bw;
> /*
> * CPU used for temporary DL bandwidth allocation during attach;
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index ae41736399a1..78c1a4071cc3 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -485,6 +485,30 @@ static void guarantee_active_cpus(struct task_struct *tsk,
> rcu_read_unlock();
> }
>
> +/* Compute the effective CPU mask cpuset_attach_task() will apply to @tsk. */
> +static void cpuset_attach_task_cpus(struct cpuset *cs, struct task_struct *tsk,
> + struct cpumask *pmask)
> +{
> + const struct cpumask *possible_mask = task_cpu_possible_mask(tsk);
> +
> + lockdep_assert_cpuset_lock_held();
> +
> + if (cs == &top_cpuset) {
> + cpumask_andnot(pmask, possible_mask, subpartitions_cpus);
> + return;
> + }
> +
> + if (WARN_ON(!cpumask_and(pmask, possible_mask, cpu_active_mask)))
> + cpumask_copy(pmask, cpu_active_mask);
> +
> + rcu_read_lock();
> + while (!cpumask_intersects(cs->effective_cpus, pmask))
> + cs = parent_cs(cs);
> +
> + cpumask_and(pmask, pmask, cs->effective_cpus);
> + rcu_read_unlock();
> +}
> +
> /*
> * Return in *pmask the portion of a cpusets's mems_allowed that
> * are online, with memory. If none are online with memory, walk
> @@ -2986,6 +3010,14 @@ static void reset_migrate_dl_data(struct cpuset *cs)
> cs->dl_bw_cpu = -1;
> }
>
> +/*
> + * Protected by cpuset_mutex. cpus_attach is used by the can_attach/attach
> + * paths but we can't allocate it dynamically there. Define it global and
> + * allocate from cpuset_init().
> + */
> +static cpumask_var_t cpus_attach;
> +static nodemask_t cpuset_attach_nodemask_to;
> +
> /* Called by cgroups to determine if a cpuset is usable; cpuset_mutex held */
> static int cpuset_can_attach(struct cgroup_taskset *tset)
> {
> @@ -2993,7 +3025,7 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> struct cpuset *cs, *oldcs;
> struct task_struct *task;
> bool setsched_check;
> - int ret;
> + int cpu = nr_cpu_ids, ret;
>
> /* used later by cpuset_attach() */
> cpuset_attach_old_cs = task_cs(cgroup_taskset_first(tset, &css));
> @@ -3038,32 +3070,47 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> }
>
> if (dl_task(task)) {
> + /*
> + * Count all migrating DL tasks for cpuset task accounting.
> + * Only tasks that need a root-domain bandwidth move
> + * contribute to sum_migrate_dl_bw.
> + */
> cs->nr_migrate_dl_tasks++;
> - cs->sum_migrate_dl_bw += task->dl.dl_bw;
> + cpuset_attach_task_cpus(cs, task, cpus_attach);
> +
> + if (dl_task_needs_bw_move(task, cpus_attach)) {
> + /*
> + * Keep the existing aggregate reservation model.
> + * Tasks in one attach enter the same destination
> + * cpuset, so the first CPU found for a task needing
> + * DL bandwidth reservation identifies the destination
> + * root domain.
> + */
> + if (cpu >= nr_cpu_ids)
> + cpu = cpumask_any_and(cpu_active_mask,
> + cpus_attach);
> + cs->sum_migrate_dl_bw += task->dl.dl_bw;
> + }
> }
> }
>
> - if (!cs->nr_migrate_dl_tasks)
> + if (!cs->sum_migrate_dl_bw)
> goto out_success;
>
> - if (!cpumask_intersects(oldcs->effective_cpus, cs->effective_cpus)) {
> - int cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
> -
> - if (unlikely(cpu >= nr_cpu_ids)) {
> - reset_migrate_dl_data(cs);
> - ret = -EINVAL;
> - goto out_unlock;
> - }
> -
> - ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
> - if (ret) {
> - reset_migrate_dl_data(cs);
> - goto out_unlock;
> - }
> + if (unlikely(cpu >= nr_cpu_ids)) {
> + reset_migrate_dl_data(cs);
> + ret = -EINVAL;
> + goto out_unlock;
> + }
>
> - cs->dl_bw_cpu = cpu;
> + ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
> + if (ret) {
> + reset_migrate_dl_data(cs);
> + goto out_unlock;
> }
>
> + cs->dl_bw_cpu = cpu;
> +
> out_success:
> /*
> * Mark attach is in progress. This makes validate_change() fail
> @@ -3099,23 +3146,11 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
> mutex_unlock(&cpuset_mutex);
> }
>
> -/*
> - * Protected by cpuset_mutex. cpus_attach is used only by cpuset_attach_task()
> - * but we can't allocate it dynamically there. Define it global and
> - * allocate from cpuset_init().
> - */
> -static cpumask_var_t cpus_attach;
> -static nodemask_t cpuset_attach_nodemask_to;
> -
> static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
> {
> lockdep_assert_cpuset_lock_held();
>
> - if (cs != &top_cpuset)
> - guarantee_active_cpus(task, cpus_attach);
> - else
> - cpumask_andnot(cpus_attach, task_cpu_possible_mask(task),
> - subpartitions_cpus);
> + cpuset_attach_task_cpus(cs, task, cpus_attach);
> /*
> * can_attach beforehand should guarantee that this doesn't
> * fail. TODO: have a better way to handle failure here
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index edca7849b165..7db4c87df83b 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -3107,20 +3107,18 @@ static void task_woken_dl(struct rq *rq, struct task_struct *p)
> static void set_cpus_allowed_dl(struct task_struct *p,
> struct affinity_context *ctx)
> {
> - struct root_domain *src_rd;
> struct rq *rq;
>
> WARN_ON_ONCE(!dl_task(p));
>
> rq = task_rq(p);
> - src_rd = rq->rd;
> /*
> * Migrating a SCHED_DEADLINE task between exclusive
> * cpusets (different root_domains) entails a bandwidth
> * update. We already made space for us in the destination
> * domain (see cpuset_can_attach()).
> */
> - if (!cpumask_intersects(src_rd->span, ctx->new_mask)) {
> + if (dl_task_needs_bw_move(p, ctx->new_mask)) {
> struct dl_bw *src_dl_b;
>
> src_dl_b = dl_bw_of(cpu_of(rq));
> @@ -3137,6 +3135,15 @@ static void set_cpus_allowed_dl(struct task_struct *p,
> set_cpus_allowed_common(p, ctx);
> }
>
> +bool dl_task_needs_bw_move(struct task_struct *p,
> + const struct cpumask *new_mask)
> +{
> + if (!dl_task(p))
> + return false;
> +
> + return !cpumask_intersects(task_rq(p)->rd->span, new_mask);
> +}
> +
> /* Assumes rq->lock is held */
> static void rq_online_dl(struct rq *rq)
> {
^ permalink raw reply
* Re: [RFC PATCH v5 20/29] sched/deadline: Allow deeper hierarchies of RT cgroups
From: Peter Zijlstra @ 2026-05-07 15:05 UTC (permalink / raw)
To: Juri Lelli
Cc: Tejun Heo, Yuri Andriaccio, Ingo Molnar, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, linux-kernel, Luca Abeni, Yuri Andriaccio,
hannes, mkoutny, cgroups
In-Reply-To: <afypzfyH0M7Rcge2@jlelli-thinkpadt14gen4.remote.csb>
On Thu, May 07, 2026 at 05:03:41PM +0200, Juri Lelli wrote:
> On 07/05/26 12:53, Peter Zijlstra wrote:
> > On Tue, May 05, 2026 at 09:56:58AM -1000, Tejun Heo wrote:
>
> ...
>
> > > - However, the cpu controller is a threaded controller which means that it
> > > can have threaded sub-hierarchy where the no-internal-process rule doesn't
> > > apply. This was created explicitly for cpu controller. The proposed change
> > > blocks it effectively forcing cpu controller into regular domain
> > > controller behavior subject to no-internal-process rule. Note these are
> > > enforced at controller granularity and this means that users who use the
> > > threaded mode will be forced to pick between the two.
> >
> > Right... this then means we need two controls, one to do hierarchical
> > bandwidth distribution, and one to assign bandwidth to the internal
> > group -- which is then subject to its own bandwidth distribution
> > constraint.
> >
> > This might be a little confusing, but there is no way around that
> > AFAICT.
>
> Just to check if I'm following, you are thinking something like below?
>
> groupA/
> cpu.rt.max = "50 50 100" <- 0.5 from root
> cpu.rt.internal = "20 20 100" <- 0.2 from groupA for threads at this
> level
> + threadA <
> + threadB <
> +- group1/
> cpu.rt.max = "30 30 100" <- 0.3 from groupA
> + threadC
>
> And we still keep it flat, so 2 dl-entities (per CPU), one handles
> threads at groupA level and the other threads inside group1?
Exactly!
^ permalink raw reply
* Re: [RFC PATCH v5 20/29] sched/deadline: Allow deeper hierarchies of RT cgroups
From: Juri Lelli @ 2026-05-07 15:03 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tejun Heo, Yuri Andriaccio, Ingo Molnar, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, linux-kernel, Luca Abeni, Yuri Andriaccio,
hannes, mkoutny, cgroups
In-Reply-To: <20260507105331.GQ1026330@noisy.programming.kicks-ass.net>
On 07/05/26 12:53, Peter Zijlstra wrote:
> On Tue, May 05, 2026 at 09:56:58AM -1000, Tejun Heo wrote:
...
> > - However, the cpu controller is a threaded controller which means that it
> > can have threaded sub-hierarchy where the no-internal-process rule doesn't
> > apply. This was created explicitly for cpu controller. The proposed change
> > blocks it effectively forcing cpu controller into regular domain
> > controller behavior subject to no-internal-process rule. Note these are
> > enforced at controller granularity and this means that users who use the
> > threaded mode will be forced to pick between the two.
>
> Right... this then means we need two controls, one to do hierarchical
> bandwidth distribution, and one to assign bandwidth to the internal
> group -- which is then subject to its own bandwidth distribution
> constraint.
>
> This might be a little confusing, but there is no way around that
> AFAICT.
Just to check if I'm following, you are thinking something like below?
groupA/
cpu.rt.max = "50 50 100" <- 0.5 from root
cpu.rt.internal = "20 20 100" <- 0.2 from groupA for threads at this
level
+ threadA <
+ threadB <
+- group1/
cpu.rt.max = "30 30 100" <- 0.3 from groupA
+ threadC
And we still keep it flat, so 2 dl-entities (per CPU), one handles
threads at groupA level and the other threads inside group1?
Thanks,
Juri
^ permalink raw reply
* Re: [PATCH v2 1/2] cgroup/cpuset: reset DL migration state on can_attach() failure
From: Waiman Long @ 2026-05-07 14:31 UTC (permalink / raw)
To: Guopeng Zhang, Tejun Heo, Michal Koutný, Ingo Molnar,
Peter Zijlstra, Juri Lelli
Cc: Chen Ridong, Johannes Weiner, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Gabriele Monaco, Will Deacon, linux-kernel,
cgroups
In-Reply-To: <20260507103310.35849-2-zhangguopeng@kylinos.cn>
On 5/7/26 6:33 AM, Guopeng Zhang wrote:
> cpuset_can_attach() accumulates temporary SCHED_DEADLINE migration
> state in the destination cpuset while walking the taskset.
>
> If a later task_can_attach() or security_task_setscheduler() check
> fails, cgroup_migrate_execute() treats cpuset as the failing subsystem
> and does not call cpuset_cancel_attach() for it. The partially
> accumulated state is then left behind and can be consumed by a later
> attach, corrupting cpuset DL task accounting and pending DL bandwidth
> accounting.
>
> Reset the pending DL migration state before returning from those
> per-task failure paths.
>
> Fixes: 2ef269ef1ac0 ("cgroup/cpuset: Free DL BW in case can_attach() fails")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> kernel/cgroup/cpuset.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index e3a081a07c6d..ae41736399a1 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -3029,12 +3029,12 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> cgroup_taskset_for_each(task, css, tset) {
> ret = task_can_attach(task);
> if (ret)
> - goto out_unlock;
> + goto out_reset_dl_data;
>
> if (setsched_check) {
> ret = security_task_setscheduler(task);
> if (ret)
> - goto out_unlock;
> + goto out_reset_dl_data;
> }
>
> if (dl_task(task)) {
> @@ -3070,6 +3070,10 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
> * changes which zero cpus/mems_allowed.
> */
> cs->attach_in_progress++;
> + goto out_unlock;
> +
> +out_reset_dl_data:
> + reset_migrate_dl_data(cs);
> out_unlock:
> mutex_unlock(&cpuset_mutex);
> return ret;
I would prefer the likely success path be a straight line instead of
doing a goto. IOW, move out_reset_dl_data below return. Other than that,
this patch looks good to me.
Cheers,
Longman
^ permalink raw reply
* Re: [RFC PATCH v5 20/29] sched/deadline: Allow deeper hierarchies of RT cgroups
From: luca abeni @ 2026-05-07 14:30 UTC (permalink / raw)
To: Tejun Heo
Cc: Peter Zijlstra, Yuri Andriaccio, Ingo Molnar, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, linux-kernel, Yuri Andriaccio,
hannes, mkoutny, cgroups
In-Reply-To: <afpLir8tD0Ycb3D8@slm.duckdns.org>
Hi Tejun,
first of all, thanks for your comments! I think this is the kind of
dicussion that we need to have...
Right now we have something that works "well enough" for real-time, but
we want to make it useful in general, so that distributions will not
disable it by default.
I need to better study your suggestions (I do not know cgroup v2
much...), but I have some questions to better understand possible
solutions:
On Tue, 5 May 2026 09:56:58 -1000
Tejun Heo <tj@kernel.org> wrote:
[...]
> - cgroup2 enforces that internal cgroups w/ controllers enabled
> cannot have threads in them. No need to enforce that separately.
>
> - However, the cpu controller is a threaded controller which means
> that it can have threaded sub-hierarchy where the no-internal-process
> rule doesn't apply. This was created explicitly for cpu controller.
> The proposed change blocks it effectively forcing cpu controller into
> regular domain controller behavior subject to no-internal-process
> rule. Note these are enforced at controller granularity and this
> means that users who use the threaded mode will be forced to pick
> between the two.
Just to better understand: would it make sense to allow non-{FIFO,RT}
tasks to be in non-leaf cgroups (as allowed by the threaded CPU
controller), while enforcing that FIFO/RR tasks can only be in leaf
cgroups? Or would this be a hack that compromises the rt-CPU controller
usefulness?
> - This has the same problem with cgroup1's rt cgroup sched support
> where there is no way to have a permissive default configuration,
> which means that users who don't really care about distributing rt
> shares hierarchically would get blocked from running rt processes by
> default, which basically forces distros to disable rt cgroup sched
> support. This is not new but it'd be a shame to put in all the work
> and the end result is that most people don't even have access to the
> feature.
Yes, we have a bad default here.
Would a default like "allow running FIFO/RR tasks without runtime
enforcement" (this is what happens to FIFO/RR tasks running in the root
control group) be acceptable?
Thanks,
Luca
>
> Here's my suggestion if there is desire for this to become something
> most people have easy access to:
>
> - Don't make it impossible to use in conjunction with other resource
> control mechanisms especially not CPU controller itself. Don't force
> people to choose between threaded mode and rt control. Allow them to
> co-exist in a reasonable manner.
>
> - The same in the wider scope. Don't let it get in the way of people
> who don't care about it. Compromising on interface / failure mode is
> better than people not being able to use it in most cases.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH] cgroup/cpuset: move PF_EXITING check before __GFP_HARDWALL in cpuset_current_node_allowed()
From: Waiman Long @ 2026-05-07 13:53 UTC (permalink / raw)
To: Michal Koutný, Chen Wandun
Cc: chenridong, tj, hannes, cgroups, linux-kernel
In-Reply-To: <afx1u4kV-2kvgEEf@localhost.localdomain>
On 5/7/26 8:33 AM, Michal Koutný wrote:
> On Thu, May 07, 2026 at 06:54:34PM +0800, Chen Wandun <chenwandun1@gmail.com> wrote:
>> This makes it unreachable in the common case, so dying tasks can get
>> stuck in direct reclaim or even trigger OOM while trying to exit,
>> despite being allowed to allocate from any node.
> (OTOH, the caused OOM could select this task and bypass the hardwall. So
> this should only expedite but no unblock the exit path.)
>
>> Move the PF_EXITING check before __GFP_HARDWALL so that dying tasks
>> can allocate memory from any node to exit quickly, even when cpusets
>> are enabled.
> This makes sense to me on its own (given other hardwall exemptions,
> namely the commit c596d9f320aaf ("cpusets: allow TIF_MEMDIE threads to
> allocate anywhere")).
>
> Acked-by: Michal Koutný <mkoutny@suse.com>
This looks good to me too.
Acked-by: Waiman Long <longman@redhat.com>
>
>
> At first, I wondered whether this could happen on cpuset v2 -- it can --
> because only per-cpuset hardwalling is absent but the generic logic for
> GFP_USER allocations is still meant to be in place. Nevertheless, it
> occured to me we can spare callback_lock in this function (a separate
> chaneg for cpuset_current_node_allowed()):
>
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -4213,6 +4213,9 @@ bool cpuset_current_node_allowed(int node, gfp_t gfp_mask)
> if (current->flags & PF_EXITING) /* Let dying task have memory */
> return true;
>
> + if (is_in_v2_mode())
> + return true;
> +
> /* Not hardwall and node outside mems_allowed: scan up cpusets */
> spin_lock_irqsave(&callback_lock, flags);
Yes, it is a performance optimization that is worth to have as cgroup v2
doesn't have the concept of memory hardwall yet.
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH] cgroup/cpuset: move PF_EXITING check before __GFP_HARDWALL in cpuset_current_node_allowed()
From: Michal Koutný @ 2026-05-07 12:33 UTC (permalink / raw)
To: Chen Wandun; +Cc: longman, chenridong, tj, hannes, cgroups, linux-kernel
In-Reply-To: <20260507105434.3266234-1-chenwandun@lixiang.com>
[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]
On Thu, May 07, 2026 at 06:54:34PM +0800, Chen Wandun <chenwandun1@gmail.com> wrote:
> This makes it unreachable in the common case, so dying tasks can get
> stuck in direct reclaim or even trigger OOM while trying to exit,
> despite being allowed to allocate from any node.
(OTOH, the caused OOM could select this task and bypass the hardwall. So
this should only expedite but no unblock the exit path.)
> Move the PF_EXITING check before __GFP_HARDWALL so that dying tasks
> can allocate memory from any node to exit quickly, even when cpusets
> are enabled.
This makes sense to me on its own (given other hardwall exemptions,
namely the commit c596d9f320aaf ("cpusets: allow TIF_MEMDIE threads to
allocate anywhere")).
Acked-by: Michal Koutný <mkoutny@suse.com>
At first, I wondered whether this could happen on cpuset v2 -- it can --
because only per-cpuset hardwalling is absent but the generic logic for
GFP_USER allocations is still meant to be in place. Nevertheless, it
occured to me we can spare callback_lock in this function (a separate
chaneg for cpuset_current_node_allowed()):
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4213,6 +4213,9 @@ bool cpuset_current_node_allowed(int node, gfp_t gfp_mask)
if (current->flags & PF_EXITING) /* Let dying task have memory */
return true;
+ if (is_in_v2_mode())
+ return true;
+
/* Not hardwall and node outside mems_allowed: scan up cpusets */
spin_lock_irqsave(&callback_lock, flags);
Regards,
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ permalink raw reply
* [PATCH] cgroup/cpuset: move PF_EXITING check before __GFP_HARDWALL in cpuset_current_node_allowed()
From: Chen Wandun @ 2026-05-07 10:54 UTC (permalink / raw)
To: longman, chenridong, tj, hannes, mkoutny; +Cc: cgroups, linux-kernel
Since prepare_alloc_pages() unconditionally adds __GFP_HARDWALL for the
fast path when cpusets are enabled, the __GFP_HARDWALL check in
cpuset_current_node_allowed() causes the PF_EXITING escape path to be
skipped on the first allocation attempt. This makes it unreachable in
the common case, so dying tasks can get stuck in direct reclaim or even
trigger OOM while trying to exit, despite being allowed to allocate from
any node.
Move the PF_EXITING check before __GFP_HARDWALL so that dying tasks
can allocate memory from any node to exit quickly, even when cpusets
are enabled.
Also update the function comment to reflect the actual behavior of
prepare_alloc_pages() and the corrected check ordering.
Signed-off-by: Chen Wandun <chenwandun@lixiang.com>
---
kernel/cgroup/cpuset.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e3a081a07c6d..a48901a0416a 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -4176,11 +4176,11 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
* current's mems_allowed, yes. If it's not a __GFP_HARDWALL request and this
* node is set in the nearest hardwalled cpuset ancestor to current's cpuset,
* yes. If current has access to memory reserves as an oom victim, yes.
- * Otherwise, no.
+ * If the current task is PF_EXITING, yes. Otherwise, no.
*
* GFP_USER allocations are marked with the __GFP_HARDWALL bit,
* and do not allow allocations outside the current tasks cpuset
- * unless the task has been OOM killed.
+ * unless the task has been OOM killed or is exiting.
* GFP_KERNEL allocations are not so marked, so can escape to the
* nearest enclosing hardwalled ancestor cpuset.
*
@@ -4194,7 +4194,9 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
* The first call here from mm/page_alloc:get_page_from_freelist()
* has __GFP_HARDWALL set in gfp_mask, enforcing hardwall cpusets,
* so no allocation on a node outside the cpuset is allowed (unless
- * in interrupt, of course).
+ * in interrupt, of course). The PF_EXITING check must therefore
+ * come before the __GFP_HARDWALL check, otherwise a dying task
+ * would be blocked on the fast path.
*
* The second pass through get_page_from_freelist() doesn't even call
* here for GFP_ATOMIC calls. For those calls, the __alloc_pages()
@@ -4204,6 +4206,7 @@ static struct cpuset *nearest_hardwall_ancestor(struct cpuset *cs)
* in_interrupt - any node ok (current task context irrelevant)
* GFP_ATOMIC - any node ok
* tsk_is_oom_victim - any node ok
+ * PF_EXITING - any node ok (let dying task exit quickly)
* GFP_KERNEL - any node in enclosing hardwalled cpuset ok
* GFP_USER - only nodes in current tasks mems allowed ok.
*/
@@ -4223,11 +4226,10 @@ bool cpuset_current_node_allowed(int node, gfp_t gfp_mask)
*/
if (unlikely(tsk_is_oom_victim(current)))
return true;
- if (gfp_mask & __GFP_HARDWALL) /* If hardwall request, stop here */
- return false;
-
if (current->flags & PF_EXITING) /* Let dying task have memory */
return true;
+ if (gfp_mask & __GFP_HARDWALL) /* If hardwall request, stop here */
+ return false;
/* Not hardwall and node outside mems_allowed: scan up cpusets */
spin_lock_irqsave(&callback_lock, flags);
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH v5 20/29] sched/deadline: Allow deeper hierarchies of RT cgroups
From: Peter Zijlstra @ 2026-05-07 10:53 UTC (permalink / raw)
To: Tejun Heo
Cc: Yuri Andriaccio, Ingo Molnar, Juri Lelli, Vincent Guittot,
Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
Valentin Schneider, linux-kernel, Luca Abeni, Yuri Andriaccio,
hannes, mkoutny, cgroups
In-Reply-To: <afpLir8tD0Ycb3D8@slm.duckdns.org>
On Tue, May 05, 2026 at 09:56:58AM -1000, Tejun Heo wrote:
> Hello,
>
> Some high level comments:
>
> - Please align it with existing cgroup2 interface files. See cpu.max. This
> can be e.g. cpu.rt.max without about the same semantics.
>
> - cgroup2 enforces that internal cgroups w/ controllers enabled cannot have
> threads in them. No need to enforce that separately.
Looking at cpu_period_quota_parse() this thing takes two u64 values for:
{runtime, period} but allows runtime to be the string "max".
I think we'd want an optional extension to that and allow 3 values for:
{runtime, period, deadline}, where if the deadline is not given, it will
be the same as period.
In previous versions there was also an option to specify a cpumask,
getting rid of that is one of the reasons I suggested making this thing
a cgroup-v2 thing, then we can use the cpuset controller's effective
mask.
> - However, the cpu controller is a threaded controller which means that it
> can have threaded sub-hierarchy where the no-internal-process rule doesn't
> apply. This was created explicitly for cpu controller. The proposed change
> blocks it effectively forcing cpu controller into regular domain
> controller behavior subject to no-internal-process rule. Note these are
> enforced at controller granularity and this means that users who use the
> threaded mode will be forced to pick between the two.
Right... this then means we need two controls, one to do hierarchical
bandwidth distribution, and one to assign bandwidth to the internal
group -- which is then subject to its own bandwidth distribution
constraint.
This might be a little confusing, but there is no way around that
AFAICT.
> - This has the same problem with cgroup1's rt cgroup sched support where
> there is no way to have a permissive default configuration, which means
> that users who don't really care about distributing rt shares
> hierarchically would get blocked from running rt processes by default,
> which basically forces distros to disable rt cgroup sched support. This is
> not new but it'd be a shame to put in all the work and the end result is
> that most people don't even have access to the feature.
Right, but cgroup-v2 allows enabling/disabling specific controllers for
a (sub)-hierarchy, right? So if the controller is not enabled (by
default), it will fall back to putting the tasks in whatever parent does
have it on, and by default the root group would have and would accept
tasks.
Additionally, I think we want a flag to allow non-priv tasks to use RT
inside the controller -- after all, these tasks would be subject to
strict bandwidth controls and cannot burn the system like unbounded/root
FIFO tasks can.
Does that all sound workable?
^ permalink raw reply
* [PATCH v2 2/2] cgroup/cpuset: align DL bandwidth reservation with attach target mask
From: Guopeng Zhang @ 2026-05-07 10:33 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Michal Koutný, Ingo Molnar,
Peter Zijlstra, Juri Lelli
Cc: Chen Ridong, Johannes Weiner, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Gabriele Monaco, Will Deacon, linux-kernel,
cgroups, Guopeng Zhang
In-Reply-To: <20260507103310.35849-1-zhangguopeng@kylinos.cn>
cpuset_can_attach() preallocates destination SCHED_DEADLINE bandwidth
before the attach commit point, while set_cpus_allowed_dl() later
subtracts bandwidth from the source root domain when the task affinity is
actually updated.
Those two decisions must be made with the same CPU mask.
cpuset_can_attach() used the destination cpuset effective mask directly,
but cpuset_attach_task() first builds a per-task target mask which is
constrained by task_cpu_possible_mask() and, if needed, by walking up the
cpuset hierarchy. On asymmetric systems, the actual target mask can
therefore be a strict subset of cs->effective_cpus.
If the source root domain intersects cs->effective_cpus only on CPUs
outside the task's possible mask, can_attach() can skip the destination
reservation even though set_cpus_allowed_dl() later sees a real
root-domain move and subtracts from the source domain.
Extract the root-domain bandwidth-move test used by
set_cpus_allowed_dl() into dl_task_needs_bw_move(), and make
cpuset_can_attach() compute the same per-task target mask that
cpuset_attach_task() applies.
Keep nr_migrate_dl_tasks counting all migrating deadline tasks for
cpuset DL task accounting. Restrict sum_migrate_dl_bw to the subset of
tasks that need destination root-domain bandwidth reservation, because a
deadline task can move between cpusets without moving bandwidth between
root domains.
This keeps the existing per-attach aggregate reservation model; it only
changes the per-task mask used to decide which tasks contribute to that
aggregate. The broader can_attach()/attach() transaction window is left
unchanged.
Fixes: 431c69fac05b ("cpuset: Honour task_cpu_possible_mask() in guarantee_online_cpus()")
Fixes: 2ef269ef1ac0 ("cgroup/cpuset: Free DL BW in case can_attach() fails")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
include/linux/sched/deadline.h | 9 +++
kernel/cgroup/cpuset-internal.h | 1 +
kernel/cgroup/cpuset.c | 97 ++++++++++++++++++++++-----------
kernel/sched/deadline.c | 13 ++++-
4 files changed, 86 insertions(+), 34 deletions(-)
diff --git a/include/linux/sched/deadline.h b/include/linux/sched/deadline.h
index 1198138cb839..ddfd5216f3fc 100644
--- a/include/linux/sched/deadline.h
+++ b/include/linux/sched/deadline.h
@@ -33,6 +33,15 @@ struct root_domain;
extern void dl_add_task_root_domain(struct task_struct *p);
extern void dl_clear_root_domain(struct root_domain *rd);
extern void dl_clear_root_domain_cpu(int cpu);
+/*
+ * Return whether moving DL task @p to @new_mask requires moving DL
+ * bandwidth accounting between root domains. This helper is specific to
+ * DL bandwidth move accounting semantics and is shared by
+ * cpuset_can_attach() and set_cpus_allowed_dl() so both paths use the
+ * same source root-domain test.
+ */
+bool dl_task_needs_bw_move(struct task_struct *p,
+ const struct cpumask *new_mask);
extern u64 dl_cookie;
extern bool dl_bw_visited(int cpu, u64 cookie);
diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index bb4e692bea30..f7aaf01f7cd5 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -167,6 +167,7 @@ struct cpuset {
*/
int nr_deadline_tasks;
int nr_migrate_dl_tasks;
+ /* DL bandwidth that needs destination reservation for this attach. */
u64 sum_migrate_dl_bw;
/*
* CPU used for temporary DL bandwidth allocation during attach;
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index ae41736399a1..78c1a4071cc3 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -485,6 +485,30 @@ static void guarantee_active_cpus(struct task_struct *tsk,
rcu_read_unlock();
}
+/* Compute the effective CPU mask cpuset_attach_task() will apply to @tsk. */
+static void cpuset_attach_task_cpus(struct cpuset *cs, struct task_struct *tsk,
+ struct cpumask *pmask)
+{
+ const struct cpumask *possible_mask = task_cpu_possible_mask(tsk);
+
+ lockdep_assert_cpuset_lock_held();
+
+ if (cs == &top_cpuset) {
+ cpumask_andnot(pmask, possible_mask, subpartitions_cpus);
+ return;
+ }
+
+ if (WARN_ON(!cpumask_and(pmask, possible_mask, cpu_active_mask)))
+ cpumask_copy(pmask, cpu_active_mask);
+
+ rcu_read_lock();
+ while (!cpumask_intersects(cs->effective_cpus, pmask))
+ cs = parent_cs(cs);
+
+ cpumask_and(pmask, pmask, cs->effective_cpus);
+ rcu_read_unlock();
+}
+
/*
* Return in *pmask the portion of a cpusets's mems_allowed that
* are online, with memory. If none are online with memory, walk
@@ -2986,6 +3010,14 @@ static void reset_migrate_dl_data(struct cpuset *cs)
cs->dl_bw_cpu = -1;
}
+/*
+ * Protected by cpuset_mutex. cpus_attach is used by the can_attach/attach
+ * paths but we can't allocate it dynamically there. Define it global and
+ * allocate from cpuset_init().
+ */
+static cpumask_var_t cpus_attach;
+static nodemask_t cpuset_attach_nodemask_to;
+
/* Called by cgroups to determine if a cpuset is usable; cpuset_mutex held */
static int cpuset_can_attach(struct cgroup_taskset *tset)
{
@@ -2993,7 +3025,7 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
struct cpuset *cs, *oldcs;
struct task_struct *task;
bool setsched_check;
- int ret;
+ int cpu = nr_cpu_ids, ret;
/* used later by cpuset_attach() */
cpuset_attach_old_cs = task_cs(cgroup_taskset_first(tset, &css));
@@ -3038,32 +3070,47 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
}
if (dl_task(task)) {
+ /*
+ * Count all migrating DL tasks for cpuset task accounting.
+ * Only tasks that need a root-domain bandwidth move
+ * contribute to sum_migrate_dl_bw.
+ */
cs->nr_migrate_dl_tasks++;
- cs->sum_migrate_dl_bw += task->dl.dl_bw;
+ cpuset_attach_task_cpus(cs, task, cpus_attach);
+
+ if (dl_task_needs_bw_move(task, cpus_attach)) {
+ /*
+ * Keep the existing aggregate reservation model.
+ * Tasks in one attach enter the same destination
+ * cpuset, so the first CPU found for a task needing
+ * DL bandwidth reservation identifies the destination
+ * root domain.
+ */
+ if (cpu >= nr_cpu_ids)
+ cpu = cpumask_any_and(cpu_active_mask,
+ cpus_attach);
+ cs->sum_migrate_dl_bw += task->dl.dl_bw;
+ }
}
}
- if (!cs->nr_migrate_dl_tasks)
+ if (!cs->sum_migrate_dl_bw)
goto out_success;
- if (!cpumask_intersects(oldcs->effective_cpus, cs->effective_cpus)) {
- int cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
-
- if (unlikely(cpu >= nr_cpu_ids)) {
- reset_migrate_dl_data(cs);
- ret = -EINVAL;
- goto out_unlock;
- }
-
- ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
- if (ret) {
- reset_migrate_dl_data(cs);
- goto out_unlock;
- }
+ if (unlikely(cpu >= nr_cpu_ids)) {
+ reset_migrate_dl_data(cs);
+ ret = -EINVAL;
+ goto out_unlock;
+ }
- cs->dl_bw_cpu = cpu;
+ ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
+ if (ret) {
+ reset_migrate_dl_data(cs);
+ goto out_unlock;
}
+ cs->dl_bw_cpu = cpu;
+
out_success:
/*
* Mark attach is in progress. This makes validate_change() fail
@@ -3099,23 +3146,11 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
mutex_unlock(&cpuset_mutex);
}
-/*
- * Protected by cpuset_mutex. cpus_attach is used only by cpuset_attach_task()
- * but we can't allocate it dynamically there. Define it global and
- * allocate from cpuset_init().
- */
-static cpumask_var_t cpus_attach;
-static nodemask_t cpuset_attach_nodemask_to;
-
static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
{
lockdep_assert_cpuset_lock_held();
- if (cs != &top_cpuset)
- guarantee_active_cpus(task, cpus_attach);
- else
- cpumask_andnot(cpus_attach, task_cpu_possible_mask(task),
- subpartitions_cpus);
+ cpuset_attach_task_cpus(cs, task, cpus_attach);
/*
* can_attach beforehand should guarantee that this doesn't
* fail. TODO: have a better way to handle failure here
diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index edca7849b165..7db4c87df83b 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -3107,20 +3107,18 @@ static void task_woken_dl(struct rq *rq, struct task_struct *p)
static void set_cpus_allowed_dl(struct task_struct *p,
struct affinity_context *ctx)
{
- struct root_domain *src_rd;
struct rq *rq;
WARN_ON_ONCE(!dl_task(p));
rq = task_rq(p);
- src_rd = rq->rd;
/*
* Migrating a SCHED_DEADLINE task between exclusive
* cpusets (different root_domains) entails a bandwidth
* update. We already made space for us in the destination
* domain (see cpuset_can_attach()).
*/
- if (!cpumask_intersects(src_rd->span, ctx->new_mask)) {
+ if (dl_task_needs_bw_move(p, ctx->new_mask)) {
struct dl_bw *src_dl_b;
src_dl_b = dl_bw_of(cpu_of(rq));
@@ -3137,6 +3135,15 @@ static void set_cpus_allowed_dl(struct task_struct *p,
set_cpus_allowed_common(p, ctx);
}
+bool dl_task_needs_bw_move(struct task_struct *p,
+ const struct cpumask *new_mask)
+{
+ if (!dl_task(p))
+ return false;
+
+ return !cpumask_intersects(task_rq(p)->rd->span, new_mask);
+}
+
/* Assumes rq->lock is held */
static void rq_online_dl(struct rq *rq)
{
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] cgroup/cpuset: reset DL migration state on can_attach() failure
From: Guopeng Zhang @ 2026-05-07 10:33 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Michal Koutný, Ingo Molnar,
Peter Zijlstra, Juri Lelli
Cc: Chen Ridong, Johannes Weiner, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Gabriele Monaco, Will Deacon, linux-kernel,
cgroups, Guopeng Zhang
In-Reply-To: <20260507103310.35849-1-zhangguopeng@kylinos.cn>
cpuset_can_attach() accumulates temporary SCHED_DEADLINE migration
state in the destination cpuset while walking the taskset.
If a later task_can_attach() or security_task_setscheduler() check
fails, cgroup_migrate_execute() treats cpuset as the failing subsystem
and does not call cpuset_cancel_attach() for it. The partially
accumulated state is then left behind and can be consumed by a later
attach, corrupting cpuset DL task accounting and pending DL bandwidth
accounting.
Reset the pending DL migration state before returning from those
per-task failure paths.
Fixes: 2ef269ef1ac0 ("cgroup/cpuset: Free DL BW in case can_attach() fails")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
kernel/cgroup/cpuset.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e3a081a07c6d..ae41736399a1 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3029,12 +3029,12 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
cgroup_taskset_for_each(task, css, tset) {
ret = task_can_attach(task);
if (ret)
- goto out_unlock;
+ goto out_reset_dl_data;
if (setsched_check) {
ret = security_task_setscheduler(task);
if (ret)
- goto out_unlock;
+ goto out_reset_dl_data;
}
if (dl_task(task)) {
@@ -3070,6 +3070,10 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
* changes which zero cpus/mems_allowed.
*/
cs->attach_in_progress++;
+ goto out_unlock;
+
+out_reset_dl_data:
+ reset_migrate_dl_data(cs);
out_unlock:
mutex_unlock(&cpuset_mutex);
return ret;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] cgroup/cpuset: fix DL attach bandwidth accounting
From: Guopeng Zhang @ 2026-05-07 10:33 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Michal Koutný, Ingo Molnar,
Peter Zijlstra, Juri Lelli
Cc: Chen Ridong, Johannes Weiner, Vincent Guittot, Dietmar Eggemann,
Steven Rostedt, Ben Segall, Mel Gorman, Valentin Schneider,
K Prateek Nayak, Gabriele Monaco, Will Deacon, linux-kernel,
cgroups, Guopeng Zhang
Hi,
cpuset_can_attach() and set_cpus_allowed_dl() must make the same
decision about whether migrating a SCHED_DEADLINE task requires moving
bandwidth accounting between root domains.
The can_attach path used the destination cpuset effective CPU mask for
that decision. The attach path, however, applies a per-task target mask
which is constrained by task_cpu_possible_mask(), cpu_active_mask, and
the fallback walk up the cpuset hierarchy. On asymmetric CPU systems,
that per-task mask can be a strict subset of the destination cpuset
effective mask. This can make cpuset_can_attach() skip destination
bandwidth reservation while set_cpus_allowed_dl() later performs the
source-side bandwidth subtraction.
There is also an internal cpuset_can_attach() failure path where
temporary DL migration state can be left behind if a later per-task
check fails before cpuset marks attach_in_progress.
Patch 1 resets the temporary DL migration state on those internal
cpuset_can_attach() failure paths.
Patch 2 computes the same per-task target mask in cpuset_can_attach()
that cpuset_attach_task() later applies, and only includes DL tasks that
actually need a root-domain bandwidth move in the destination bandwidth
reservation.
The broader can_attach()/attach() transaction window is left unchanged.
This series does not attempt to rework sched_setattr() or source cpuset
resmask TOCTOU issues. It only aligns the reservation decision with the
attach-time bandwidth move decision and fixes the temporary state leak.
Guopeng Zhang (2):
cgroup/cpuset: reset DL migration state on can_attach() failure
cgroup/cpuset: align DL bandwidth reservation with attach target mask
include/linux/sched/deadline.h | 9 +++
kernel/cgroup/cpuset-internal.h | 1 +
kernel/cgroup/cpuset.c | 105 ++++++++++++++++++++++----------
kernel/sched/deadline.c | 13 +++-
4 files changed, 92 insertions(+), 36 deletions(-)
---
Changes since v1:
- Split the original patch into two patches.
- Reset temporary DL migration state on cpuset_can_attach() internal
failure paths.
- Computed the same per-task attach mask in cpuset_can_attach() as
cpuset_attach_task().
- Kept nr_migrate_dl_tasks counting all migrating DL tasks for cpuset
task accounting, while restricting sum_migrate_dl_bw to tasks that need
destination DL bandwidth reservation.
- Tightened Fixes tags.
- Documented the existing aggregate reservation invariant near the
dl_bw_cpu selection.
- Removed the unnecessary RCU guard from dl_task_needs_bw_move().
v1:
https://lore.kernel.org/all/20260421083449.95750-1-zhangguopeng@kylinos.cn
--
2.43.0
^ permalink raw reply
* [PATCH v3] blk-cgroup: fix leaks and online flag on radix_tree_insert failure
From: Tao Cui @ 2026-05-07 6:12 UTC (permalink / raw)
To: cuitao; +Cc: axboe, cgroups, josef, tj
In-Reply-To: <20260506131124.16755-1-cuitao@kylinos.cn>
When radix_tree_insert() fails in blkg_create(), the error path has two
issues:
1. blkg->online is set to true unconditionally, even when the blkg was
never fully inserted. Move the assignment inside the success block.
2. The error path calls blkg_put() without first calling
percpu_ref_kill(). Because the refcount is still in percpu mode,
percpu_ref_put() only does this_cpu_sub() without checking for zero,
so blkg_release() is never triggered. This permanently leaks the
blkg memory, its percpu iostat, policy data, the parent blkg
reference, and the cgroup css reference — the latter preventing the
cgroup from ever being destroyed.
Fix by replacing blkg_put() with percpu_ref_kill(), matching the pattern
used in blkg_destroy().
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
v3:
- Remove the redundant blkg_put() after percpu_ref_kill() to avoid a
double-put that causes the refcount to go negative and bypass
blkg_release(), as pointed out by the sashiko AI review.
v2:
- Also fix the percpu_ref leak on the radix_tree_insert() error path by
adding percpu_ref_kill() before blkg_put(), as pointed out by the
sashiko AI review.
---
block/blk-cgroup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 554c87bb4a86..9fe850deef3b 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -431,15 +431,15 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
blkg->pd[i]->online = true;
}
}
+ blkg->online = true;
}
- blkg->online = true;
spin_unlock(&blkcg->lock);
if (!ret)
return blkg;
/* @blkg failed fully initialized, use the usual release path */
- blkg_put(blkg);
+ percpu_ref_kill(&blkg->refcnt);
return ERR_PTR(ret);
err_put_css:
--
2.43.0
^ permalink raw reply related
* [PATCH v2] blk-cgroup: fix leaks and online flag on radix_tree_insert failure
From: Tao Cui @ 2026-05-07 4:42 UTC (permalink / raw)
To: cuitao; +Cc: axboe, cgroups, josef, tj
In-Reply-To: <20260506131124.16755-1-cuitao@kylinos.cn>
When radix_tree_insert() fails in blkg_create(), the error path has two
issues:
1. blkg->online is set to true unconditionally, even when the blkg was
never fully inserted. Move the assignment inside the success block.
2. The error path calls blkg_put() without first calling
percpu_ref_kill(). Because the refcount is still in percpu mode,
percpu_ref_put() only does this_cpu_sub() without checking for zero,
so blkg_release() is never triggered. This permanently leaks the
blkg memory, its percpu iostat, policy data, the parent blkg
reference, and the cgroup css reference — the latter preventing the
cgroup from ever being destroyed.
Fix by adding percpu_ref_kill() before blkg_put(), matching the pattern
used in blkg_destroy().
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
v2:
- Also fix the percpu_ref leak on the radix_tree_insert() error path by
adding percpu_ref_kill() before blkg_put(), as pointed out by the
Sashiko AI review.
---
block/blk-cgroup.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 554c87bb4a86..845211f318fc 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -431,14 +431,15 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
blkg->pd[i]->online = true;
}
}
+ blkg->online = true;
}
- blkg->online = true;
spin_unlock(&blkcg->lock);
if (!ret)
return blkg;
/* @blkg failed fully initialized, use the usual release path */
+ percpu_ref_kill(&blkg->refcnt);
blkg_put(blkg);
return ERR_PTR(ret);
--
2.43.0
^ permalink raw reply related
* [PATCH v3] selftests: cgroup: Add basic tests for rdma controller
From: Tao Cui @ 2026-05-07 1:43 UTC (permalink / raw)
To: cuitao; +Cc: cgroups, hannes, mkoutny, tj
In-Reply-To: <20260506124307.15729-1-cuitao@kylinos.cn>
This patch adds (and wires in) new test program for verifying rdma
controller behavior -- checking that rdma.current correctly tracks
resource allocation/deallocation and that rdma.max limits are enforced.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
Changes in v3:
- Add test_rdma to .gitignore to avoid untracked file after compilation.
Changes in v2:
- Replace rdma.max interface-only tests with tests that verify actual
rdma.current behavior and limit enforcement
- Use libibverbs (conditionally compiled) to allocate real IB resources
- Skip gracefully when no RDMA device is present
---
tools/testing/selftests/cgroup/.gitignore | 1 +
tools/testing/selftests/cgroup/Makefile | 9 +
tools/testing/selftests/cgroup/test_rdma.c | 327 +++++++++++++++++++++
3 files changed, 337 insertions(+)
create mode 100644 tools/testing/selftests/cgroup/test_rdma.c
diff --git a/tools/testing/selftests/cgroup/.gitignore b/tools/testing/selftests/cgroup/.gitignore
index 952e4448bf07..70e4ef310ebd 100644
--- a/tools/testing/selftests/cgroup/.gitignore
+++ b/tools/testing/selftests/cgroup/.gitignore
@@ -8,5 +8,6 @@ test_kill
test_kmem
test_memcontrol
test_pids
+test_rdma
test_zswap
wait_inotify
diff --git a/tools/testing/selftests/cgroup/Makefile b/tools/testing/selftests/cgroup/Makefile
index e01584c2189a..df6abf633bab 100644
--- a/tools/testing/selftests/cgroup/Makefile
+++ b/tools/testing/selftests/cgroup/Makefile
@@ -16,6 +16,7 @@ TEST_GEN_PROGS += test_kill
TEST_GEN_PROGS += test_kmem
TEST_GEN_PROGS += test_memcontrol
TEST_GEN_PROGS += test_pids
+TEST_GEN_PROGS += test_rdma
TEST_GEN_PROGS += test_zswap
LOCAL_HDRS += $(selfdir)/clone3/clone3_selftests.h $(selfdir)/pidfd/pidfd.h
@@ -32,4 +33,12 @@ $(OUTPUT)/test_kill: $(LIBCGROUP_O)
$(OUTPUT)/test_kmem: $(LIBCGROUP_O)
$(OUTPUT)/test_memcontrol: $(LIBCGROUP_O)
$(OUTPUT)/test_pids: $(LIBCGROUP_O)
+$(OUTPUT)/test_rdma: $(LIBCGROUP_O)
$(OUTPUT)/test_zswap: $(LIBCGROUP_O)
+
+# Conditionally enable rdma.current/limit tests when libibverbs is available
+IBVERBS_AVAILABLE := $(shell pkg-config --exists libibverbs 2>/dev/null && echo y)
+ifeq ($(IBVERBS_AVAILABLE),y)
+$(OUTPUT)/test_rdma: CFLAGS += -DHAVE_LIBIBVERBS
+$(OUTPUT)/test_rdma: LDLIBS += -libverbs
+endif
diff --git a/tools/testing/selftests/cgroup/test_rdma.c b/tools/testing/selftests/cgroup/test_rdma.c
new file mode 100644
index 000000000000..5c2719ed7e62
--- /dev/null
+++ b/tools/testing/selftests/cgroup/test_rdma.c
@@ -0,0 +1,327 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <linux/limits.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "kselftest.h"
+#include "cgroup_util.h"
+
+#ifdef HAVE_LIBIBVERBS
+#include <infiniband/verbs.h>
+
+static char *rdmacg_get_first_device(const char *cgroup)
+{
+ char buf[PAGE_SIZE];
+ char *space;
+
+ if (cg_read(cgroup, "rdma.max", buf, sizeof(buf)))
+ return NULL;
+
+ if (buf[0] == '\0')
+ return NULL;
+
+ space = strchr(buf, ' ');
+ if (!space)
+ return NULL;
+
+ return strndup(buf, space - buf);
+}
+
+static long rdmacg_get_current_value(const char *cgroup, const char *device,
+ const char *resource)
+{
+ char buf[PAGE_SIZE];
+ char pattern[256];
+ char *p;
+
+ if (cg_read(cgroup, "rdma.current", buf, sizeof(buf)))
+ return -1;
+
+ snprintf(pattern, sizeof(pattern), "%s ", device);
+ p = strstr(buf, pattern);
+ if (!p)
+ return -1;
+
+ snprintf(pattern, sizeof(pattern), "%s=", resource);
+ p = strstr(p, pattern);
+ if (!p)
+ return -1;
+ p += strlen(pattern);
+
+ return strtol(p, NULL, 10);
+}
+
+static struct ibv_context *rdmacg_open_device(const char *device_name)
+{
+ struct ibv_device **dev_list;
+ struct ibv_context *ctx = NULL;
+ int i;
+
+ dev_list = ibv_get_device_list(NULL);
+ if (!dev_list)
+ return NULL;
+
+ for (i = 0; dev_list[i]; i++) {
+ if (!strcmp(ibv_get_device_name(dev_list[i]), device_name)) {
+ ctx = ibv_open_device(dev_list[i]);
+ break;
+ }
+ }
+ ibv_free_device_list(dev_list);
+ return ctx;
+}
+
+static int rdmacg_current_fn(const char *cgroup, void *arg)
+{
+ const char *device_name = (const char *)arg;
+ struct ibv_context *ctx = NULL;
+ struct ibv_pd *pd = NULL;
+ long val;
+ int ret = EXIT_FAILURE;
+
+ ctx = rdmacg_open_device(device_name);
+ if (!ctx)
+ return EXIT_FAILURE;
+
+ val = rdmacg_get_current_value(cgroup, device_name, "hca_handle");
+ if (val != 1) {
+ ksft_print_msg("hca_handle should be 1 after open, got %ld\n", val);
+ goto cleanup;
+ }
+ val = rdmacg_get_current_value(cgroup, device_name, "hca_object");
+ if (val != 0) {
+ ksft_print_msg("hca_object should be 0 before alloc, got %ld\n", val);
+ goto cleanup;
+ }
+
+ pd = ibv_alloc_pd(ctx);
+ if (!pd) {
+ ksft_print_msg("ibv_alloc_pd failed: %s\n", strerror(errno));
+ goto cleanup;
+ }
+ val = rdmacg_get_current_value(cgroup, device_name, "hca_object");
+ if (val != 1) {
+ ksft_print_msg("hca_object should be 1 after alloc_pd, got %ld\n", val);
+ goto cleanup;
+ }
+
+ /* After ibv_dealloc_pd: hca_object should be 0 */
+ ibv_dealloc_pd(pd);
+ pd = NULL;
+ val = rdmacg_get_current_value(cgroup, device_name, "hca_object");
+ if (val != 0) {
+ ksft_print_msg("hca_object should be 0 after dealloc_pd, got %ld\n", val);
+ goto cleanup;
+ }
+
+ /* After ibv_close_device: hca_handle should be 0 */
+ ibv_close_device(ctx);
+ ctx = NULL;
+ val = rdmacg_get_current_value(cgroup, device_name, "hca_handle");
+ if (val != 0) {
+ ksft_print_msg("hca_handle should be 0 after close, got %ld\n", val);
+ goto cleanup;
+ }
+
+ ret = EXIT_SUCCESS;
+
+cleanup:
+ if (pd)
+ ibv_dealloc_pd(pd);
+ if (ctx)
+ ibv_close_device(ctx);
+ return ret;
+}
+
+/*
+ * Test: rdma.current responds to actual IB resource allocation and deallocation.
+ */
+static int test_rdmacg_current_response(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cg;
+ char *device = NULL;
+
+ cg = cg_name(root, "rdmacg_test_1");
+ if (!cg)
+ return KSFT_FAIL;
+
+ if (cg_create(cg))
+ goto cleanup;
+
+ device = rdmacg_get_first_device(cg);
+ if (!device) {
+ ret = KSFT_SKIP;
+ goto cleanup;
+ }
+
+ if (!cg_run(cg, rdmacg_current_fn, device))
+ ret = KSFT_PASS;
+
+cleanup:
+ free(device);
+ cg_destroy(cg);
+ free(cg);
+ return ret;
+}
+
+static int rdmacg_limit_fn(const char *cgroup, void *arg)
+{
+ const char *device_name = (const char *)arg;
+ struct ibv_context *ctx = NULL;
+ struct ibv_pd *pd1 = NULL, *pd2 = NULL;
+ int ret = EXIT_FAILURE;
+
+ ctx = rdmacg_open_device(device_name);
+ if (!ctx)
+ return EXIT_FAILURE;
+
+ /* First PD allocation should succeed (within hca_object=1 limit) */
+ pd1 = ibv_alloc_pd(ctx);
+ if (!pd1) {
+ ksft_print_msg("first ibv_alloc_pd failed: %s\n", strerror(errno));
+ goto cleanup;
+ }
+
+ /* Second PD allocation should fail (exceeds hca_object=1 limit) */
+ pd2 = ibv_alloc_pd(ctx);
+ if (pd2) {
+ ksft_print_msg("second ibv_alloc_pd should have failed\n");
+ goto cleanup;
+ }
+
+ /* Free first PD, then try again -- should succeed */
+ ibv_dealloc_pd(pd1);
+ pd1 = NULL;
+
+ pd1 = ibv_alloc_pd(ctx);
+ if (!pd1) {
+ ksft_print_msg("ibv_alloc_pd after free failed: %s\n", strerror(errno));
+ goto cleanup;
+ }
+
+ ret = EXIT_SUCCESS;
+
+cleanup:
+ if (pd1)
+ ibv_dealloc_pd(pd1);
+ if (pd2)
+ ibv_dealloc_pd(pd2);
+ if (ctx)
+ ibv_close_device(ctx);
+ return ret;
+}
+
+/*
+ * Test: rdma.max limits are enforced -- exceeding hca_object limit causes
+ * allocation failure.
+ */
+static int test_rdmacg_limit_enforcement(const char *root)
+{
+ int ret = KSFT_FAIL;
+ char *cg;
+ char *device = NULL;
+ char buf[256];
+
+ cg = cg_name(root, "rdmacg_test_2");
+ if (!cg)
+ return KSFT_FAIL;
+
+ if (cg_create(cg))
+ goto cleanup;
+
+ device = rdmacg_get_first_device(cg);
+ if (!device) {
+ ret = KSFT_SKIP;
+ goto cleanup;
+ }
+
+ snprintf(buf, sizeof(buf), "%s hca_handle=max hca_object=1", device);
+ if (cg_write(cg, "rdma.max", buf)) {
+ ksft_print_msg("failed to set hca_object=1 limit\n");
+ goto cleanup;
+ }
+
+ if (!cg_run(cg, rdmacg_limit_fn, device))
+ ret = KSFT_PASS;
+
+cleanup:
+ free(device);
+ cg_destroy(cg);
+ free(cg);
+ return ret;
+}
+
+#define T(x) { x, #x }
+struct rdmacg_test {
+ int (*fn)(const char *root);
+ const char *name;
+} tests[] = {
+ T(test_rdmacg_current_response),
+ T(test_rdmacg_limit_enforcement),
+};
+#undef T
+
+int main(int argc, char **argv)
+{
+ char root[PATH_MAX];
+ char orig_subtree[PAGE_SIZE] = {0};
+ bool rdma_was_enabled = false;
+
+ ksft_print_header();
+ ksft_set_plan(ARRAY_SIZE(tests));
+
+ if (cg_find_unified_root(root, sizeof(root), NULL))
+ ksft_exit_skip("cgroup v2 isn't mounted\n");
+
+ if (cg_read_strstr(root, "cgroup.controllers", "rdma"))
+ ksft_exit_skip("rdma controller isn't available\n");
+
+ /* Save original subtree_control so we can restore it later */
+ if (cg_read(root, "cgroup.subtree_control", orig_subtree,
+ sizeof(orig_subtree)))
+ orig_subtree[0] = '\0';
+
+ rdma_was_enabled = (strstr(orig_subtree, "rdma") != NULL);
+
+ /* Enable rdma controller if not already enabled */
+ if (!rdma_was_enabled) {
+ if (cg_write(root, "cgroup.subtree_control", "+rdma"))
+ ksft_exit_skip("Failed to enable rdma controller\n");
+ }
+
+ for (int i = 0; i < ARRAY_SIZE(tests); i++) {
+ switch (tests[i].fn(root)) {
+ case KSFT_PASS:
+ ksft_test_result_pass("%s\n", tests[i].name);
+ break;
+ case KSFT_SKIP:
+ ksft_test_result_skip("%s\n", tests[i].name);
+ break;
+ default:
+ ksft_test_result_fail("%s\n", tests[i].name);
+ break;
+ }
+ }
+
+ /* Restore original subtree_control state */
+ if (!rdma_was_enabled)
+ cg_write(root, "cgroup.subtree_control", "-rdma");
+
+ ksft_finished();
+}
+
+#else /* !HAVE_LIBIBVERBS */
+
+int main(int argc, char **argv)
+{
+ ksft_print_header();
+ ksft_exit_skip("test requires libibverbs\n");
+}
+
+#endif /* HAVE_LIBIBVERBS */
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 06/12] mm/memcg, swap: tidy up cgroup v1 memsw swap helpers
From: Chris Li @ 2026-05-06 20:57 UTC (permalink / raw)
To: kasong
Cc: linux-mm, Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Barry Song, Hugh Dickins, Kemeng Shi, Nhat Pham, Baoquan He,
Johannes Weiner, Youngjun Park, Chengming Zhou, Roman Gushchin,
Shakeel Butt, Muchun Song, Qi Zheng, linux-kernel, cgroups,
Yosry Ahmed, Lorenzo Stoakes, Dev Jain, Lance Yang, Michal Hocko,
Michal Hocko, Suren Baghdasaryan, Axel Rasmussen
In-Reply-To: <20260421-swap-table-p4-v3-6-2f23759a76bc@tencent.com>
On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@kernel.org> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> The cgroup v1 swap helpers always operate on swap cache folios whose
> swap entry is stable: the folio is locked and in the swap cache. There
> is no need to pass the swap entry or page count as separate parameters
> when they can be derived from the folio itself.
>
> Simplify the redundant parameters and add sanity checks to document
> the required preconditions.
>
> Also rename memcg1_swapout to __memcg1_swapout to indicate it requires
> special calling context: the folio must be isolated and dying, and the
> call must be made with interrupts disabled.
>
> No functional change.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
Acked-by: Chris Li <chrisl@kernel.org>
Chris
> ---
> include/linux/memcontrol.h | 8 ++++----
> include/linux/swap.h | 10 ++++------
> mm/huge_memory.c | 2 +-
> mm/memcontrol-v1.c | 33 ++++++++++++++++++++-------------
> mm/memcontrol.c | 9 ++++-----
> mm/swap_state.c | 4 ++--
> mm/swapfile.c | 2 +-
> mm/vmscan.c | 2 +-
> 8 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index dc3fa687759b..7d08128de1fd 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -1899,8 +1899,8 @@ static inline void mem_cgroup_exit_user_fault(void)
> current->in_user_fault = 0;
> }
>
> -void memcg1_swapout(struct folio *folio, swp_entry_t entry);
> -void memcg1_swapin(swp_entry_t entry, unsigned int nr_pages);
> +void __memcg1_swapout(struct folio *folio);
> +void memcg1_swapin(struct folio *folio);
>
> #else /* CONFIG_MEMCG_V1 */
> static inline
> @@ -1929,11 +1929,11 @@ static inline void mem_cgroup_exit_user_fault(void)
> {
> }
>
> -static inline void memcg1_swapout(struct folio *folio, swp_entry_t entry)
> +static inline void __memcg1_swapout(struct folio *folio)
> {
> }
>
> -static inline void memcg1_swapin(swp_entry_t entry, unsigned int nr_pages)
> +static inline void memcg1_swapin(struct folio *folio)
> {
> }
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 1930f81e6be4..f2949f5844a6 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -574,13 +574,12 @@ static inline void folio_throttle_swaprate(struct folio *folio, gfp_t gfp)
> #endif
>
> #if defined(CONFIG_MEMCG) && defined(CONFIG_SWAP)
> -int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry);
> -static inline int mem_cgroup_try_charge_swap(struct folio *folio,
> - swp_entry_t entry)
> +int __mem_cgroup_try_charge_swap(struct folio *folio);
> +static inline int mem_cgroup_try_charge_swap(struct folio *folio)
> {
> if (mem_cgroup_disabled())
> return 0;
> - return __mem_cgroup_try_charge_swap(folio, entry);
> + return __mem_cgroup_try_charge_swap(folio);
> }
>
> extern void __mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages);
> @@ -594,8 +593,7 @@ static inline void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_p
> extern long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg);
> extern bool mem_cgroup_swap_full(struct folio *folio);
> #else
> -static inline int mem_cgroup_try_charge_swap(struct folio *folio,
> - swp_entry_t entry)
> +static inline int mem_cgroup_try_charge_swap(struct folio *folio)
> {
> return 0;
> }
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 970e077019b7..9630e283cf25 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4431,7 +4431,7 @@ void deferred_split_folio(struct folio *folio, bool partially_mapped)
>
> /*
> * Exclude swapcache: originally to avoid a corrupt deferred split
> - * queue. Nowadays that is fully prevented by memcg1_swapout();
> + * queue. Nowadays that is fully prevented by __memcg1_swapout();
> * but if page reclaim is already handling the same folio, it is
> * unnecessary to handle it again in the shrinker, so excluding
> * swapcache here may still be a useful optimization.
> diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
> index 433bba9dfe71..36c507d81dc5 100644
> --- a/mm/memcontrol-v1.c
> +++ b/mm/memcontrol-v1.c
> @@ -604,18 +604,23 @@ void memcg1_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
> }
>
> /**
> - * memcg1_swapout - transfer a memsw charge to swap
> + * __memcg1_swapout - transfer a memsw charge to swap
> * @folio: folio whose memsw charge to transfer
> - * @entry: swap entry to move the charge to
> *
> - * Transfer the memsw charge of @folio to @entry.
> + * Transfer the memsw charge of @folio to the swap entry stored in
> + * folio->swap.
> + *
> + * Context: folio must be isolated, unmapped, locked and is just about
> + * to be freed, and caller must disable IRQs.
> */
> -void memcg1_swapout(struct folio *folio, swp_entry_t entry)
> +void __memcg1_swapout(struct folio *folio)
> {
> struct mem_cgroup *memcg, *swap_memcg;
> struct obj_cgroup *objcg;
> unsigned int nr_entries;
>
> + VM_WARN_ON_ONCE_FOLIO(!folio_test_swapcache(folio), folio);
> + VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
> VM_BUG_ON_FOLIO(folio_ref_count(folio), folio);
>
> @@ -641,7 +646,7 @@ void memcg1_swapout(struct folio *folio, swp_entry_t entry)
> swap_memcg = mem_cgroup_private_id_get_online(memcg, nr_entries);
> mod_memcg_state(swap_memcg, MEMCG_SWAP, nr_entries);
>
> - swap_cgroup_record(folio, mem_cgroup_private_id(swap_memcg), entry);
> + swap_cgroup_record(folio, mem_cgroup_private_id(swap_memcg), folio->swap);
>
> folio_unqueue_deferred_split(folio);
> folio->memcg_data = 0;
> @@ -671,18 +676,20 @@ void memcg1_swapout(struct folio *folio, swp_entry_t entry)
> obj_cgroup_put(objcg);
> }
>
> -/*
> +/**
> * memcg1_swapin - uncharge swap slot
> - * @entry: the first swap entry for which the pages are charged
> - * @nr_pages: number of pages which will be uncharged
> + * @folio: folio being swapped in
> *
> - * Call this function after successfully adding the charged page to swapcache.
> + * Call this function after successfully adding the charged
> + * folio to swapcache.
> *
> - * Note: This function assumes the page for which swap slot is being uncharged
> - * is order 0 page.
> + * Context: The folio has to be in swap cache and locked.
> */
> -void memcg1_swapin(swp_entry_t entry, unsigned int nr_pages)
> +void memcg1_swapin(struct folio *folio)
> {
> + VM_WARN_ON_ONCE_FOLIO(!folio_test_swapcache(folio), folio);
> + VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> +
> /*
> * Cgroup1's unified memory+swap counter has been charged with the
> * new swapcache page, finish the transfer by uncharging the swap
> @@ -701,7 +708,7 @@ void memcg1_swapin(swp_entry_t entry, unsigned int nr_pages)
> * let's not wait for it. The page already received a
> * memory+swap charge, drop the swap entry duplicate.
> */
> - mem_cgroup_uncharge_swap(entry, nr_pages);
> + mem_cgroup_uncharge_swap(folio->swap, folio_nr_pages(folio));
> }
> }
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c3d98ab41f1f..c7df30ca5aa7 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -5456,13 +5456,12 @@ int __init mem_cgroup_init(void)
> /**
> * __mem_cgroup_try_charge_swap - try charging swap space for a folio
> * @folio: folio being added to swap
> - * @entry: swap entry to charge
> *
> - * Try to charge @folio's memcg for the swap space at @entry.
> + * Try to charge @folio's memcg for the swap space at folio->swap.
> *
> * Returns 0 on success, -ENOMEM on failure.
> */
> -int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry)
> +int __mem_cgroup_try_charge_swap(struct folio *folio)
> {
> unsigned int nr_pages = folio_nr_pages(folio);
> struct page_counter *counter;
> @@ -5479,7 +5478,7 @@ int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry)
>
> rcu_read_lock();
> memcg = obj_cgroup_memcg(objcg);
> - if (!entry.val) {
> + if (!folio_test_swapcache(folio)) {
> memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
> rcu_read_unlock();
> return 0;
> @@ -5498,7 +5497,7 @@ int __mem_cgroup_try_charge_swap(struct folio *folio, swp_entry_t entry)
> }
> mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
>
> - swap_cgroup_record(folio, mem_cgroup_private_id(memcg), entry);
> + swap_cgroup_record(folio, mem_cgroup_private_id(memcg), folio->swap);
>
> return 0;
> }
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 6ebd062bcece..12b290d43e45 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -451,8 +451,8 @@ static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
> return ERR_PTR(-ENOMEM);
> }
>
> - /* For memsw accounting, swap is uncharged when folio is added to swap cache */
> - memcg1_swapin(entry, 1 << order);
> + /* memsw uncharges swap when folio is added to swap cache */
> + memcg1_swapin(folio);
> if (shadow)
> workingset_refault(folio, shadow);
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 2e384d1c78c3..e1ad77a69e54 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1730,7 +1730,7 @@ int folio_alloc_swap(struct folio *folio)
> }
>
> /* Need to call this even if allocation failed, for MEMCG_SWAP_FAIL. */
> - if (unlikely(mem_cgroup_try_charge_swap(folio, folio->swap)))
> + if (unlikely(mem_cgroup_try_charge_swap(folio)))
> swap_cache_del_folio(folio);
>
> if (unlikely(!folio_test_swapcache(folio)))
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bd1b1aa12581..63d06930d8e3 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -739,7 +739,7 @@ static int __remove_mapping(struct address_space *mapping, struct folio *folio,
>
> if (reclaimed && !mapping_exiting(mapping))
> shadow = workingset_eviction(folio, target_memcg);
> - memcg1_swapout(folio, swap);
> + __memcg1_swapout(folio);
> __swap_cache_del_folio(ci, folio, swap, shadow);
> swap_cluster_unlock_irq(ci);
> } else {
>
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH v3 05/12] mm, swap: unify large folio allocation
From: Chris Li @ 2026-05-06 20:48 UTC (permalink / raw)
To: kasong
Cc: linux-mm, Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Barry Song, Hugh Dickins, Kemeng Shi, Nhat Pham, Baoquan He,
Johannes Weiner, Youngjun Park, Chengming Zhou, Roman Gushchin,
Shakeel Butt, Muchun Song, Qi Zheng, linux-kernel, cgroups,
Yosry Ahmed, Lorenzo Stoakes, Dev Jain, Lance Yang, Michal Hocko,
Michal Hocko, Suren Baghdasaryan, Axel Rasmussen
In-Reply-To: <20260421-swap-table-p4-v3-5-2f23759a76bc@tencent.com>
On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@kernel.org> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> Now that direct large order allocation is supported in the swap cache,
> both anon and shmem can use it instead of implementing their own methods.
> This unifies the fallback and swap cache check, which also reduces the
> TOCTOU race window of swap cache state: previously, high order swapin
> required checking swap cache states first, then allocating and falling
> back separately. Now all these steps happen in the same compact loop.
>
> Order fallback and statistics are also unified, callers just need to
> check and pass the acceptable order bitmask.
>
> There is basically no behavior change. This only makes things more
> unified and prepares for later commits. Cgroup and zero map checks can
> also be moved into the compact loop, further reducing race windows and
> redundancy
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
> mm/memory.c | 77 ++++++------------------------
> mm/shmem.c | 94 +++++++++---------------------------
> mm/swap.h | 30 ++----------
> mm/swap_state.c | 145 ++++++++++----------------------------------------------
Thanks for unifying the different code paths. I really like those diff stats.
The execution flow for swap in is easier to read now. Good job.
Acked-by: Chris Li <chrisl@kernel.org>
Chris
> mm/swapfile.c | 3 +-
> 5 files changed, 67 insertions(+), 282 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index ea6568571131..404734a5bcff 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4593,26 +4593,6 @@ static vm_fault_t handle_pte_marker(struct vm_fault *vmf)
> return VM_FAULT_SIGBUS;
> }
>
> -static struct folio *__alloc_swap_folio(struct vm_fault *vmf)
> -{
> - struct vm_area_struct *vma = vmf->vma;
> - struct folio *folio;
> - softleaf_t entry;
> -
> - folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vmf->address);
> - if (!folio)
> - return NULL;
> -
> - entry = softleaf_from_pte(vmf->orig_pte);
> - if (mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
> - GFP_KERNEL, entry)) {
> - folio_put(folio);
> - return NULL;
> - }
> -
> - return folio;
> -}
> -
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> /*
> * Check if the PTEs within a range are contiguous swap entries
> @@ -4642,8 +4622,6 @@ static bool can_swapin_thp(struct vm_fault *vmf, pte_t *ptep, int nr_pages)
> */
> if (unlikely(swap_zeromap_batch(entry, nr_pages, NULL) != nr_pages))
> return false;
> - if (unlikely(non_swapcache_batch(entry, nr_pages) != nr_pages))
> - return false;
>
> return true;
> }
> @@ -4671,16 +4649,14 @@ static inline unsigned long thp_swap_suitable_orders(pgoff_t swp_offset,
> return orders;
> }
>
> -static struct folio *alloc_swap_folio(struct vm_fault *vmf)
> +static unsigned long thp_swapin_suitable_orders(struct vm_fault *vmf)
> {
> struct vm_area_struct *vma = vmf->vma;
> unsigned long orders;
> - struct folio *folio;
> unsigned long addr;
> softleaf_t entry;
> spinlock_t *ptl;
> pte_t *pte;
> - gfp_t gfp;
> int order;
>
> /*
> @@ -4688,7 +4664,7 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf)
> * maintain the uffd semantics.
> */
> if (unlikely(userfaultfd_armed(vma)))
> - goto fallback;
> + return 0;
>
> /*
> * A large swapped out folio could be partially or fully in zswap. We
> @@ -4696,7 +4672,7 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf)
> * folio.
> */
> if (!zswap_never_enabled())
> - goto fallback;
> + return 0;
>
> entry = softleaf_from_pte(vmf->orig_pte);
> /*
> @@ -4710,12 +4686,12 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf)
> vmf->address, orders);
>
> if (!orders)
> - goto fallback;
> + return 0;
>
> pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd,
> vmf->address & PMD_MASK, &ptl);
> if (unlikely(!pte))
> - goto fallback;
> + return 0;
>
> /*
> * For do_swap_page, find the highest order where the aligned range is
> @@ -4731,29 +4707,12 @@ static struct folio *alloc_swap_folio(struct vm_fault *vmf)
>
> pte_unmap_unlock(pte, ptl);
>
> - /* Try allocating the highest of the remaining orders. */
> - gfp = vma_thp_gfp_mask(vma);
> - while (orders) {
> - addr = ALIGN_DOWN(vmf->address, PAGE_SIZE << order);
> - folio = vma_alloc_folio(gfp, order, vma, addr);
> - if (folio) {
> - if (!mem_cgroup_swapin_charge_folio(folio, vma->vm_mm,
> - gfp, entry))
> - return folio;
> - count_mthp_stat(order, MTHP_STAT_SWPIN_FALLBACK_CHARGE);
> - folio_put(folio);
> - }
> - count_mthp_stat(order, MTHP_STAT_SWPIN_FALLBACK);
> - order = next_order(&orders, order);
> - }
> -
> -fallback:
> - return __alloc_swap_folio(vmf);
> + return orders;
> }
> #else /* !CONFIG_TRANSPARENT_HUGEPAGE */
> -static struct folio *alloc_swap_folio(struct vm_fault *vmf)
> +static unsigned long thp_swapin_suitable_orders(struct vm_fault *vmf)
> {
> - return __alloc_swap_folio(vmf);
> + return 0;
> }
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> @@ -4859,21 +4818,13 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> if (folio)
> swap_update_readahead(folio, vma, vmf->address);
> if (!folio) {
> - if (data_race(si->flags & SWP_SYNCHRONOUS_IO)) {
> - folio = alloc_swap_folio(vmf);
> - if (folio) {
> - /*
> - * folio is charged, so swapin can only fail due
> - * to raced swapin and return NULL.
> - */
> - swapcache = swapin_folio(entry, folio);
> - if (swapcache != folio)
> - folio_put(folio);
> - folio = swapcache;
> - }
> - } else {
> + /* Swapin bypasses readahead for SWP_SYNCHRONOUS_IO devices */
> + if (data_race(si->flags & SWP_SYNCHRONOUS_IO))
> + folio = swapin_entry(entry, GFP_HIGHUSER_MOVABLE,
> + thp_swapin_suitable_orders(vmf),
> + vmf, NULL, 0);
> + else
> folio = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE, vmf);
> - }
>
> if (!folio) {
> /*
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 5916acf594a8..17e3da11bb1d 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -159,7 +159,7 @@ static unsigned long shmem_default_max_inodes(void)
>
> static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> struct folio **foliop, enum sgp_type sgp, gfp_t gfp,
> - struct vm_area_struct *vma, vm_fault_t *fault_type);
> + struct vm_fault *vmf, vm_fault_t *fault_type);
>
> static inline struct shmem_sb_info *SHMEM_SB(struct super_block *sb)
> {
> @@ -2017,68 +2017,24 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
> }
>
> static struct folio *shmem_swap_alloc_folio(struct inode *inode,
> - struct vm_area_struct *vma, pgoff_t index,
> + struct vm_fault *vmf, pgoff_t index,
> swp_entry_t entry, int order, gfp_t gfp)
> {
> + pgoff_t ilx;
> + struct folio *folio;
> + struct mempolicy *mpol;
> + unsigned long orders = BIT(order);
> struct shmem_inode_info *info = SHMEM_I(inode);
> - struct folio *new, *swapcache;
> - int nr_pages = 1 << order;
> - gfp_t alloc_gfp = gfp;
> -
> - if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) {
> - if (WARN_ON_ONCE(order))
> - return ERR_PTR(-EINVAL);
> - } else if (order) {
> - /*
> - * If uffd is active for the vma, we need per-page fault
> - * fidelity to maintain the uffd semantics, then fallback
> - * to swapin order-0 folio, as well as for zswap case.
> - * Any existing sub folio in the swap cache also blocks
> - * mTHP swapin.
> - */
> - if ((vma && unlikely(userfaultfd_armed(vma))) ||
> - !zswap_never_enabled() ||
> - non_swapcache_batch(entry, nr_pages) != nr_pages)
> - goto fallback;
>
> - alloc_gfp = thp_limit_gfp_mask(vma_thp_gfp_mask(vma), gfp);
> - }
> -retry:
> - new = shmem_alloc_folio(alloc_gfp, order, info, index);
> - if (!new) {
> - new = ERR_PTR(-ENOMEM);
> - goto fallback;
> - }
> + if ((vmf && unlikely(userfaultfd_armed(vmf->vma))) ||
> + !zswap_never_enabled())
> + orders = 0;
>
> - if (mem_cgroup_swapin_charge_folio(new, vma ? vma->vm_mm : NULL,
> - alloc_gfp, entry)) {
> - folio_put(new);
> - new = ERR_PTR(-ENOMEM);
> - goto fallback;
> - }
> + mpol = shmem_get_pgoff_policy(info, index, order, &ilx);
> + folio = swapin_entry(entry, gfp, orders, vmf, mpol, ilx);
> + mpol_cond_put(mpol);
>
> - swapcache = swapin_folio(entry, new);
> - if (swapcache != new) {
> - folio_put(new);
> - if (!swapcache) {
> - /*
> - * The new folio is charged already, swapin can
> - * only fail due to another raced swapin.
> - */
> - new = ERR_PTR(-EEXIST);
> - goto fallback;
> - }
> - }
> - return swapcache;
> -fallback:
> - /* Order 0 swapin failed, nothing to fallback to, abort */
> - if (!order)
> - return new;
> - entry.val += index - round_down(index, nr_pages);
> - alloc_gfp = gfp;
> - nr_pages = 1;
> - order = 0;
> - goto retry;
> + return folio;
> }
>
> /*
> @@ -2265,11 +2221,12 @@ static int shmem_split_large_entry(struct inode *inode, pgoff_t index,
> */
> static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> struct folio **foliop, enum sgp_type sgp,
> - gfp_t gfp, struct vm_area_struct *vma,
> + gfp_t gfp, struct vm_fault *vmf,
> vm_fault_t *fault_type)
> {
> struct address_space *mapping = inode->i_mapping;
> - struct mm_struct *fault_mm = vma ? vma->vm_mm : NULL;
> + struct vm_area_struct *vma = vmf ? vmf->vma : NULL;
> + struct mm_struct *fault_mm = vmf ? vmf->vma->vm_mm : NULL;
> struct shmem_inode_info *info = SHMEM_I(inode);
> swp_entry_t swap;
> softleaf_t index_entry;
> @@ -2310,20 +2267,15 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> if (!folio) {
> if (data_race(si->flags & SWP_SYNCHRONOUS_IO)) {
> /* Direct swapin skipping swap cache & readahead */
> - folio = shmem_swap_alloc_folio(inode, vma, index,
> - index_entry, order, gfp);
> - if (IS_ERR(folio)) {
> - error = PTR_ERR(folio);
> - folio = NULL;
> - goto failed;
> - }
> + folio = shmem_swap_alloc_folio(inode, vmf, index,
> + swap, order, gfp);
> } else {
> /* Cached swapin only supports order 0 folio */
> folio = shmem_swapin_cluster(swap, gfp, info, index);
> - if (!folio) {
> - error = -ENOMEM;
> - goto failed;
> - }
> + }
> + if (!folio) {
> + error = -ENOMEM;
> + goto failed;
> }
> if (fault_type) {
> *fault_type |= VM_FAULT_MAJOR;
> @@ -2471,7 +2423,7 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
>
> if (xa_is_value(folio)) {
> error = shmem_swapin_folio(inode, index, &folio,
> - sgp, gfp, vma, fault_type);
> + sgp, gfp, vmf, fault_type);
> if (error == -EEXIST)
> goto repeat;
>
> diff --git a/mm/swap.h b/mm/swap.h
> index 6774af10a943..80c2f1bf7a57 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -300,7 +300,8 @@ struct folio *swap_cluster_readahead(swp_entry_t entry, gfp_t flag,
> struct mempolicy *mpol, pgoff_t ilx);
> struct folio *swapin_readahead(swp_entry_t entry, gfp_t flag,
> struct vm_fault *vmf);
> -struct folio *swapin_folio(swp_entry_t entry, struct folio *folio);
> +struct folio *swapin_entry(swp_entry_t entry, gfp_t flag, unsigned long orders,
> + struct vm_fault *vmf, struct mempolicy *mpol, pgoff_t ilx);
> void swap_update_readahead(struct folio *folio, struct vm_area_struct *vma,
> unsigned long addr);
>
> @@ -334,24 +335,6 @@ static inline int swap_zeromap_batch(swp_entry_t entry, int max_nr,
> return find_next_bit(sis->zeromap, end, start) - start;
> }
>
> -static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
> -{
> - int i;
> -
> - /*
> - * While allocating a large folio and doing mTHP swapin, we need to
> - * ensure all entries are not cached, otherwise, the mTHP folio will
> - * be in conflict with the folio in swap cache.
> - */
> - for (i = 0; i < max_nr; i++) {
> - if (swap_cache_has_folio(entry))
> - return i;
> - entry.val++;
> - }
> -
> - return i;
> -}
> -
> #else /* CONFIG_SWAP */
> struct swap_iocb;
> static inline struct swap_cluster_info *swap_cluster_lock(
> @@ -433,7 +416,9 @@ static inline struct folio *swapin_readahead(swp_entry_t swp, gfp_t gfp_mask,
> return NULL;
> }
>
> -static inline struct folio *swapin_folio(swp_entry_t entry, struct folio *folio)
> +static inline struct folio *swapin_entry(
> + swp_entry_t entry, gfp_t flag, unsigned long orders,
> + struct vm_fault *vmf, struct mempolicy *mpol, pgoff_t ilx)
> {
> return NULL;
> }
> @@ -493,10 +478,5 @@ static inline int swap_zeromap_batch(swp_entry_t entry, int max_nr,
> {
> return 0;
> }
> -
> -static inline int non_swapcache_batch(swp_entry_t entry, int max_nr)
> -{
> - return 0;
> -}
> #endif /* CONFIG_SWAP */
> #endif /* _MM_SWAP_H */
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index f5c77f348bbd..6ebd062bcece 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -235,45 +235,6 @@ void __swap_cache_add_folio(struct swap_cluster_info *ci,
> lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
> }
>
> -/**
> - * swap_cache_add_folio - Add a folio into the swap cache.
> - * @folio: The folio to be added.
> - * @entry: The swap entry corresponding to the folio.
> - * @shadowp: If a shadow is found, return the shadow.
> - *
> - * Add a folio into the swap cache. Will return error if any slot is no
> - * longer a valid swapped out slot or already occupied by another folio.
> - *
> - * Context: Caller must ensure @entry is valid and protect the swap device
> - * with reference count or locks.
> - */
> -static int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> - void **shadowp)
> -{
> - int err;
> - void *shadow = NULL;
> - unsigned int ci_off;
> - struct swap_info_struct *si;
> - struct swap_cluster_info *ci;
> - unsigned long nr_pages = folio_nr_pages(folio);
> -
> - si = __swap_entry_to_info(entry);
> - ci = swap_cluster_lock(si, swp_offset(entry));
> - ci_off = swp_cluster_offset(entry);
> - err = __swap_cache_add_check(ci, entry, nr_pages, &shadow);
> - if (err) {
> - swap_cluster_unlock(ci);
> - return err;
> - }
> -
> - __swap_cache_add_folio(ci, folio, entry);
> - swap_cluster_unlock(ci);
> - if (shadowp)
> - *shadowp = shadow;
> -
> - return 0;
> -}
> -
> static void __swap_cache_do_del_folio(struct swap_cluster_info *ci,
> struct folio *folio,
> swp_entry_t entry, void *shadow)
> @@ -644,51 +605,6 @@ void swap_update_readahead(struct folio *folio, struct vm_area_struct *vma,
> }
> }
>
> -/**
> - * __swap_cache_prepare_and_add - Prepare the folio and add it to swap cache.
> - * @entry: swap entry to be bound to the folio.
> - * @folio: folio to be added.
> - * @gfp: memory allocation flags for charge, can be 0 if @charged if true.
> - * @charged: if the folio is already charged.
> - *
> - * Update the swap_map and add folio as swap cache, typically before swapin.
> - * All swap slots covered by the folio must have a non-zero swap count.
> - *
> - * Context: Caller must protect the swap device with reference count or locks.
> - * Return: 0 if success, error code if failed.
> - */
> -static int __swap_cache_prepare_and_add(swp_entry_t entry,
> - struct folio *folio,
> - gfp_t gfp, bool charged)
> -{
> - void *shadow;
> - int ret;
> -
> - __folio_set_locked(folio);
> - __folio_set_swapbacked(folio);
> -
> - if (!charged && mem_cgroup_swapin_charge_folio(folio, NULL, gfp, entry)) {
> - ret = -ENOMEM;
> - goto failed;
> - }
> -
> - ret = swap_cache_add_folio(folio, entry, &shadow);
> - if (ret)
> - goto failed;
> -
> - memcg1_swapin(entry, folio_nr_pages(folio));
> - if (shadow)
> - workingset_refault(folio, shadow);
> -
> - /* Caller will initiate read into locked folio */
> - folio_add_lru(folio);
> - return 0;
> -
> -failed:
> - folio_unlock(folio);
> - return ret;
> -}
> -
> static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp,
> struct mempolicy *mpol, pgoff_t ilx,
> struct swap_iocb **plug, bool readahead)
> @@ -704,7 +620,6 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp,
> folio = swap_cache_get_folio(entry);
> if (folio)
> return folio;
> -
> folio = swap_cache_alloc_folio(entry, gfp, 0, NULL, mpol, ilx);
> } while (IS_ERR(folio) && PTR_ERR(folio) == -EEXIST);
>
> @@ -721,49 +636,37 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp,
> }
>
> /**
> - * swapin_folio - swap-in one or multiple entries skipping readahead.
> - * @entry: starting swap entry to swap in
> - * @folio: a new allocated and charged folio
> + * swapin_entry - swap-in one or multiple entries skipping readahead.
> + * @entry: swap entry indicating the target slot
> + * @gfp: memory allocation flags
> + * @orders: allocation orders
> + * @vmf: fault information
> + * @mpol: NUMA memory allocation policy to be applied
> + * @ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
> *
> - * Reads @entry into @folio, @folio will be added to the swap cache.
> - * If @folio is a large folio, the @entry will be rounded down to align
> - * with the folio size.
> + * This allocates a folio suitable for given @orders, or returns the
> + * existing folio in the swap cache for @entry. This initiates the IO, too,
> + * if needed. @entry is rounded down if @orders allow large allocation.
> *
> - * Return: returns pointer to @folio on success. If folio is a large folio
> - * and this raced with another swapin, NULL will be returned to allow fallback
> - * to order 0. Else, if another folio was already added to the swap cache,
> - * return that swap cache folio instead.
> + * Context: Caller must ensure @entry is valid and pin the swap device with refcount.
> + * Return: Returns the folio on success, NULL if failed.
> */
> -struct folio *swapin_folio(swp_entry_t entry, struct folio *folio)
> +struct folio *swapin_entry(swp_entry_t entry, gfp_t gfp, unsigned long orders,
> + struct vm_fault *vmf, struct mempolicy *mpol, pgoff_t ilx)
> {
> - int ret;
> - struct folio *swapcache;
> - pgoff_t offset = swp_offset(entry);
> - unsigned long nr_pages = folio_nr_pages(folio);
> -
> - entry = swp_entry(swp_type(entry), round_down(offset, nr_pages));
> - for (;;) {
> - ret = __swap_cache_prepare_and_add(entry, folio, 0, true);
> - if (!ret) {
> - swap_read_folio(folio, NULL);
> - break;
> - }
> + struct folio *folio;
>
> - /*
> - * Large order allocation needs special handling on
> - * race: if a smaller folio exists in cache, swapin needs
> - * to fall back to order 0, and doing a swap cache lookup
> - * might return a folio that is irrelevant to the faulting
> - * entry because @entry is aligned down. Just return NULL.
> - */
> - if (ret != -EEXIST || nr_pages > 1)
> - return NULL;
> + do {
> + folio = swap_cache_get_folio(entry);
> + if (folio)
> + return folio;
> + folio = swap_cache_alloc_folio(entry, gfp, orders, vmf, mpol, ilx);
> + } while (IS_ERR(folio) && PTR_ERR(folio) == -EEXIST);
>
> - swapcache = swap_cache_get_folio(entry);
> - if (swapcache)
> - return swapcache;
> - }
> + if (IS_ERR(folio))
> + return NULL;
>
> + swap_read_folio(folio, NULL);
> return folio;
> }
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index c7e173b93e11..2e384d1c78c3 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1826,8 +1826,7 @@ void folio_put_swap(struct folio *folio, struct page *subpage)
> * do_swap_page()
> * ... swapoff+swapon
> * swap_cache_alloc_folio()
> - * swap_cache_add_folio()
> - * // check swap_map
> + * // check swap_map
> * // verify PTE not changed
> *
> * In __swap_duplicate(), the swap_map need to be checked before
>
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH v3 04/12] mm, swap: add support for stable large allocation in swap cache directly
From: Chris Li @ 2026-05-06 20:27 UTC (permalink / raw)
To: kasong
Cc: linux-mm, Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Barry Song, Hugh Dickins, Kemeng Shi, Nhat Pham, Baoquan He,
Johannes Weiner, Youngjun Park, Chengming Zhou, Roman Gushchin,
Shakeel Butt, Muchun Song, Qi Zheng, linux-kernel, cgroups,
Yosry Ahmed, Lorenzo Stoakes, Dev Jain, Lance Yang, Michal Hocko,
Michal Hocko, Suren Baghdasaryan, Axel Rasmussen
In-Reply-To: <20260421-swap-table-p4-v3-4-2f23759a76bc@tencent.com>
On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@kernel.org> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> To make it possible to allocate large folios directly in swap cache,
> provide a new infrastructure helper to handle the swap cache status
> check, allocation, and order fallback in the swap cache layer
>
> The new helper replaces the existing swap_cache_alloc_folio. Based on
> this, all the separate swap folio allocation that is being done by anon
> / shmem before is converted to use this helper directly, unifying folio
> allocation for anon, shmem, and readahead.
>
> This slightly consolidates how allocation is synchronized, making it
> more stable and less prone to errors. The slot-count and cache-conflict
> check is now always performed with the cluster lock held before
> allocation, and repeated under the same lock right before cache
> insertion. This double check produces a stable result compared to the
> previous anon and shmem mTHP allocation implementation, avoids the
> false-negative conflict checks that the lockless path can return — large
> allocations no longer have to be unwound because the range turned out to
> be occupied — and aborts early for already-freed slots, which helps
> ordinary swapin and especially readahead, with only a marginal increase
> in cluster-lock contention (the lock is very lightly contended and stays
> local in the first place). Hence, callers of swap_cache_alloc_folio() no
> longer need to check the swap slot count or swap cache status
> themselves.
>
> And now whoever first successfully allocates a folio in the swap cache
> will be the one who charges it and performs the swap-in. The race window
> of swapping is also reduced since the loop is much more compact.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
Overall looks good. There seems to be some typo on the expression of
orders below.
> ---
> mm/swap.h | 3 +-
> mm/swap_state.c | 222 +++++++++++++++++++++++++++++++++++++++++---------------
> mm/zswap.c | 2 +-
> 3 files changed, 165 insertions(+), 62 deletions(-)
>
> diff --git a/mm/swap.h b/mm/swap.h
> index ad8b17a93758..6774af10a943 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -280,7 +280,8 @@ bool swap_cache_has_folio(swp_entry_t entry);
> struct folio *swap_cache_get_folio(swp_entry_t entry);
> void *swap_cache_get_shadow(swp_entry_t entry);
> void swap_cache_del_folio(struct folio *folio);
> -struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_flags,
> +struct folio *swap_cache_alloc_folio(swp_entry_t target_entry, gfp_t gfp_mask,
> + unsigned long orders, struct vm_fault *vmf,
> struct mempolicy *mpol, pgoff_t ilx);
> /* Below helpers require the caller to lock and pass in the swap cluster. */
> void __swap_cache_add_folio(struct swap_cluster_info *ci,
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 3da285a891b2..f5c77f348bbd 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -139,10 +139,10 @@ void *swap_cache_get_shadow(swp_entry_t entry)
>
> /**
> * __swap_cache_add_check - Check if a range is suitable for adding a folio.
> - * @ci: The locked swap cluster.
> - * @ci_off: Range start offset.
> - * @nr: Number of slots to check.
> - * @shadow: Returns the shadow value if one exists in the range.
> + * @ci: The locked swap cluster
> + * @targ_entry: The target swap entry to check, will be rounded down by @nr
> + * @nr: Number of slots to check, must be a power of 2
> + * @shadowp: Returns the shadow value if one exists in the range.
> *
> * Check if all slots covered by given range have a swap count >= 1.
> * Retrieves the shadow if there is one.
> @@ -150,22 +150,38 @@ void *swap_cache_get_shadow(swp_entry_t entry)
> * Context: Caller must lock the cluster.
> */
> static int __swap_cache_add_check(struct swap_cluster_info *ci,
> - unsigned int ci_off, unsigned int nr,
> - void **shadow)
> + swp_entry_t targ_entry,
> + unsigned long nr, void **shadowp)
> {
> - unsigned int ci_end = ci_off + nr;
> + unsigned int ci_off, ci_end;
> unsigned long old_tb;
>
> + /*
> + * If the target slot is not swapped out, return
> + * -EEXIST or -ENOENT. If the batch is not suitable, could be a
> + * race with concurrent free or cache add, return -EBUSY.
> + */
> if (unlikely(!ci->table))
> return -ENOENT;
> + ci_off = swp_cluster_offset(targ_entry);
> + old_tb = __swap_table_get(ci, ci_off);
> + if (swp_tb_is_folio(old_tb))
> + return -EEXIST;
> + if (!__swp_tb_get_count(old_tb))
> + return -ENOENT;
> + if (swp_tb_is_shadow(old_tb) && shadowp)
> + *shadowp = swp_tb_to_shadow(old_tb);
> +
> + if (nr == 1)
> + return 0;
> +
> + ci_off = round_down(ci_off, nr);
> + ci_end = ci_off + nr;
> do {
> old_tb = __swap_table_get(ci, ci_off);
> - if (unlikely(swp_tb_is_folio(old_tb)))
> - return -EEXIST;
> - if (unlikely(!__swp_tb_get_count(old_tb)))
> - return -ENOENT;
> - if (swp_tb_is_shadow(old_tb))
> - *shadow = swp_tb_to_shadow(old_tb);
> + if (unlikely(swp_tb_is_folio(old_tb) ||
> + !__swp_tb_get_count(old_tb)))
> + return -EBUSY;
> } while (++ci_off < ci_end);
>
> return 0;
> @@ -244,7 +260,7 @@ static int swap_cache_add_folio(struct folio *folio, swp_entry_t entry,
> si = __swap_entry_to_info(entry);
> ci = swap_cluster_lock(si, swp_offset(entry));
> ci_off = swp_cluster_offset(entry);
> - err = __swap_cache_add_check(ci, ci_off, nr_pages, &shadow);
> + err = __swap_cache_add_check(ci, entry, nr_pages, &shadow);
> if (err) {
> swap_cluster_unlock(ci);
> return err;
> @@ -399,6 +415,137 @@ void __swap_cache_replace_folio(struct swap_cluster_info *ci,
> }
> }
>
> +/*
> + * Try to allocate a folio of given order in the swap cache.
> + *
> + * This helper resolves the potential races of swap allocation
> + * and prepares a folio to be used for swap IO. May return following
> + * value:
> + *
> + * -ENOMEM / -EBUSY: Order is too large or in conflict with sub slot,
> + * caller should shrink the order and retry
> + * -ENOENT / -EEXIST: Target swap entry is unavailable or cached, the caller
> + * should abort or try to use the cached folio instead
> + */
> +static struct folio *__swap_cache_alloc(struct swap_cluster_info *ci,
> + swp_entry_t targ_entry, gfp_t gfp,
> + unsigned int order, struct vm_fault *vmf,
> + struct mempolicy *mpol, pgoff_t ilx)
> +{
> + int err;
> + swp_entry_t entry;
> + struct folio *folio;
> + void *shadow = NULL;
> + unsigned long address, nr_pages = 1 << order;
> + struct vm_area_struct *vma = vmf ? vmf->vma : NULL;
> +
> + entry.val = round_down(targ_entry.val, nr_pages);
> +
> + /* Check if the slot and range are available, skip allocation if not */
> + spin_lock(&ci->lock);
> + err = __swap_cache_add_check(ci, targ_entry, nr_pages, NULL);
> + spin_unlock(&ci->lock);
> + if (unlikely(err))
> + return ERR_PTR(err);
> +
> + /*
> + * Limit THP gfp. The limitation is a no-op for typical
> + * GFP_HIGHUSER_MOVABLE but matters for shmem.
> + */
> + if (order)
> + gfp = thp_limit_gfp_mask(vma_thp_gfp_mask(vma), gfp);
> +
> + if (mpol || !vmf) {
> + folio = folio_alloc_mpol(gfp, order, mpol, ilx, numa_node_id());
> + } else {
> + address = round_down(vmf->address, PAGE_SIZE << order);
> + folio = vma_alloc_folio(gfp, order, vmf->vma, address);
> + }
> + if (unlikely(!folio))
> + return ERR_PTR(-ENOMEM);
> +
> + /* Double check the range is still not in conflict */
> + spin_lock(&ci->lock);
> + err = __swap_cache_add_check(ci, targ_entry, nr_pages, &shadow);
> + if (unlikely(err)) {
> + spin_unlock(&ci->lock);
> + folio_put(folio);
> + return ERR_PTR(err);
> + }
> +
> + __folio_set_locked(folio);
> + __folio_set_swapbacked(folio);
> + __swap_cache_do_add_folio(ci, folio, entry);
> + spin_unlock(&ci->lock);
> +
> + if (mem_cgroup_swapin_charge_folio(folio, vmf ? vmf->vma->vm_mm : NULL,
> + gfp, entry)) {
> + spin_lock(&ci->lock);
> + __swap_cache_do_del_folio(ci, folio, entry, shadow);
> + spin_unlock(&ci->lock);
> + folio_unlock(folio);
> + /* nr_pages refs from swap cache, 1 from allocation */
> + folio_put_refs(folio, nr_pages + 1);
> + count_mthp_stat(order, MTHP_STAT_SWPIN_FALLBACK_CHARGE);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + /* For memsw accounting, swap is uncharged when folio is added to swap cache */
> + memcg1_swapin(entry, 1 << order);
> + if (shadow)
> + workingset_refault(folio, shadow);
> +
> + node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
> + lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
> +
> + /* Caller will initiate read into locked new_folio */
> + folio_add_lru(folio);
> + return folio;
> +}
> +
> +/**
> + * swap_cache_alloc_folio - Allocate folio for swapped out slot in swap cache.
> + * @targ_entry: swap entry indicating the target slot
> + * @gfp: memory allocation flags
> + * @orders: allocation orders
> + * @vmf: fault information
> + * @mpol: NUMA memory allocation policy to be applied
> + * @ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
> + *
> + * Allocate a folio in the swap cache for one swap slot, typically before
> + * doing IO (e.g. swap in or zswap writeback). The swap slot indicated by
> + * @targ_entry must have a non-zero swap count (swapped out).
> + *
> + * Context: Caller must protect the swap device with reference count or locks.
> + * Return: Returns the folio if allocation succeeded and folio is added to
> + * swap cache. Returns error code if allocation failed due to race.
> + */
> +struct folio *swap_cache_alloc_folio(swp_entry_t targ_entry, gfp_t gfp,
> + unsigned long orders, struct vm_fault *vmf,
> + struct mempolicy *mpol, pgoff_t ilx)
> +{
> + int order, err;
> + struct folio *ret;
> + struct swap_cluster_info *ci;
> +
> + /* Always allow order 0 so swap won't fail under pressure. */
> + order = orders ? highest_order(orders |= BIT(0)) : 0;
I can't understand this line. You seem to have put an order variable
assignment in an expression which feels odd to me. I assume you mean
"orders | BIT(0)".
BTW, can you write this as:
order = highest_order(orders | BIT(0));
Because when orders is zero, highest_order(BIT(0)) should be 0 as well.
Chris
> + ci = __swap_entry_to_cluster(targ_entry);
> + for (;;) {
> + ret = __swap_cache_alloc(ci, targ_entry, gfp, order,
> + vmf, mpol, ilx);
> + if (!IS_ERR(ret))
> + break;
> + err = PTR_ERR(ret);
> + if (!order || (err && err != -EBUSY && err != -ENOMEM))
> + break;
> + count_mthp_stat(order, MTHP_STAT_SWPIN_FALLBACK);
> + order = next_order(&orders, order);
> + }
> +
> + return ret;
> +}
> +
> /*
> * If we are the only user, then try to free up the swap cache.
> *
> @@ -542,51 +689,10 @@ static int __swap_cache_prepare_and_add(swp_entry_t entry,
> return ret;
> }
>
> -/**
> - * swap_cache_alloc_folio - Allocate folio for swapped out slot in swap cache.
> - * @entry: the swapped out swap entry to be binded to the folio.
> - * @gfp_mask: memory allocation flags
> - * @mpol: NUMA memory allocation policy to be applied
> - * @ilx: NUMA interleave index, for use only when MPOL_INTERLEAVE
> - *
> - * Allocate a folio in the swap cache for one swap slot, typically before
> - * doing IO (e.g. swap in or zswap writeback). The swap slot indicated by
> - * @entry must have a non-zero swap count (swapped out).
> - * Currently only supports order 0.
> - *
> - * Context: Caller must protect the swap device with reference count or locks.
> - * Return: Returns the folio if allocation succeeded and folio is added to
> - * swap cache. Returns error code if allocation failed due to race.
> - */
> -struct folio *swap_cache_alloc_folio(swp_entry_t entry, gfp_t gfp_mask,
> - struct mempolicy *mpol, pgoff_t ilx)
> -{
> - int ret;
> - struct folio *folio;
> -
> - /* Allocate a new folio to be added into the swap cache. */
> - folio = folio_alloc_mpol(gfp_mask, 0, mpol, ilx, numa_node_id());
> - if (!folio)
> - return ERR_PTR(-ENOMEM);
> -
> - /*
> - * Try to add the new folio to the swap cache. It returns
> - * -EEXIST if the entry is already cached.
> - */
> - ret = __swap_cache_prepare_and_add(entry, folio, gfp_mask, false);
> - if (ret) {
> - folio_put(folio);
> - return ERR_PTR(ret);
> - }
> -
> - return folio;
> -}
> -
> static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp,
> struct mempolicy *mpol, pgoff_t ilx,
> struct swap_iocb **plug, bool readahead)
> {
> - struct swap_info_struct *si = __swap_entry_to_info(entry);
> struct folio *folio;
>
> /* Check the swap cache again for readahead path. */
> @@ -594,16 +700,12 @@ static struct folio *swap_cache_read_folio(swp_entry_t entry, gfp_t gfp,
> if (folio)
> return folio;
>
> - /* Skip allocation for unused and bad swap slot for readahead. */
> - if (!swap_entry_swapped(si, entry))
> - return NULL;
> -
> do {
> folio = swap_cache_get_folio(entry);
> if (folio)
> return folio;
>
> - folio = swap_cache_alloc_folio(entry, gfp, mpol, ilx);
> + folio = swap_cache_alloc_folio(entry, gfp, 0, NULL, mpol, ilx);
> } while (IS_ERR(folio) && PTR_ERR(folio) == -EEXIST);
>
> if (IS_ERR_OR_NULL(folio))
> diff --git a/mm/zswap.c b/mm/zswap.c
> index e27f6e96f003..4fcd95eb24cb 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1000,7 +1000,7 @@ static int zswap_writeback_entry(struct zswap_entry *entry,
> return -EEXIST;
>
> mpol = get_task_policy(current);
> - folio = swap_cache_alloc_folio(swpentry, GFP_KERNEL, mpol,
> + folio = swap_cache_alloc_folio(swpentry, GFP_KERNEL, 0, NULL, mpol,
> NO_INTERLEAVE_INDEX);
> put_swap_device(si);
>
>
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH 0/2] cgroup/dmem: introduce a peak file
From: Michal Koutný @ 2026-05-06 15:09 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: Tejun Heo, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, Jonathan Corbet,
Shuah Khan, Maarten Lankhorst, Maxime Ripard, Natalie Vock,
Tvrtko Ursulin, cgroups, linux-kernel, linux-mm, linux-doc,
dri-devel, kernel-dev
In-Reply-To: <aftNsqrv2sGPOPHX@quatroqueijos.cascardo.eti.br>
[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]
On Wed, May 06, 2026 at 11:18:26AM -0300, Thadeu Lima de Souza Cascardo <cascardo@igalia.com> wrote:
> I used void *, at first, but as the only current use is for the pool and as
> mixing different uses may lead to misuse, I thought it would be safer to
> use the type directly. This has been pointed out before for other members
> of cgroup_file_ctx. See [1].
That mail reacts to union overlaps and pointer vs embedded struct
allocations. Correct me if I missed your part.
I agree that having properly typed pointer is safer.
cgroup_file_ctx sub-structs are for generic cgroup files. But here
somehow a specific controller needs propagated to the generic member.
What about storing also the `list_head *watchers` inside `struct
cgroup_of_peak` and each subsys would manage it as needed?
(ofp->watchers == NULL could also substitute ofp->value ==
OFP_PEAK_UNSET)
> I started with a non-resettable peak file, but as memory.peak can be reset,
> I added that feature too.
At the same time pids.peak has survived without reset option till today.
> If we want to merge a non-resettable support ealier and need to take
> longer to discuss how to work on the resettable support given the
> above, I can resubmit. But I guess we can see if we can reach an
> agreement sonner rather than later.
What kind of users do you envision (i.e. would they need resets at all)?
Anyway, the behavior should be explained in cgroup-v2.rst since that's
where they'll look for it.
HTH,
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ permalink raw reply
* Re: [PATCH v3 03/12] mm/huge_memory: move THP gfp limit helper into header
From: Chris Li @ 2026-05-06 14:46 UTC (permalink / raw)
To: kasong
Cc: linux-mm, Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Barry Song, Hugh Dickins, Kemeng Shi, Nhat Pham, Baoquan He,
Johannes Weiner, Youngjun Park, Chengming Zhou, Roman Gushchin,
Shakeel Butt, Muchun Song, Qi Zheng, linux-kernel, cgroups,
Yosry Ahmed, Lorenzo Stoakes, Dev Jain, Lance Yang, Michal Hocko,
Michal Hocko, Suren Baghdasaryan, Axel Rasmussen
In-Reply-To: <20260421-swap-table-p4-v3-3-2f23759a76bc@tencent.com>
On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@kernel.org> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> Shmem has some special requirements for THP GFP and has to limit it in
> certain zones or provide a more lenient fallback.
>
> We'll use this helper for generic swap THP allocation, which needs to
> support shmem. For a typical GFP_HIGHUSER_MOVABLE swap-in, this helper
> is basically a no-op. But it's necessary for certain shmem users, mostly
> drivers.
>
> No feature change.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
Acked-by: Chris Li <chrisl@kernel.org>
Chris
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-05-06 14:43 UTC (permalink / raw)
To: Alejandro Lucero Palau
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: <b704b05e-3e65-4a73-84c0-21557b0cc38f@amd.com>
On Wed, Feb 25, 2026 at 12:40:09PM +0000, Alejandro Lucero Palau wrote:
>
> I can see the nid param is just a "preferred nid" with alloc pages. Using
> __GFP_PRIVATE will restrict the allocation to private nodes but I think the
> idea here is:
>
>
> 1) I own this node
>
> 2) Do not give me memory from another private node but from mine.
>
>
I mildly mis-read this question, apologies.
Multiple private nodes in the nodemask are ignored, because the nodemask
is a filter function for the fallback lists - and private nodes never
show up in the fallback lists (except for their own).
So for example
Nodes: Normal(A,B), Private(C,D)
Fallback lists:
A: [A,B]
B: [B,A]
C: [C,A,B]
D: [D,B,A]
combination | possible result
----------------------------------------------------------------
__GFP_PRIVATE + pref_node(C) + nodemask(NULL) = (C or A or B)
__GFP_PRIVATE + pref_node(C) + nodemask(C,D) = C
GFP_PRIVATE + pref_node(C) + nodemask(ALL) = C
Basically private nodes are completely ignored in the nodemask, so you
cannot do fallback allocations to other private nodes.
There is no good abstraction (that I have found) to communicate
multi-private-node allocations simply because this would imply needing
private nodes to be in the fallback lists for other nodes.
Maybe there is a possibility of modifying fallback lists explicitly, but
I think that is out of scope for the first implementation.
~Gregory
^ permalink raw reply
* Re: [PATCH v3 02/12] mm, swap: move common swap cache operations into standalone helpers
From: Chris Li @ 2026-05-06 14:42 UTC (permalink / raw)
To: kasong
Cc: linux-mm, Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Barry Song, Hugh Dickins, Kemeng Shi, Nhat Pham, Baoquan He,
Johannes Weiner, Youngjun Park, Chengming Zhou, Roman Gushchin,
Shakeel Butt, Muchun Song, Qi Zheng, linux-kernel, cgroups,
Yosry Ahmed, Lorenzo Stoakes, Dev Jain, Lance Yang, Michal Hocko,
Michal Hocko, Suren Baghdasaryan, Axel Rasmussen
In-Reply-To: <20260421-swap-table-p4-v3-2-2f23759a76bc@tencent.com>
On Tue, Apr 21, 2026 at 8:16 AM Kairui Song via B4 Relay
<devnull+kasong.tencent.com@kernel.org> wrote:
>
> From: Kairui Song <kasong@tencent.com>
>
> Move a few swap cache checking, adding, and deletion operations
> into standalone helpers to be used later. And while at it, add
> proper kernel doc.
>
> No feature or behavior change.
>
> Signed-off-by: Kairui Song <kasong@tencent.com>
Acked-by: Chris Li <chrisl@kernel.org>
> ---
> mm/swap_state.c | 141 ++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 95 insertions(+), 46 deletions(-)
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 204a9499d50c..3da285a891b2 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -137,8 +137,42 @@ void *swap_cache_get_shadow(swp_entry_t entry)
> return NULL;
> }
>
> -void __swap_cache_add_folio(struct swap_cluster_info *ci,
> - struct folio *folio, swp_entry_t entry)
> +/**
> + * __swap_cache_add_check - Check if a range is suitable for adding a folio.
> + * @ci: The locked swap cluster.
> + * @ci_off: Range start offset.
> + * @nr: Number of slots to check.
> + * @shadow: Returns the shadow value if one exists in the range.
> + *
> + * Check if all slots covered by given range have a swap count >= 1.
> + * Retrieves the shadow if there is one.
> + *
> + * Context: Caller must lock the cluster.
> + */
> +static int __swap_cache_add_check(struct swap_cluster_info *ci,
> + unsigned int ci_off, unsigned int nr,
> + void **shadow)
> +{
> + unsigned int ci_end = ci_off + nr;
> + unsigned long old_tb;
> +
Nitpick: Can add lockdep_assert_held(&ci->lock);
Can check ci_end < SWAPFILE_CLUSTER and bail out on error.
> + if (unlikely(!ci->table))
> + return -ENOENT;
> + do {
> + old_tb = __swap_table_get(ci, ci_off);
> + if (unlikely(swp_tb_is_folio(old_tb)))
> + return -EEXIST;
> + if (unlikely(!__swp_tb_get_count(old_tb)))
> + return -ENOENT;
> + if (swp_tb_is_shadow(old_tb))
> + *shadow = swp_tb_to_shadow(old_tb);
Nitpick: You can create a local variable for the shadow and assign it
at the end. Because it is a pointer, the compiler can't optimize the
store away.
Chris
^ permalink raw reply
* Re: [PATCH RFC] memcg: add per-cgroup dirty page controls (dirty_ratio, dirty_min)
From: Jan Kara @ 2026-05-06 14:21 UTC (permalink / raw)
To: haghdoost
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Matthew Wilcox (Oracle), Jan Kara,
cgroups, linux-mm, linux-kernel, linux-fsdevel, Kshitij Doshi
In-Reply-To: <20260501-rfc-memcg-dirty-v1-v1-1-9a8c80036ec1@uber.com>
On Fri 01-05-26 22:28:38, Alireza Haghdoost via B4 Relay wrote:
> From: Alireza Haghdoost <haghdoost@uber.com>
>
> Add two cgroup v2 memory-controller knobs that bring
> balance_dirty_pages() throttling into per-cgroup scope so one noisy
> writer cannot stall peers sharing the same host:
>
> memory.dirty_ratio Per-cgroup dirty-page ceiling, 0..100 percent of
> the cgroup's dirtyable memory. 0 (default) leaves
> the cgroup subject to the global threshold only.
>
> memory.dirty_min Guaranteed dirty-page floor, byte value (default 0).
>
> The two knobs compose: dirty_ratio bounds how much dirty memory a
> cgroup may accrue, dirty_min guarantees a floor below which it is
> never throttled.
>
> Motivation, design trade-offs, cost analysis, validation data, and
> open questions are in the cover letter.
>
> Co-developed-by: Kshitij Doshi <kshitijd@uber.com>
> Signed-off-by: Kshitij Doshi <kshitijd@uber.com>
> Signed-off-by: Alireza Haghdoost <haghdoost@uber.com>
> Assisted-by: Cursor:claude-sonnet-4.5
> ---
Things like motivation actually belong to the changelog itself, measured
results how the patch helps as well. On the other hand stuff like history
is largely irrelevant here, frankly I don't have a bandwidth to carefully
read the huge amount of text LLM has generated below so please try to make
it more concise for next time.
> This RFC adds two cgroup v2 memory-controller knobs that give operators
> per-cgroup control over dirty-page throttling in balance_dirty_pages():
> memory.dirty_ratio (per-cgroup ceiling) and memory.dirty_min (guaranteed
> floor). A third knob, memory.dirty_weight, is forthcoming in a follow-up
> once we have validated the application site (see "Follow-ups" below).
> We are posting this as an RFC, as a single squashed patch, to get design
> feedback before splitting the prototype into a per-logical-change series.
>
> Motivation
> ==========
>
> balance_dirty_pages() (BDP) is a global throttle. It sleeps writers
> once the host-wide dirty count crosses a single threshold. On a
> container host that threshold is shared across cgroups. A cgroup
> that dirties pages faster than storage can drain them pushes the
> count over the limit. Every writer on the host then parks in
> io_schedule_timeout() -- including cgroups that have not dirtied a
> single page of their own.
>
> cgroup v2 already has per-memcg dirty accounting, but that accounting
> does not translate into per-memcg dirty throttling.
Not quite true. We do have per-memcg writeback workers and we do have
per-memcg dirty limits (inferred from global dirty limit tunings) that are
enforced...
> We see this in production: a buffered write-heavy container generates
> multi-second stalls for co-located latency-sensitive workloads.
> Moreover, dirty-page accumulation from a single noisy neighbor is a
> recurring contributor to mount-responsiveness degradation on shared hosts.
... and I quite don't see how a multisecond stalls you are describing would
happen. There is something I must be missing. The throttling works as
follows: Until we cross global freerun limit (that is (background_limit +
dirty_limit) / 2) nobody is throttled. Once we cross it, memcg dirty limits
start to be taken into account. If we are below freerun in the memcg, the
task dirtying folios from that memcg shouldn't be throttled at all, once we
get above freerun we throttle by maximum of throttling delay decided from
global and memcg situation so then long delays can start happening but it
would mean the "innocent" task's memcg had to get at least over the freerun
limit.
So can you perhaps share more details about the configuration where you
observe these delays to innocent tasks due to another task dirtying a lot of
memory? How many page cache in total and dirty pages are there in each
memcg (both for the aggressive dirtier and wrongly delayed task)? Is the
delayed task really throttled in balance_dirty_pages()?
Honza
> Prior work
> ==========
>
> Per-memcg dirty-page limits have been proposed before. Andrea Righi
> posted an initial RFC in February 2010 [1]; Greg Thelen continued the
> work through v9 in August 2011 [2]. That series added per-memcg dirty
> counters and hooked them into balance_dirty_pages(), but it bolted
> per-cgroup limits onto the global writeback path without making
> writeback itself cgroup-aware. Without cgroup-aware flusher threads,
> a cgroup exceeding its limit triggered writeback of inodes from any
> cgroup, giving poor isolation. The series was not merged.
>
> Konstantin Khlebnikov posted "[PATCHSET RFC 0/6] memcg: inode-based
> dirty-set controller" in January 2015 [4], which proposed
> memory.dirty_ratio (the same interface name this series uses) via an
> inode-tagged, filtered-writeback approach. Tejun Heo reviewed it
> and rejected it as a "dead end" that duplicated lower-layer policy
> without solving the underlying isolation problem; this rejection
> directly motivated Tejun's native cgwb rework described below.
>
> Tejun Heo's 48-patch cgroup-writeback rework, merged in Linux 4.2
> (commit e4bc13adfd01, "Merge branch 'for-4.2/writeback'"), took the
> different approach of restructuring writeback to be natively
> cgroup-aware: per-memcg wb_domain (commit 841710aa6e4a), per-memcg
> NR_FILE_DIRTY / NR_WRITEBACK accounting, and cgroup-aware flusher
> threads [3]. That work deliberately deferred user-facing policy knobs.
> This series adds the policy surface that consumes Tejun's
> infrastructure. The dirty_min reservation concept is, to our
> knowledge, new.
>
> A November 2023 LKML thread by Chengming Zhou [5] independently
> identified the identical throttling regression on cgroup v2 (a 5 GB
> container constantly throttled because memory.max * dirty_ratio yields
> too small a threshold for bursty workloads). Jan Kara participated and
> endorsed a bytes-based per-memcg dirty limit; no patches followed that
> discussion, confirming the gap this series fills.
>
> [1] https://lwn.net/Articles/408349/
> [2] https://lore.kernel.org/lkml/1313597705-6093-1-git-send-email-gthelen@google.com/
> [3] https://lwn.net/Articles/648292/
> [4] https://lore.kernel.org/all/20150115180242.10450.92.stgit@buzz/
> [5] https://lore.kernel.org/all/109029e0-1772-4102-a2a8-ab9076462454@linux.dev/
>
> Proposed interface
> ==================
>
> Two new cgroup v2 files under the memory controller, absent on the root
> cgroup (CFTYPE_NOT_ON_ROOT):
>
> memory.dirty_ratio
> Integer [0, 100]. Per-cgroup dirty-page ceiling as a percentage of
> the cgroup's dirtyable memory (mdtc->avail: file cache plus
> reclaimable slack), the same base the global vm_dirty_ratio scales
> against. 0 (the default) disables the per-cgroup ceiling and leaves
> the cgroup subject to the global threshold only. A non-zero value
> that is stricter than vm_dirty_ratio overrides the global ratio for
> this cgroup via min(mdtc->thresh, cg_thresh); because both sides
> scale off the same base, the knob can never widen the cgroup past
> the global ceiling. A memory.dirty_bytes companion for byte-precise
> caps (mirroring vm_dirty_bytes) is noted under "Follow-ups" below.
> The prototype reads the value for the immediate memcg only;
> hierarchical enforcement (clamping against ancestors, like
> memory.max) is not implemented yet. We would like guidance on
> whether this is required for v1 or can follow in a subsequent
> series.
>
> memory.dirty_min
> Byte value (K/M/G suffixes accepted), default 0 (no reservation).
> Guaranteed dirty-page floor: while cgroup_dirty < dirty_min,
> throttling is bypassed (goto free_running). Lets a latency-sensitive
> cgroup buffer a small write burst even under global dirty pressure.
>
> dirty_min is an admission guarantee, so we have to prevent it from
> breaking the global dirty invariant. Two aspects:
>
> - Global cap. The sum of dirty_min reservations across all cgroups
> must not exceed a fraction of the global dirty threshold (our
> working number is 80%), so the system always retains some shared
> capacity. The prototype does not enforce this cap yet; we expect
> to either reject at write() time or clamp on read in a cheap
> precomputed effective_dirty_min. We would appreciate feedback on
> which approach the cgroup maintainers prefer.
>
> - Per-cgroup cap. A cgroup should not be able to reserve more dirty
> capacity than it can hold. Our tentative rule is
> effective_dirty_min = min(dirty_min, memory.max - memory.min),
> evaluated at enforcement time so it tracks live memory.max changes,
> rather than rejected at write. This is similar to how memory.low
> composes with memory.max.
>
> Neither cap is implemented in the prototype; both would land before
> a non-RFC posting.
>
> The two knobs compose: dirty_ratio bounds how much dirty memory a
> cgroup may accrue, dirty_min guarantees a floor below which it is never
> throttled.
>
> Test setup and results
> ======================
>
> To show the problem and the fix, we built a single reproducer that runs
> on an unmodified stock kernel and then on the patched kernel, using the
> same setup for both.
>
> Setup: QEMU guest with a virtio-blk disk throttled to 256 KB/s
> (bps_wr=262144). Two sibling cgroups, no io.weight; both share disk
> bandwidth equally. dirty_bytes=32MB, dirty_background_bytes=16MB
> (freerun = 24 MB). Files pre-allocated with fallocate before dirty
> pressure. Two phases per run: (1) victim alone (baseline), (2) noisy
> fills global dirty to the 32 MB cap, then victim runs contended for
> 30 s.
>
> - noisy: single fio job, unlimited write rate, fills global dirty pool.
> - victim: single fio job, rate-limited to 500 KB/s (128 IOPS target),
> 4 KiB sequential write().
>
> The high freerun (24 MB) ensures victim's solo dirty accumulation
> (244 KB/s x 30 s = 7.3 MB) stays below the threshold. BDP does not fire
> during the solo phase on either kernel, giving identical baselines.
>
> Stock kernel results (the problem):
>
> solo (no contention) contended inflation
> victim IOPS 125 5.1 24.5x worse
> victim p99 0.6 ms 152 ms 253x worse
>
> The contended p99 sits at fio's percentile bucket nearest MAX_PAUSE =
> HZ/5 = 200 ms (mm/page-writeback.c:49), the hard kernel ceiling on BDP
> sleep. The victim has near-zero dirty pages of its own but is forced to
> sleep because balance_dirty_pages() sees gdtc->dirty = NR_FILE_DIRTY +
> NR_WRITEBACK above the freerun threshold. Most of noisy's pages are
> queued in NR_WRITEBACK waiting for the throttled disk.
> memory.events.local on both cgroups shows max 0 throughout the run;
> this is not memory pressure inside either cgroup.
>
> Patched kernel results (the fix), with victim/memory.dirty_min = 16 MB:
>
> solo (no contention) contended inflation
> victim IOPS 125 125 1.0x
> victim p99 0.9 ms 0.7 ms 1.0x
>
> The patched kernel checks cgroup_dirty < dirty_min (4096 pages) before
> computing any sleep. Because the rate-limited victim's resident dirty
> set stays well below the reservation, the check fires on every write()
> -> goto free_running -> write() returns in ~7 us. The victim is fully
> protected.
>
> The figures above are the per-kernel medians of N=5 iterations and
> reflect a deterministic outcome on every iter: stock had cont_iops in
> 4.4..6.1 (retention 0.035..0.049) on all five iters, patched had
> cont_iops = 125.0 (retention = 1.000) on all five iters. The 5/5 stock
> iters all hit BDP's throttled regime; the 5/5 patched iters all
> bypassed it via the dirty_min check.
>
> Implementation
> ==============
>
> The patch touches five files:
>
> - include/linux/memcontrol.h: two new fields on struct mem_cgroup
> (dirty_ratio, dirty_min).
> - include/linux/writeback.h: a per-pass cg_dirty_cap field on
> struct dirty_throttle_control used to publish the memcg clamp to
> BDP's setpoint and rate-limit math.
> - include/trace/events/writeback.h: cg_dirty_cap added to the
> balance_dirty_pages tracepoint so operators can distinguish the
> memcg clamp from the global dirty_limit at runtime.
> - mm/memcontrol.c: registers the two cgroup v2 files with input
> validation.
> - mm/page-writeback.c: the throttling changes.
>
> The key changes in page-writeback.c:
>
> - The dirty_ratio clamp lives inside domain_dirty_limits(), keyed
> on wb->memcg_css (the inode owner's memcg). Every consumer of
> the memcg dtc -- writer throttle, flusher kworker,
> cgwb_calc_thresh -- sees the same clamped thresh and bg_thresh.
> The clamp uses mult_frac() so a small memcg does not collapse
> to a zero ceiling.
>
> - The dirty_min bypass lives in balance_dirty_pages() and is
> writer-keyed: the writing task's memcg is looked up under RCU,
> and when its dirty+in-flight backlog is below dirty_min the
> loop jumps to free_running, bypassing both the global and the
> per-memcg BDP gates. dirty_min is an admission guarantee for
> the writer's own cgroup, not for inode owners.
>
> - When the clamp engages, mdtc->dirty is replaced with the
> memcg-wide NR_FILE_DIRTY + NR_WRITEBACK sum so freerun /
> setpoint / rate-limit smoothing see the real backlog and pages
> migrating from NR_FILE_DIRTY into NR_WRITEBACK cannot silently
> widen the cap.
>
> Fast-path cost when neither knob is set: one rcu_read_lock/unlock
> pair plus a READ_ONCE(dirty_min) per balance_dirty_pages()
> iteration, and one READ_ONCE(dirty_ratio) per domain_dirty_limits()
> call. The memcg counter reads are gated on "knob armed" and do
> not fire on the default path. We have not measured the added
> cost yet, but we expect it to be in the noise of existing BDP
> bookkeeping. A tight pwrite() microbenchmark will confirm this
> before a non-RFC posting.
>
> Scope
> =====
>
> This series affects the writer-side throttle (balance_dirty_pages())
> only. It does not partition the flusher-side writeback queue. A
> cgroup's fsync() can still block behind inodes from other cgroups in
> writeback_sb_inodes(). We document this limit explicitly and expect
> writeback-queue partitioning to be a separate, larger effort.
>
> Interaction with block-layer throttles
> ======================================
>
> The two knobs are orthogonal to io.max / io.cost. balance_dirty_pages()
> runs before the bio reaches the block layer, so dirty_min simply allows
> a cgroup to keep accepting write() syscalls up to its reservation; the
> actual I/O is still subject to whatever block throttle is in effect. In
> the reproducer above, the disk-level bandwidth limit (256 KB/s) is
> applied at the QEMU virtio-blk layer, and the protected victim dirties
> roughly equal to the rate it can drain, so the block throttle is
> exercised on both kernels. We have not yet tested interaction with
> guest-side io.max settings; this is on the list before a non-RFC
> posting.
>
> Questions for maintainers
> =========================
>
> 1. Is writer-throttle-only scope (no flusher/writeback-queue work)
> acceptable for the first series?
> 2. Does dirty_ratio belong on struct mem_cgroup (as the prototype
> has it) or on the memcg's wb_domain? Routing it through wb_domain
> would let us reuse __wb_calc_thresh() and keep all threshold
> policy in one place; a possible split is dirty_ratio on wb_domain
> (it is threshold policy), dirty_min on the memcg (throttle
> bypass). We can go either way.
> 3. For dirty_min safety caps (per-cgroup and global sum), which
> approach do you prefer: reject at write time, or clamp on read in
> an effective_dirty_min?
> 4. Is hierarchical enforcement of dirty_ratio (clamp against
> ancestors) required for v1, or can it follow in a subsequent
> series?
>
> What's missing before a non-RFC posting
> =======================================
>
> - Split the monolithic prototype into a proper series (one patch per
> concept + Documentation + selftest).
> - Documentation/admin-guide/cgroup-v2.rst entries for both knobs.
> - tools/testing/selftests/cgroup/ test for interface surface and
> noisy-neighbor protection.
> - Implement the per-cgroup and global dirty_min safety caps described
> in the memory.dirty_min bullet.
> - Fast-path microbenchmark: confirm zero measurable regression for
> cgroups that have neither knob set.
> - Larger-N validation on real hardware (the current N=5 data is from
> a QEMU guest on a throttled virtio-blk).
>
> Follow-ups (out of scope for this series)
> =========================================
>
> - memory.dirty_weight: a priority weight knob that scales the BDP
> pause length, planned as a separate series. The prototype validated
> the interface surface but the application site (post-pause scaling
> vs. folding into pos_ratio / dirty_ratelimit) needs to be settled
> before we ship it. Happy to discuss in advance of that posting.
> - memory.dirty_bytes: a byte-value companion to memory.dirty_ratio,
> mirroring the global vm_dirty_bytes. For operators who want a
> byte-predictable per-cgroup dirty cap rather than a ratio of the
> cgroup's dirtyable memory. We have not prototyped this yet; we
> are listing it so reviewers know it is on the roadmap, since
> the ratio-only interface omits that use case.
> - Writeback-queue partitioning: flusher-side fairness across
> cgroups, as noted in Scope above.
>
> Looking forward to feedback.
>
> Thanks,
> Alireza Haghdoost <haghdoost@uber.com>
> Kshitij Doshi <kshitijd@uber.com>
> ---
> include/linux/memcontrol.h | 10 +++
> include/linux/writeback.h | 4 +
> include/trace/events/writeback.h | 5 +-
> mm/memcontrol.c | 62 ++++++++++++++
> mm/page-writeback.c | 179 ++++++++++++++++++++++++++++++++++++---
> 5 files changed, 249 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index dc3fa687759b..45ca949a4c68 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -323,6 +323,16 @@ struct mem_cgroup {
> spinlock_t event_list_lock;
> #endif /* CONFIG_MEMCG_V1 */
>
> + /* Per-memcg dirty-page controls (memory.dirty_ratio, memory.dirty_min) */
> + /*
> + * dirty_ratio: [0, 100] percent of dirtyable memory (mdtc->avail),
> + * matching the global vm_dirty_ratio base; 0 inherits the global
> + * threshold.
> + * dirty_min: dirty-page reservation, in pages; 0 disables the bypass.
> + */
> + unsigned int dirty_ratio;
> + unsigned long dirty_min;
> +
> struct mem_cgroup_per_node *nodeinfo[];
> };
>
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index 62552a2ce5b9..e37632f728be 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -318,6 +318,10 @@ struct dirty_throttle_control {
> unsigned long thresh; /* dirty threshold */
> unsigned long bg_thresh; /* dirty background threshold */
> unsigned long limit; /* hard dirty limit */
> + unsigned long cg_dirty_cap; /* per-memcg dirty_ratio clamp for
> + * this pass, or PAGE_COUNTER_MAX
> + * when no memcg clamp applies
> + */
>
> unsigned long wb_dirty; /* per-wb counterparts */
> unsigned long wb_thresh;
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index bdac0d685a98..0bf86b3c903c 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -672,6 +672,7 @@ TRACE_EVENT(balance_dirty_pages,
> __array( char, bdi, 32)
> __field(u64, cgroup_ino)
> __field(unsigned long, limit)
> + __field(unsigned long, cg_dirty_cap)
> __field(unsigned long, setpoint)
> __field(unsigned long, dirty)
> __field(unsigned long, wb_setpoint)
> @@ -691,6 +692,7 @@ TRACE_EVENT(balance_dirty_pages,
> strscpy_pad(__entry->bdi, bdi_dev_name(wb->bdi), 32);
>
> __entry->limit = dtc->limit;
> + __entry->cg_dirty_cap = dtc->cg_dirty_cap;
> __entry->setpoint = (dtc->limit + freerun) / 2;
> __entry->dirty = dtc->dirty;
> __entry->wb_setpoint = __entry->setpoint *
> @@ -710,13 +712,14 @@ TRACE_EVENT(balance_dirty_pages,
>
>
> TP_printk("bdi %s: "
> - "limit=%lu setpoint=%lu dirty=%lu "
> + "limit=%lu cg_dirty_cap=%lu setpoint=%lu dirty=%lu "
> "wb_setpoint=%lu wb_dirty=%lu "
> "dirty_ratelimit=%lu task_ratelimit=%lu "
> "dirtied=%u dirtied_pause=%u "
> "paused=%lu pause=%ld period=%lu think=%ld cgroup_ino=%llu",
> __entry->bdi,
> __entry->limit,
> + __entry->cg_dirty_cap,
> __entry->setpoint,
> __entry->dirty,
> __entry->wb_setpoint,
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c3d98ab41f1f..c43fe4f394eb 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4748,6 +4748,56 @@ static ssize_t memory_max_write(struct kernfs_open_file *of,
> return nbytes;
> }
>
> +static int memory_dirty_ratio_show(struct seq_file *m, void *v)
> +{
> + struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> +
> + seq_printf(m, "%u\n", READ_ONCE(memcg->dirty_ratio));
> + return 0;
> +}
> +
> +static ssize_t memory_dirty_ratio_write(struct kernfs_open_file *of,
> + char *buf, size_t nbytes, loff_t off)
> +{
> + struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
> + unsigned int ratio;
> + int err;
> +
> + err = kstrtouint(strstrip(buf), 0, &ratio);
> + if (err)
> + return err;
> +
> + if (ratio > 100)
> + return -EINVAL;
> +
> + WRITE_ONCE(memcg->dirty_ratio, ratio);
> + return nbytes;
> +}
> +
> +static int memory_dirty_min_show(struct seq_file *m, void *v)
> +{
> + struct mem_cgroup *memcg = mem_cgroup_from_seq(m);
> +
> + /* seq_puts_memcg_tunable automatically multiplies by PAGE_SIZE for the user */
> + return seq_puts_memcg_tunable(m, READ_ONCE(memcg->dirty_min));
> +}
> +
> +static ssize_t memory_dirty_min_write(struct kernfs_open_file *of,
> + char *buf, size_t nbytes, loff_t off)
> +{
> + struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
> + unsigned long dirty_min;
> + int err;
> +
> + /* page_counter_memparse converts strings like "512M" into a page count */
> + err = page_counter_memparse(strstrip(buf), "max", &dirty_min);
> + if (err)
> + return err;
> +
> + WRITE_ONCE(memcg->dirty_min, dirty_min);
> + return nbytes;
> +}
> +
> /*
> * Note: don't forget to update the 'samples/cgroup/memcg_event_listener'
> * if any new events become available.
> @@ -4950,6 +5000,18 @@ static struct cftype memory_files[] = {
> .flags = CFTYPE_NS_DELEGATABLE,
> .write = memory_reclaim,
> },
> + {
> + .name = "dirty_ratio",
> + .flags = CFTYPE_NOT_ON_ROOT,
> + .seq_show = memory_dirty_ratio_show,
> + .write = memory_dirty_ratio_write,
> + },
> + {
> + .name = "dirty_min",
> + .flags = CFTYPE_NOT_ON_ROOT,
> + .seq_show = memory_dirty_min_show,
> + .write = memory_dirty_min_write,
> + },
> { } /* terminate */
> };
>
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 88cd53d4ba09..2847b2c1e59a 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -124,14 +124,17 @@ struct wb_domain global_wb_domain;
>
> #define GDTC_INIT(__wb) .wb = (__wb), \
> .dom = &global_wb_domain, \
> - .wb_completions = &(__wb)->completions
> + .wb_completions = &(__wb)->completions, \
> + .cg_dirty_cap = PAGE_COUNTER_MAX
>
> -#define GDTC_INIT_NO_WB .dom = &global_wb_domain
> +#define GDTC_INIT_NO_WB .dom = &global_wb_domain, \
> + .cg_dirty_cap = PAGE_COUNTER_MAX
>
> #define MDTC_INIT(__wb, __gdtc) .wb = (__wb), \
> .dom = mem_cgroup_wb_domain(__wb), \
> .wb_completions = &(__wb)->memcg_completions, \
> - .gdtc = __gdtc
> + .gdtc = __gdtc, \
> + .cg_dirty_cap = PAGE_COUNTER_MAX
>
> static bool mdtc_valid(struct dirty_throttle_control *dtc)
> {
> @@ -183,8 +186,9 @@ static void wb_min_max_ratio(struct bdi_writeback *wb,
> #else /* CONFIG_CGROUP_WRITEBACK */
>
> #define GDTC_INIT(__wb) .wb = (__wb), \
> - .wb_completions = &(__wb)->completions
> -#define GDTC_INIT_NO_WB
> + .wb_completions = &(__wb)->completions, \
> + .cg_dirty_cap = PAGE_COUNTER_MAX
> +#define GDTC_INIT_NO_WB .cg_dirty_cap = PAGE_COUNTER_MAX
> #define MDTC_INIT(__wb, __gdtc)
>
> static bool mdtc_valid(struct dirty_throttle_control *dtc)
> @@ -392,6 +396,58 @@ static void domain_dirty_limits(struct dirty_throttle_control *dtc)
> bg_thresh += bg_thresh / 4 + global_wb_domain.dirty_limit / 32;
> thresh += thresh / 4 + global_wb_domain.dirty_limit / 32;
> }
> +
> + /*
> + * Apply the per-memcg dirty_ratio clamp on mdtc (gdtc != NULL
> + * iff @dtc is a memcg dtc). dirty_ratio is scaled against
> + * the memcg's own dirtyable memory (@available_memory), matching
> + * the semantics of vm_dirty_ratio so the two knobs share a base
> + * and compose via a plain min() on thresh. The clamp is keyed
> + * on wb->memcg_css (the inode-owner's memcg) rather than on
> + * current's memcg, so balance_dirty_pages(), wb_over_bg_thresh()
> + * (flusher kworker context), and cgwb_calc_thresh() all see the
> + * same clamped value.
> + *
> + * Published on dtc->cg_dirty_cap as well so hard_dirty_limit()
> + * callers in balance_dirty_pages() can ignore the slower
> + * dom->dirty_limit smoothing when deriving setpoint/
> + * rate-limit from the clamped ceiling.
> + *
> + * Clamp is applied after the rt/dl boost: dirty_ratio is a
> + * strict override, not widened by priority. bg_thresh is
> + * scaled by the same factor we apply to thresh so the
> + * user-configured bg/thresh ratio survives clamping instead
> + * of snapping to thresh/2 via the bg_thresh >= thresh guard
> + * below. mult_frac() preserves precision for small memcgs
> + * where a plain "(avail / 100) * ratio" would collapse to 0.
> + */
> + if (gdtc) {
> + struct mem_cgroup *memcg =
> + mem_cgroup_from_css(dtc->wb->memcg_css);
> + unsigned int cg_ratio = memcg ?
> + READ_ONCE(memcg->dirty_ratio) : 0;
> +
> + /*
> + * dtc is reused across balance_dirty_pages() iterations,
> + * so reset the published clamp every call -- an admin
> + * clearing memory.dirty_ratio mid-flight must take effect
> + * on the next pass.
> + */
> + dtc->cg_dirty_cap = PAGE_COUNTER_MAX;
> +
> + if (cg_ratio) {
> + unsigned long cg_thresh = mult_frac(available_memory,
> + cg_ratio, 100);
> +
> + if (cg_thresh < thresh) {
> + bg_thresh = mult_frac(bg_thresh, cg_thresh,
> + thresh);
> + thresh = cg_thresh;
> + dtc->cg_dirty_cap = cg_thresh;
> + }
> + }
> + }
> +
> /*
> * Dirty throttling logic assumes the limits in page units fit into
> * 32-bits. This gives 16TB dirty limits max which is hopefully enough.
> @@ -1065,7 +1121,9 @@ static void wb_position_ratio(struct dirty_throttle_control *dtc)
> struct bdi_writeback *wb = dtc->wb;
> unsigned long write_bw = READ_ONCE(wb->avg_write_bandwidth);
> unsigned long freerun = dirty_freerun_ceiling(dtc->thresh, dtc->bg_thresh);
> - unsigned long limit = dtc->limit = hard_dirty_limit(dtc_dom(dtc), dtc->thresh);
> + unsigned long limit = dtc->limit = min(hard_dirty_limit(dtc_dom(dtc),
> + dtc->thresh),
> + dtc->cg_dirty_cap);
> unsigned long wb_thresh = dtc->wb_thresh;
> unsigned long x_intercept;
> unsigned long setpoint; /* dirty pages' target balance point */
> @@ -1334,7 +1392,8 @@ static void wb_update_dirty_ratelimit(struct dirty_throttle_control *dtc,
> struct bdi_writeback *wb = dtc->wb;
> unsigned long dirty = dtc->dirty;
> unsigned long freerun = dirty_freerun_ceiling(dtc->thresh, dtc->bg_thresh);
> - unsigned long limit = hard_dirty_limit(dtc_dom(dtc), dtc->thresh);
> + unsigned long limit = min(hard_dirty_limit(dtc_dom(dtc), dtc->thresh),
> + dtc->cg_dirty_cap);
> unsigned long setpoint = (freerun + limit) / 2;
> unsigned long write_bw = wb->avg_write_bandwidth;
> unsigned long dirty_ratelimit = wb->dirty_ratelimit;
> @@ -1822,22 +1881,122 @@ static int balance_dirty_pages(struct bdi_writeback *wb,
> int ret = 0;
>
> for (;;) {
> + unsigned long cg_dirty_min = 0;
> + unsigned long cg_dirty_pages = 0;
> unsigned long now = jiffies;
>
> nr_dirty = global_node_page_state(NR_FILE_DIRTY);
>
> balance_domain_limits(gdtc, strictlimit);
> +
> + /*
> + * Under RCU, snapshot the current memcg's memory.dirty_min
> + * reservation. When it is non-zero, also snapshot the
> + * memcg-wide dirty backlog. These feed the per-writer
> + * dirty_min bypass below; the dirty_ratio clamp itself
> + * is applied inside domain_dirty_limits() keyed on
> + * wb->memcg_css so balance_dirty_pages(),
> + * wb_over_bg_thresh() (flusher kworker context), and
> + * cgwb_calc_thresh() all see a consistent clamped
> + * threshold.
> + *
> + * rcu_read_lock() is held only for the __rcu dereference
> + * of current->cgroups; the memcg pointer does not escape
> + * the critical section. The counter read matches
> + * domain_dirty_avail(mdtc, true) so the bypass compares
> + * the same dirty+in-flight backlog the global path uses.
> + */
> + rcu_read_lock();
> + {
> + struct mem_cgroup *memcg =
> + mem_cgroup_from_task(current);
> +
> + if (memcg) {
> + cg_dirty_min = READ_ONCE(memcg->dirty_min);
> + if (cg_dirty_min)
> + cg_dirty_pages =
> + memcg_page_state(memcg,
> + NR_FILE_DIRTY) +
> + memcg_page_state(memcg,
> + NR_WRITEBACK);
> + }
> + }
> + rcu_read_unlock();
> +
> if (mdtc) {
> /*
> - * If @wb belongs to !root memcg, repeat the same
> - * basic calculations for the memcg domain.
> + * For !root memcg, repeat the same three-step
> + * sequence as balance_domain_limits(gdtc):
> + * avail -> limits -> freerun. We inline it here
> + * so we can insert the mdtc->dirty override
> + * between step 2 (domain_dirty_limits, which
> + * publishes the per-memcg dirty_ratio clamp on
> + * cg_dirty_cap) and step 3 (domain_dirty_freerun,
> + * which consumes mdtc->dirty along with
> + * thresh/bg_thresh).
> + */
> + domain_dirty_avail(mdtc, true);
> + domain_dirty_limits(mdtc);
> +
> + /*
> + * When the dirty_ratio clamp engaged, replace the
> + * per-wb dirty count from mem_cgroup_wb_stats()
> + * with the memcg-wide NR_FILE_DIRTY + NR_WRITEBACK
> + * sum so freerun, the setpoint, and the rate-limit
> + * smoothing see the true memcg backlog instead of
> + * the subset that has migrated to this cgwb (cgwb
> + * migration is lazy and can lag by many seconds),
> + * and so a burst of buffered writes cannot silently
> + * bypass the clamp by shifting pages from
> + * NR_FILE_DIRTY into NR_WRITEBACK.
> + *
> + * Keyed on wb->memcg_css to match the clamp itself.
> + * The cgwb holds a css reference, so the memcg
> + * pointer is stable without additional locking.
> + *
> + * Caveat: memcg_page_state() aggregates across ALL
> + * backing devices owned by this memcg, while mdtc
> + * is scoped to one wb. A writer to a fast BDI may
> + * observe backlog accumulated on slow BDIs in the
> + * same memcg and throttle more than strictly needed.
> + * Accepted for v1; the alternative (summing per-wb
> + * dirty across the memcg's cgwbs) walks the cgwb
> + * list under RCU on a hot path.
> */
> - balance_domain_limits(mdtc, strictlimit);
> + if (mdtc->cg_dirty_cap != PAGE_COUNTER_MAX) {
> + struct mem_cgroup *wb_memcg =
> + mem_cgroup_from_css(mdtc->wb->memcg_css);
> +
> + if (wb_memcg)
> + mdtc->dirty =
> + memcg_page_state(wb_memcg,
> + NR_FILE_DIRTY) +
> + memcg_page_state(wb_memcg,
> + NR_WRITEBACK);
> + }
> +
> + domain_dirty_freerun(mdtc, strictlimit);
> }
>
> if (nr_dirty > gdtc->bg_thresh && !writeback_in_progress(wb))
> wb_start_background_writeback(wb);
>
> + /*
> + * dirty_min bypass: when the current memcg's dirty+in-flight
> + * backlog is below its memory.dirty_min reservation, let the
> + * writer proceed without throttling. This check must live
> + * outside the if (mdtc) block because a writer's file may not
> + * yet have been migrated to a cgwb; without cgwb, mdtc is NULL
> + * and the per-memcg block above is skipped entirely.
> + *
> + * cg_dirty_min and cg_dirty_pages come from the per-iteration
> + * snapshot taken above under rcu_read_lock; both are stored
> + * in pages (page_counter_memparse converts bytes -> pages for
> + * dirty_min), so no unit conversion is needed.
> + */
> + if (cg_dirty_min && cg_dirty_pages < cg_dirty_min)
> + goto free_running;
> +
> /*
> * If memcg domain is in effect, @dirty should be under
> * both global and memcg freerun ceilings.
>
> ---
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> change-id: 20260501-rfc-memcg-dirty-v1-ed4644c3fa8a
>
> Best regards,
> --
> Alireza Haghdoost <haghdoost@uber.com>
>
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ 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