* Re: [PATCH 2/5] bfq: protect q->blkg_list iteration in bfq_end_wr_async() with blkcg_mutex
From: Nilay Shroff @ 2026-06-04 17:31 UTC (permalink / raw)
To: Yu Kuai, Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Bart Van Assche, linux-block,
cgroups, linux-kernel
In-Reply-To: <89f9448c5d703e6123e1be6c8e0550c803e9c057.1780492756.git.yukuai@fygo.io>
On 6/3/26 6:57 PM, Yu Kuai wrote:
> bfq_end_wr_async() iterates q->blkg_list while only holding bfqd->lock,
> but not blkcg_mutex. This can race with blkg_free_workfn() that removes
> blkgs from the list while holding blkcg_mutex.
>
> Add blkcg_mutex protection in bfq_end_wr() before taking bfqd->lock to
> ensure proper synchronization when iterating q->blkg_list.
>
> Signed-off-by: Yu Kuai <yukuai@fygo.io>
> ---
> block/bfq-cgroup.c | 3 ++-
> block/bfq-iosched.c | 6 ++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
> index 37ab70930c8d..f765e767d36a 100644
> --- a/block/bfq-cgroup.c
> +++ b/block/bfq-cgroup.c
> @@ -939,11 +939,12 @@ void bfq_end_wr_async(struct bfq_data *bfqd)
> struct blkcg_gq *blkg;
>
> list_for_each_entry(blkg, &bfqd->queue->blkg_list, q_node) {
> struct bfq_group *bfqg = blkg_to_bfqg(blkg);
>
> - bfq_end_wr_async_queues(bfqd, bfqg);
> + if (bfqg)
> + bfq_end_wr_async_queues(bfqd, bfqg);
> }
> bfq_end_wr_async_queues(bfqd, bfqd->root_group);
> }
>
> static int bfq_io_show_weight_legacy(struct seq_file *sf, void *v)
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 141c602d5e85..42ccfd0c6140 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -2643,10 +2643,13 @@ void bfq_end_wr_async_queues(struct bfq_data *bfqd,
> static void bfq_end_wr(struct bfq_data *bfqd)
> {
> struct bfq_queue *bfqq;
> int i;
>
> +#ifdef CONFIG_BFQ_GROUP_IOSCHED
> + mutex_lock(&bfqd->queue->blkcg_mutex);
> +#endif
> spin_lock_irq(&bfqd->lock);
>
> for (i = 0; i < bfqd->num_actuators; i++) {
> list_for_each_entry(bfqq, &bfqd->active_list[i], bfqq_list)
> bfq_bfqq_end_wr(bfqq);
> @@ -2654,10 +2657,13 @@ static void bfq_end_wr(struct bfq_data *bfqd)
> list_for_each_entry(bfqq, &bfqd->idle_list, bfqq_list)
> bfq_bfqq_end_wr(bfqq);
> bfq_end_wr_async(bfqd);
>
> spin_unlock_irq(&bfqd->lock);
> +#ifdef CONFIG_BFQ_GROUP_IOSCHED
> + mutex_unlock(&bfqd->queue->blkcg_mutex);
> +#endif
> }
The above change protects the q->blkg_list iteration in bfq_end_wr_async()
against list removal in blkg_free_workfn(). However the blkg insertion in
blkg_create() still doesn't use q->blkcg_mutex and so list traversal in
bfq_end_wr_async() may still race with blkg_create().
So I think we may also need to protect blkg insert in blkg_create() using
q->blkcg_mutex.
Thanks,
--Nilay
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Nhat Pham @ 2026-06-04 17:23 UTC (permalink / raw)
To: Hao Jia
Cc: Yosry Ahmed, akpm, tj, hannes, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <a60eedb6-f3fd-4092-b726-04a17a695ace@gmail.com>
On Thu, Jun 4, 2026 at 6:06 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> On 2026/6/4 13:34, Yosry Ahmed wrote:
> >>>> For instance, suppose a parent memcg has two children, memcg1 and memcg2,
> >>>> each with 200MB of zswap (100MB inactive). Triggering proactive writeback on
> >>>> the parent memcg will exhaust memcg1's inactive zswap pages. After that,
> >>>> even though memcg2 still has plenty of inactive zswap pages, it will
> >>>> continue to write back memcg1's active zswap pages. Writing back active
> >>>> zswap pages causes the user-space agent to prematurely abort the writeback
> >>>> because it detects that certain memcg metrics have exceeded predefined
> >>>> thresholds.
> >>>
> >>> This will only happen if the reclaim size is smaller than the batch
> >>> size, right? Otherwise the kernel should reclaim more or less equally
> >>> from both memcgs?
> >>>
> >>
> >> I gave it some thought. Not using a cursor could lead to unfairness
> >> issues with certain writeback sizes:
> >>
> >> - If the writeback size is an odd multiple of WB_BATCH (e.g.,
> >> triggering a writeback of 3 * WB_BATCH), with 2 child cgroups, the
> >> writeback ratio might end up being 2:1.
> >> - If a memcg has 5 child cgroups and a writeback of 2 * WB_BATCH is
> >> triggered, it might repeatedly write back from only the first 2 child
> >> cgroups.
> >>
> >> Although setting a smaller WB_BATCH might mitigate this unfairness, it
> >> could hurt writeback efficiency. Let's just use per-memcg cursors to
> >> completely fix these corner cases.
> >
> > Exactly, the batch size should be small enough that any unfairness is
> > not a problem. I would honestly just do batching without a per-memcg
> > cursor, unless we have numbers to prove that the efficiency is
> > affected when we use a small batch size. Let's only introduce
> > complexity when needed please.
I'm impartial towards the complexity of per-memcg cursor. I don't
think it's that big of a deal, but only if it's warranted.
Hao, if you're convinced that doing small batch is not efficient,
could you run some experiments to show the improvement bigger batchign
and fairness? Maybe implement a small batch, no-memcg cursor first.
Then implement a patch on top of it to add per-memcg cursor, and show
how much performance win we can get from that patch on top of the
patch series?
FWIW, zswap writeback right now is not that batch-efficient :) There
is no IO batching, or batched lock operations (we drop the lock
whenever we attempt to writeback a page), etc. Might be a good avenue
to optimize.
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Yosry Ahmed @ 2026-06-04 16:10 UTC (permalink / raw)
To: Hao Jia
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <a60eedb6-f3fd-4092-b726-04a17a695ace@gmail.com>
On Thu, Jun 4, 2026 at 6:06 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>
>
>
> On 2026/6/4 13:34, Yosry Ahmed wrote:
> >>>> For instance, suppose a parent memcg has two children, memcg1 and memcg2,
> >>>> each with 200MB of zswap (100MB inactive). Triggering proactive writeback on
> >>>> the parent memcg will exhaust memcg1's inactive zswap pages. After that,
> >>>> even though memcg2 still has plenty of inactive zswap pages, it will
> >>>> continue to write back memcg1's active zswap pages. Writing back active
> >>>> zswap pages causes the user-space agent to prematurely abort the writeback
> >>>> because it detects that certain memcg metrics have exceeded predefined
> >>>> thresholds.
> >>>
> >>> This will only happen if the reclaim size is smaller than the batch
> >>> size, right? Otherwise the kernel should reclaim more or less equally
> >>> from both memcgs?
> >>>
> >>
> >> I gave it some thought. Not using a cursor could lead to unfairness
> >> issues with certain writeback sizes:
> >>
> >> - If the writeback size is an odd multiple of WB_BATCH (e.g.,
> >> triggering a writeback of 3 * WB_BATCH), with 2 child cgroups, the
> >> writeback ratio might end up being 2:1.
> >> - If a memcg has 5 child cgroups and a writeback of 2 * WB_BATCH is
> >> triggered, it might repeatedly write back from only the first 2 child
> >> cgroups.
> >>
> >> Although setting a smaller WB_BATCH might mitigate this unfairness, it
> >> could hurt writeback efficiency. Let's just use per-memcg cursors to
> >> completely fix these corner cases.
> >
> > Exactly, the batch size should be small enough that any unfairness is
> > not a problem. I would honestly just do batching without a per-memcg
> > cursor, unless we have numbers to prove that the efficiency is
> > affected when we use a small batch size. Let's only introduce
> > complexity when needed please.
>
>
> If you prefer not to use per-cgroup cursors, do we still need to keep
> the global cursor (i.e., the root cgroup's cursor) zswap_next_shrink?
> I found this part to be quite tricky when trying to reuse the main logic
> of shrink_worker() in zswap_proactive_writeback().
>
> Of course, I think we could also keep zswap_next_shrink and write a
> small helper to check if it's the root cgroup, allowing us to use
> different memcg iteration methods.
I think we want to keep the global cursor, at least for now.
^ permalink raw reply
* [PATCH-next v6 6/6] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach()
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260604150229.414135-1-longman@redhat.com>
With cgroup v2, the cgroup_taskset structure passed into the cgroup
can_attach() and attach() methods can contain task migration data with
multiple destination or source cpusets when the cpuset controller is
enabled or disabled respectively.
Since cpuset is threaded in both v1 and v2, another possible way to
cause many-to-one migration is to move the whole process with multiple
threads in different cpuset enabled threaded cgroups into another cpuset
enabled cgroup.
The current cpuset_can_attach() and cpuset_attach() functions still
expect task migration is from one source cpuset to one destination
cpuset. This has been the case since cpuset was enabled for cgroup v2
in commit 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default
hierarchy").
This problem is less an issue when enabling the cpuset controller as all
the newly created child cpusets will have exactly the same set of CPUs
and memory nodes except when deadline tasks are involved in migration
as the deadline task accounting data can be off.
It can be more problematic when the cpuset controller is disabled as
their set of CPUs and memory nodes may differ from their parent or with
the moving of multi-threaded process from different threaded cgroups.
Fix that by tracking the set of source (old) and destination cpusets
in singly linked lists and iterating them all to properly update the
internal data. Also keep the current cs and oldcs variables up-to-date
with the css and task iterators.
This commit assumes that the set of source and destination cpusets
are distnct. IOW, the cgroup core shouldn't move a task from a given
cpuset back to itself. So a given cpuset cannot be both a source and a
destination cpuset. Running an experiment by moving a multithreaded
process with threads in multiple cpusets in threaded cgroups back to
its domain cgroup confirms that only threads that need to be moved
are included into the cgroup_taskset passed to cpuset_can_attach(). A
WARN_ON_ONCE() is added to cpuset_can_attach_check() to make sure that
this should always be the case.
To ensure proper DL tasks accounting, the nr_migrate_dl_tasks in both
the source and destination cpusets are decremented/incremented with
their values added to nr_deadline_tasks when the migration is successful.
Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default hierarchy")
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset-internal.h | 6 +
kernel/cgroup/cpuset.c | 216 ++++++++++++++++++++++++--------
2 files changed, 172 insertions(+), 50 deletions(-)
diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index f7aaf01f7cd5..4c2772a7fd5e 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -161,6 +161,12 @@ struct cpuset {
*/
bool remote_partition;
+ /*
+ * cpuset_can_attach() and cpuset_attach() specific data
+ */
+ bool attach_node_in_llist;
+ struct llist_node attach_node;
+
/*
* number of SCHED_DEADLINE tasks attached to this cpuset, so that we
* know when to rebuild associated root domain bandwidth information.
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 1142d5eba58d..d624cd0a1e04 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -37,6 +37,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/task_work.h>
+#include <linux/llist.h>
DEFINE_STATIC_KEY_FALSE(cpusets_pre_enable_key);
DEFINE_STATIC_KEY_FALSE(cpusets_enabled_key);
@@ -2986,8 +2987,11 @@ static int update_prstate(struct cpuset *cs, int new_prs)
* cpuset_attach() or cpuset_fork() and used in cpuset_attach_task().
*/
static struct cpuset *cpuset_attach_old_cs;
+static LLIST_HEAD(src_cs_head);
+static LLIST_HEAD(dst_cs_head);
static bool attach_cpus_updated;
static bool attach_mems_updated;
+static bool attach_conflict;
/*
* Check to see if a cpuset can accept a new task
@@ -3008,6 +3012,23 @@ static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
if (!oldcs)
return 0;
+ /*
+ * The cgroup core shouldn't migrate a task to its original cpuset.
+ * In the unlikely event that it happens, it will be treated as a
+ * destination cpuset only and the attach_conflict flag will be set.
+ */
+ if (WARN_ON_ONCE(cs == oldcs))
+ attach_conflict = true;
+
+ if (!cs->attach_node_in_llist) {
+ llist_add(&cs->attach_node, &dst_cs_head);
+ cs->attach_node_in_llist = true;
+ }
+ if (!oldcs->attach_node_in_llist) {
+ llist_add(&oldcs->attach_node, &src_cs_head);
+ oldcs->attach_node_in_llist = true;
+ }
+
/*
* Skip rights over task setsched check in v2 when nothing changes,
* migration permission derives from hierarchy ownership in
@@ -3030,33 +3051,101 @@ static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
return 0;
}
-static int cpuset_reserve_dl_bw(struct cpuset *cs)
+/*
+ * If reset_dl_bw is set, reset the previous dl_bw_alloc() call. Otherwise,
+ * update nr_deadline_tasks according to nr_migrate_dl_tasks in both source
+ * and destination cpusets.
+ */
+static void clear_attach_data(bool reset_dl_bw)
{
+ struct cpuset *cs, *next;
+
+ llist_for_each_entry_safe(cs, next, src_cs_head.first, attach_node) {
+ cs->attach_node.next = NULL;
+ cs->attach_node_in_llist = false;
+ if (cs->nr_migrate_dl_tasks && !reset_dl_bw)
+ cs->nr_deadline_tasks += cs->nr_migrate_dl_tasks;
+ cs->nr_migrate_dl_tasks = 0;
+ }
+
+ llist_for_each_entry_safe(cs, next, dst_cs_head.first, attach_node) {
+ cs->attach_node.next = NULL;
+ cs->attach_node_in_llist = false;
+ if (reset_dl_bw && cs->dl_bw_cpu >= 0)
+ dl_bw_free(cs->dl_bw_cpu, cs->sum_migrate_dl_bw);
+ if (cs->nr_migrate_dl_tasks && !reset_dl_bw)
+ cs->nr_deadline_tasks += cs->nr_migrate_dl_tasks;
+ cs->nr_migrate_dl_tasks = 0;
+ cs->sum_migrate_dl_bw = 0;
+ cs->dl_bw_cpu = -1;
+ }
+
+ src_cs_head.first = NULL;
+ dst_cs_head.first = NULL;
+}
+
+static int cpuset_reserve_dl_bw(void)
+{
+ struct cpuset *cs;
int cpu, ret;
- if (!cs->sum_migrate_dl_bw)
- return 0;
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node) {
+ if (!cs->sum_migrate_dl_bw)
+ continue;
- cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
- if (unlikely(cpu >= nr_cpu_ids))
- return -EINVAL;
+ cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
+ if (unlikely(cpu >= nr_cpu_ids))
+ return -EINVAL;
- ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
- if (ret)
- return ret;
+ ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
+ if (ret)
+ return ret;
- cs->dl_bw_cpu = cpu;
+ cs->dl_bw_cpu = cpu;
+ }
return 0;
}
-static void reset_migrate_dl_data(struct cpuset *cs)
+static void set_attach_in_progress(void)
{
- cs->nr_migrate_dl_tasks = 0;
- cs->sum_migrate_dl_bw = 0;
- cs->dl_bw_cpu = -1;
+ struct cpuset *cs;
+
+ /*
+ * Mark attach is in progress. This makes validate_change() fail
+ * changes which zero cpus/mems_allowed.
+ */
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node)
+ cs->attach_in_progress++;
}
-/* Called by cgroups to determine if a cpuset is usable; cpuset_mutex held */
+static void reset_attach_in_progress(void)
+{
+ struct cpuset *cs;
+
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node)
+ dec_attach_in_progress_locked(cs);
+}
+
+/*
+ * Called by cgroups to determine if a cpuset is usable; cpuset_mutex held.
+ *
+ * With cgroup v2, enabling of cpuset controller in a cgroup subtree can
+ * cause @tset to contain task migration data from one parent cpuset to multiple
+ * child cpusets. Not much is needed to be done here other than tracking the
+ * number of DL tasks in each cpuset as the CPUs and memory nodes of the child
+ * cpusets are exactly the same as the parent.
+ *
+ * Conversely, disabling of cpuset controller can cause @tset to contain task
+ * migration data from multiple child cpusets to one parent cpuset. Here, the
+ * CPUs and memory nodes of the child cpusets may be different from the parent,
+ * but must be a subset of its parent.
+ *
+ * Another possible many-to-one migration is the moving of the whole
+ * multithreaded process with threads in different cpusets to another cpuset.
+ *
+ * For all other use cases, @tset task migration data should be from one source
+ * cpuset to one destination cpuset.
+ */
static int cpuset_can_attach(struct cgroup_taskset *tset)
{
struct cgroup_subsys_state *css;
@@ -3069,6 +3158,7 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
cpuset_attach_old_cs = task_cs(cgroup_taskset_first(tset, &css));
oldcs = cpuset_attach_old_cs;
cs = css_cs(css);
+ attach_conflict = false;
mutex_lock(&cpuset_mutex);
@@ -3095,6 +3185,16 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
* selected as cpuset_attach_old_cs.
*/
cgroup_taskset_for_each(task, css, tset) {
+ struct cpuset *newcs = css_cs(css);
+ struct cpuset *new_oldcs = task_cs(task);
+
+ if ((newcs != cs) || (new_oldcs != oldcs)) {
+ cs = newcs;
+ oldcs = new_oldcs;
+ ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
+ if (ret)
+ goto out_unlock;
+ }
ret = task_can_attach(task);
if (ret)
goto out_unlock;
@@ -3116,23 +3216,19 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
* contribute to sum_migrate_dl_bw.
*/
cs->nr_migrate_dl_tasks++;
+ oldcs->nr_migrate_dl_tasks--;
if (dl_task_needs_bw_move(task, cs->effective_cpus))
cs->sum_migrate_dl_bw += task->dl.dl_bw;
}
}
- ret = cpuset_reserve_dl_bw(cs);
+ ret = cpuset_reserve_dl_bw();
out_unlock:
- if (ret) {
- reset_migrate_dl_data(cs);
- } else {
- /*
- * Mark attach is in progress. This makes validate_change() fail
- * changes which zero cpus/mems_allowed.
- */
- cs->attach_in_progress++;
- }
+ if (ret)
+ clear_attach_data(true);
+ else
+ set_attach_in_progress();
mutex_unlock(&cpuset_mutex);
return ret;
@@ -3147,14 +3243,8 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
cs = css_cs(css);
mutex_lock(&cpuset_mutex);
- dec_attach_in_progress_locked(cs);
-
- if (cs->dl_bw_cpu >= 0)
- dl_bw_free(cs->dl_bw_cpu, cs->sum_migrate_dl_bw);
-
- if (cs->nr_migrate_dl_tasks)
- reset_migrate_dl_data(cs);
-
+ reset_attach_in_progress();
+ clear_attach_data(true);
mutex_unlock(&cpuset_mutex);
}
@@ -3227,48 +3317,74 @@ static void cpuset_attach(struct cgroup_taskset *tset)
struct task_struct *task;
struct cgroup_subsys_state *css;
struct cpuset *cs;
- struct cpuset *oldcs = cpuset_attach_old_cs;
+ bool many_sources = src_cs_head.first && src_cs_head.first->next;
cgroup_taskset_first(tset, &css);
- cs = css_cs(css);
lockdep_assert_cpus_held(); /* see cgroup_attach_lock() */
mutex_lock(&cpuset_mutex);
queue_task_work = false;
- attach_cpus_updated = !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus);
- attach_mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
+ /*
+ * attach_cpus_updated/attach_mems_updated can be set to false if
+ * source and destination masks are the same and there is only one
+ * source cpuset with attach_conflict flag unset. IOW, a task migration
+ * with many source cpusets is always treated as updated as the tasks
+ * to old cpuset mapping is lost.
+ */
+ if (many_sources || attach_conflict) {
+ attach_cpus_updated = true;
+ attach_mems_updated = true;
+ } else {
+ /* Only one source cpuset */
+ struct cpuset *oldcs = cpuset_attach_old_cs;
+
+ attach_cpus_updated = false;
+ attach_mems_updated = false;
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node) {
+ attach_cpus_updated |= !cpumask_equal(cs->effective_cpus,
+ oldcs->effective_cpus);
+ attach_mems_updated |= !nodes_equal(cs->effective_mems,
+ oldcs->effective_mems);
+ }
+ }
+ cs = css_cs(css);
/*
* In the default hierarchy, enabling cpuset in the child cgroups
* will trigger a cpuset_attach() call with no change in effective cpus
* and mems. In that case, we can optimize out by skipping the task
- * iteration and update.
+ * iteration and update, but the destination cpuset list is iterated to
+ * set old_mems_sllowed.
*/
if (cpuset_v2()) {
cpuset_attach_nodemask_to = cs->effective_mems;
- if (!attach_cpus_updated && !attach_mems_updated)
+ if (!attach_cpus_updated && !attach_mems_updated) {
+ llist_for_each_entry(cs, dst_cs_head.first, attach_node)
+ cs->old_mems_allowed = cs->effective_mems;
goto out;
+ }
} else {
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
}
- cgroup_taskset_for_each(task, css, tset)
+ cgroup_taskset_for_each(task, css, tset) {
+ struct cpuset *newcs = css_cs(css);
+
+ if (newcs != cs) {
+ cs->old_mems_allowed = cpuset_attach_nodemask_to;
+ cs = newcs;
+ guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
+ }
cpuset_attach_task(cs, task);
+ }
-out:
if (queue_task_work)
schedule_flush_migrate_mm();
cs->old_mems_allowed = cpuset_attach_nodemask_to;
-
- if (cs->nr_migrate_dl_tasks) {
- cs->nr_deadline_tasks += cs->nr_migrate_dl_tasks;
- oldcs->nr_deadline_tasks -= cs->nr_migrate_dl_tasks;
- reset_migrate_dl_data(cs);
- }
-
- dec_attach_in_progress_locked(cs);
-
+out:
+ reset_attach_in_progress();
+ clear_attach_data(false);
mutex_unlock(&cpuset_mutex);
}
--
2.54.0
^ permalink raw reply related
* [PATCH-next v6 5/6] cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside cpuset_attach_task()
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260604150229.414135-1-longman@redhat.com>
The cpuset_attach_task() was introduced in commit 42a11bf5c543
("cgroup/cpuset: Make cpuset_fork() handle CLONE_INTO_CGROUP properly")
to enable the CLONE_INTO_CGROUP flag of clone(2) to behave more like
moving a task from one cpuset into another one. That commits didn't
move the mpol_rebind_mm() and cpuset_migrate_mm() calls for group leader
into cpuset_attach_task().
When the CLONE_INTO_CGROUP flag is used without CLONE_THREAD, the new
task is its own group leader. So it is still not equivalent to moving
task between cpusets in this case. Make CLONE_INTO_CGROUP behaves
more close to cpuset_attach() by moving the mpol_rebind_mm() and
cpuset_migrate_mm() calls inside cpuset_attach_task(). As a result,
the following static variables will have to be updated in cpuset_fork().
- cpuset_attach_old_cs
- attach_cpus_updated
- attach_mems_updated
- queue_task_work
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 103 ++++++++++++++++++++++++-----------------
1 file changed, 60 insertions(+), 43 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e29129467c98..1142d5eba58d 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2981,8 +2981,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
/*
* cpuset_can_attach() and cpuset_attach() specific internal data
* Protected by cpuset_mutex
+ *
+ * The attach_cpus_updated/attach_mems_updated flags are set in either
+ * cpuset_attach() or cpuset_fork() and used in cpuset_attach_task().
*/
static struct cpuset *cpuset_attach_old_cs;
+static bool attach_cpus_updated;
+static bool attach_mems_updated;
/*
* Check to see if a cpuset can accept a new task
@@ -3160,9 +3165,12 @@ static void cpuset_cancel_attach(struct cgroup_taskset *tset)
*/
static cpumask_var_t cpus_attach;
static nodemask_t cpuset_attach_nodemask_to;
+static bool queue_task_work;
static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
{
+ struct mm_struct *mm;
+
lockdep_assert_cpuset_lock_held();
if (cs != &top_cpuset)
@@ -3176,28 +3184,60 @@ static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task)
*/
WARN_ON_ONCE(set_cpus_allowed_ptr(task, cpus_attach));
+ if (cpuset_v2() && !attach_mems_updated)
+ return;
+
cpuset_change_task_nodemask(task, &cpuset_attach_nodemask_to);
cpuset1_update_task_spread_flags(cs, task);
+
+ if ((task != task->group_leader) ||
+ (!is_memory_migrate(cs) && !attach_mems_updated))
+ return;
+
+ /*
+ * Change mm for threadgroup leader. This is expensive and may
+ * sleep and should be moved outside migration path proper.
+ */
+ mm = get_task_mm(task);
+ if (mm) {
+ struct cpuset *oldcs = cpuset_attach_old_cs;
+
+ mpol_rebind_mm(mm, &cs->effective_mems);
+
+ /*
+ * old_mems_allowed is the same with mems_allowed
+ * here, except if this task is being moved
+ * automatically due to hotplug. In that case
+ * @mems_allowed has been updated and is empty, so
+ * @old_mems_allowed is the right nodesets that we
+ * migrate mm from.
+ */
+ if (is_memory_migrate(cs)) {
+ cpuset_migrate_mm(mm, &oldcs->old_mems_allowed,
+ &cpuset_attach_nodemask_to);
+ queue_task_work = true;
+ } else {
+ mmput(mm);
+ }
+ }
}
static void cpuset_attach(struct cgroup_taskset *tset)
{
struct task_struct *task;
- struct task_struct *leader;
struct cgroup_subsys_state *css;
struct cpuset *cs;
struct cpuset *oldcs = cpuset_attach_old_cs;
- bool cpus_updated, mems_updated;
- bool queue_task_work = false;
cgroup_taskset_first(tset, &css);
cs = css_cs(css);
lockdep_assert_cpus_held(); /* see cgroup_attach_lock() */
mutex_lock(&cpuset_mutex);
- cpus_updated = !cpumask_equal(cs->effective_cpus,
- oldcs->effective_cpus);
- mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
+ queue_task_work = false;
+
+ attach_cpus_updated = !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus);
+ attach_mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
/*
* In the default hierarchy, enabling cpuset in the child cgroups
@@ -3207,7 +3247,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
*/
if (cpuset_v2()) {
cpuset_attach_nodemask_to = cs->effective_mems;
- if (!cpus_updated && !mems_updated)
+ if (!attach_cpus_updated && !attach_mems_updated)
goto out;
} else {
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
@@ -3216,38 +3256,6 @@ static void cpuset_attach(struct cgroup_taskset *tset)
cgroup_taskset_for_each(task, css, tset)
cpuset_attach_task(cs, task);
- /*
- * Change mm for all threadgroup leaders. This is expensive and may
- * sleep and should be moved outside migration path proper. Skip it
- * if there is no change in effective_mems and CS_MEMORY_MIGRATE is
- * not set.
- */
- if (!is_memory_migrate(cs) && !mems_updated)
- goto out;
-
- cgroup_taskset_for_each_leader(leader, css, tset) {
- struct mm_struct *mm = get_task_mm(leader);
-
- if (mm) {
- mpol_rebind_mm(mm, &cs->effective_mems);
-
- /*
- * old_mems_allowed is the same with mems_allowed
- * here, except if this task is being moved
- * automatically due to hotplug. In that case
- * @mems_allowed has been updated and is empty, so
- * @old_mems_allowed is the right nodesets that we
- * migrate mm from.
- */
- if (is_memory_migrate(cs)) {
- cpuset_migrate_mm(mm, &oldcs->old_mems_allowed,
- &cpuset_attach_nodemask_to);
- queue_task_work = true;
- } else
- mmput(mm);
- }
- }
-
out:
if (queue_task_work)
schedule_flush_migrate_mm();
@@ -3681,15 +3689,14 @@ static void cpuset_cancel_fork(struct task_struct *task, struct css_set *cset)
*/
static void cpuset_fork(struct task_struct *task)
{
- struct cpuset *cs;
- bool same_cs;
+ struct cpuset *cs, *oldcs;
rcu_read_lock();
cs = task_cs(task);
- same_cs = (cs == task_cs(current));
+ oldcs = task_cs(current);
rcu_read_unlock();
- if (same_cs) {
+ if (cs == oldcs) {
if (cs == &top_cpuset)
return;
@@ -3701,7 +3708,17 @@ static void cpuset_fork(struct task_struct *task)
/* CLONE_INTO_CGROUP */
mutex_lock(&cpuset_mutex);
guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
+ /*
+ * Assume CPUs and memory nodes are updated
+ * A CLONE_INTO_CGROUP operation should have taken the cgroup mutex
+ * and so there shouldn't be a competing cpuset_attach() operation.
+ */
+ attach_cpus_updated = attach_mems_updated = true;
+ queue_task_work = false;
+ cpuset_attach_old_cs = oldcs;
cpuset_attach_task(cs, task);
+ if (queue_task_work)
+ schedule_flush_migrate_mm();
dec_attach_in_progress_locked(cs);
mutex_unlock(&cpuset_mutex);
--
2.54.0
^ permalink raw reply related
* [PATCH-next v6 4/6] cgroup/cpuset: Make cpuset_attach_old_cs track task group leaders
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260604150229.414135-1-longman@redhat.com>
There are two possible ways that migration of tasks from multiple source
cpusets to a target cpuset can happen. Either a multithread application
with threads in different cpusets is wholely moved to a new cpuset
or disabling of v2 cpuset controller will move all the tasks in child
cpusets to the parent cpuset.
In the former case, it is the mm setting of the group leader that really
matters. So cpuset_attach_old_cs should track the oldcs of the thread
leader. In the latter case, effective_mems of child cpusets must always
be a subset of the parent. So no real page migration will be necessary
no matter which child cpuset is selected as cpuset_attach_old_cs.
IOW, cpuset_attach_old_cs should be updated to match the latest task
group leader in cpuset_can_attach(), but fall back to that of the first
task if there is no group leader in the taskset.
Suggested-by: Ridong Chen <ridong.chen@linux.dev>
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 90fb40760dcc..e29129467c98 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2978,6 +2978,10 @@ static int update_prstate(struct cpuset *cs, int new_prs)
return 0;
}
+/*
+ * cpuset_can_attach() and cpuset_attach() specific internal data
+ * Protected by cpuset_mutex
+ */
static struct cpuset *cpuset_attach_old_cs;
/*
@@ -3068,11 +3072,32 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
if (ret)
goto out_unlock;
+ /*
+ * The cpuset_attach_old_cs is used mainly by cpuset_migrate_mm() to get
+ * the old_mems_allowed value. There are two ways that many-to-one
+ * cpuset migration can happen:
+ * 1) A multithread application with threads in different cpusets is
+ * wholely migrated to a new cpuset.
+ * 2) Disabling v2 cpuset controller will move all the tasks in child
+ * cpusets to the parent cpuset.
+ *
+ * In the former case, it is the mm setting of the group leader that
+ * really matters. So cpuset_attach_old_cs should track the oldcs of the
+ * group leader. It falls back to the oldcs of the first task if there
+ * is no group leader in the taskset. In the latter case, effective_mems
+ * of child cpusets must always be a subset of the parent. So no real
+ * page migration will be necessary no matter which child cpuset is
+ * selected as cpuset_attach_old_cs.
+ */
cgroup_taskset_for_each(task, css, tset) {
ret = task_can_attach(task);
if (ret)
goto out_unlock;
+ /* Update cpuset_attach_old_cs to the latest group leader */
+ if (task == task->group_leader)
+ cpuset_attach_old_cs = task_cs(task);
+
if (setsched_check) {
ret = security_task_setscheduler(task);
if (ret)
--
2.54.0
^ permalink raw reply related
* [PATCH-next v6 3/6] cgroup/cpuset: Expand the scope of cpuset_can_attach_check()
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260604150229.414135-1-longman@redhat.com>
Expand the scope of cpuset_can_attach_check() by including the setting
of setsched flag inside cpuset_can_attach_check() with the new @oldcs
and @psetsched argument. As cpuset_can_attach_check() is also called
from cpuset_can_fork(), set the new arguments to NULL from that caller.
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 52 ++++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 7c23d26a04fc..90fb40760dcc 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2985,12 +2985,39 @@ static struct cpuset *cpuset_attach_old_cs;
* For v1, cpus_allowed and mems_allowed can't be empty.
* For v2, effective_cpus can't be empty.
* Note that in v1, effective_cpus = cpus_allowed.
+ *
+ * Also set the boolean flag passed in by @psetsched depending on if
+ * security_task_setscheduler() call is needed and @oldcs is not NULL.
*/
-static int cpuset_can_attach_check(struct cpuset *cs)
+static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
+ bool *psetsched)
{
if (cpumask_empty(cs->effective_cpus) ||
(!is_in_v2_mode() && nodes_empty(cs->mems_allowed)))
return -ENOSPC;
+
+ if (!oldcs)
+ return 0;
+
+ /*
+ * Skip rights over task setsched check in v2 when nothing changes,
+ * migration permission derives from hierarchy ownership in
+ * cgroup_procs_write_permission()).
+ */
+ *psetsched = !cpuset_v2() ||
+ !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
+ !nodes_equal(cs->effective_mems, oldcs->effective_mems);
+
+ /*
+ * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
+ * brings the last online CPU offline as users are not allowed to empty
+ * cpuset.cpus when there are active tasks inside. When that happens,
+ * we should allow tasks to migrate out without security check to make
+ * sure they will be able to run after migration.
+ */
+ if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
+ *psetsched = false;
+
return 0;
}
@@ -3037,29 +3064,10 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
mutex_lock(&cpuset_mutex);
/* Check to see if task is allowed in the cpuset */
- ret = cpuset_can_attach_check(cs);
+ ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
if (ret)
goto out_unlock;
- /*
- * Skip rights over task setsched check in v2 when nothing changes,
- * migration permission derives from hierarchy ownership in
- * cgroup_procs_write_permission()).
- */
- setsched_check = !cpuset_v2() ||
- !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
- !nodes_equal(cs->effective_mems, oldcs->effective_mems);
-
- /*
- * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
- * brings the last online CPU offline as users are not allowed to empty
- * cpuset.cpus when there are active tasks inside. When that happens,
- * we should allow tasks to migrate out without security check to make
- * sure they will be able to run after migration.
- */
- if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
- setsched_check = false;
-
cgroup_taskset_for_each(task, css, tset) {
ret = task_can_attach(task);
if (ret)
@@ -3604,7 +3612,7 @@ static int cpuset_can_fork(struct task_struct *task, struct css_set *cset)
mutex_lock(&cpuset_mutex);
/* Check to see if task is allowed in the cpuset */
- ret = cpuset_can_attach_check(cs);
+ ret = cpuset_can_attach_check(cs, NULL, NULL);
if (ret)
goto out_unlock;
--
2.54.0
^ permalink raw reply related
* [PATCH-next v6 2/6] cgroup/cpuset: Add a cpuset_reserve_dl_bw() helper
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260604150229.414135-1-longman@redhat.com>
Extract the DL bandwidth allocation code in cpuset_attach() to a new
cpuset_reserve_dl_bw() helper to simplify code.
No functional change is expected.
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 53 ++++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 8305b5830c3c..7c23d26a04fc 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2994,6 +2994,25 @@ static int cpuset_can_attach_check(struct cpuset *cs)
return 0;
}
+static int cpuset_reserve_dl_bw(struct cpuset *cs)
+{
+ int cpu, ret;
+
+ if (!cs->sum_migrate_dl_bw)
+ return 0;
+
+ cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
+ if (unlikely(cpu >= nr_cpu_ids))
+ return -EINVAL;
+
+ ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
+ if (ret)
+ return ret;
+
+ cs->dl_bw_cpu = cpu;
+ return 0;
+}
+
static void reset_migrate_dl_data(struct cpuset *cs)
{
cs->nr_migrate_dl_tasks = 0;
@@ -3008,7 +3027,7 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
struct cpuset *cs, *oldcs;
struct task_struct *task;
bool setsched_check;
- int cpu, ret;
+ int ret;
/* used later by cpuset_attach() */
cpuset_attach_old_cs = task_cs(cgroup_taskset_first(tset, &css));
@@ -3064,31 +3083,19 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
}
}
- if (!cs->sum_migrate_dl_bw)
- goto out_success;
-
- cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
- if (unlikely(cpu >= nr_cpu_ids)) {
- ret = -EINVAL;
- goto out_unlock;
- }
-
- ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
- if (ret)
- goto out_unlock;
-
- cs->dl_bw_cpu = cpu;
-
-out_success:
- /*
- * Mark attach is in progress. This makes validate_change() fail
- * changes which zero cpus/mems_allowed.
- */
- cs->attach_in_progress++;
+ ret = cpuset_reserve_dl_bw(cs);
out_unlock:
- if (ret)
+ if (ret) {
reset_migrate_dl_data(cs);
+ } else {
+ /*
+ * Mark attach is in progress. This makes validate_change() fail
+ * changes which zero cpus/mems_allowed.
+ */
+ cs->attach_in_progress++;
+ }
+
mutex_unlock(&cpuset_mutex);
return ret;
}
--
2.54.0
^ permalink raw reply related
* [PATCH-next v6 1/6] cgroup/cpuset: Fix node inconsistencies between cpuset_update_tasks_nodemask() and cpuset_attach()
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
In-Reply-To: <20260604150229.414135-1-longman@redhat.com>
Whenever memory node mask is changed, there are 4 places where the node
mask has to be updated or used.
1) task's node mask via cpuset_change_task_nodemask()
2) memory policy binding via mpol_rebind_mm()
3) if memory migration is enabled, migrate from old_mems_allowed to
the new node mask via cpuset_migrate_mm().
4) setting old_mems_allowed
These memory actions are done in cpuset_update_tasks_nodemask() and
cpuset_attach(). However there are inconsistencies in what node masks
are being used in these 2 functions.
In cpuset_update_tasks_nodemask(),
- cpuset_change_task_nodemask(): guarantee_online_mems()
- mpol_rebind_mm(): mems_allowed
- cpuset_migrate_mm(): guarantee_online_mems()
- old_mems_allowed: guarantee_online_mems()
In cpuset_attach(),
- cpuset_change_task_nodemask(): guarantee_online_mems()
- mpol_rebind_mm(): effective_mems
- cpuset_migrate_mm(): effective_mems
- old_mems_allowed: effective_mems
These inconsistencies dates back to quite a long time ago and it is
hard to say what should be the correct values.
The guarantee_online_mems() function returns a node mask from current or
an ancestor cpuset that is a subset of node_states[N_MEMORY]. Nodes in
node_states[N_MEMORY] are all online, i.e. in node_states[N_ONLINE].
However, node in node_states[N_ONLINE] may not have memory. So
node_states[N_MEMORY] should be a subset of node_states[N_ONLINE].
The guarantee_online_mems() function should only be useful for v1 where
mems_allowed is the same as effective_mems. With v2, the memory nodes
in effective_mems should always be a subset of node_states[N_MEMORY].
The only time that may not be true is when a memory hot-unplug operation
is in progress and a memory node is removed from node_states[N_MEMORY]
but not yet reflected in effective_mems as cpuset_handle_hotplug()
has not yet been called from cpuset_track_online_nodes(). When
cpuset_handle_hotplug() is called later, the memory node setting
of the relevant cpusets and tasks will be updated. So replacing the
guarantee_online_mems() call by just using cs->effective_mems should
be fine.
Let use the following setup for both of them and make them consistent.
- cpuset_change_task_nodemask(): guarantee_online_mems()
- mpol_rebind_mm(): effective_mems
- cpuset_migrate_mm(): guarantee_online_mems()
- old_mems_allowed: guarantee_online_mems()
So for v2, it is effectively all effective_mems. For v1, mpol_rebind_mm()
uses mems_allowed which may differ from what guarantee_online_mems()
returns.
Signed-off-by: Waiman Long <longman@redhat.com>
---
kernel/cgroup/cpuset.c | 37 +++++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 6bdb68689c24..8305b5830c3c 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -489,7 +489,10 @@ static void guarantee_active_cpus(struct task_struct *tsk,
* Return in *pmask the portion of a cpusets's mems_allowed that
* are online, with memory. If none are online with memory, walk
* up the cpuset hierarchy until we find one that does have some
- * online mems. The top cpuset always has some mems online.
+ * online mems. The top cpuset always has some mems online. With v2,
+ * effective_mems should always contain online memory nodes except
+ * during the transition period where a memory node hotunplug operation
+ * is in progress.
*
* One way or another, we guarantee to return some non-empty subset
* of node_states[N_MEMORY].
@@ -498,6 +501,10 @@ static void guarantee_active_cpus(struct task_struct *tsk,
*/
static void guarantee_online_mems(struct cpuset *cs, nodemask_t *pmask)
{
+ if (cpuset_v2()) {
+ *pmask = cs->effective_mems;
+ return;
+ }
while (!nodes_and(*pmask, cs->effective_mems, node_states[N_MEMORY]))
cs = parent_cs(cs);
}
@@ -2616,6 +2623,13 @@ static void *cpuset_being_rebound;
* Iterate through each task of @cs updating its mems_allowed to the
* effective cpuset's. As this function is called with cpuset_mutex held,
* cpuset membership stays stable.
+ *
+ * - cpuset_change_task_nodemask(): guarantee_online_mems()
+ * - mpol_rebind_mm(): effective_mems
+ * - cpuset_migrate_mm(): guarantee_online_mems()
+ * - old_mems_allowed: guarantee_online_mems()
+ *
+ * For v2, guarantee_online_mems() should just return effective_mems.
*/
void cpuset_update_tasks_nodemask(struct cpuset *cs)
{
@@ -2624,7 +2638,6 @@ void cpuset_update_tasks_nodemask(struct cpuset *cs)
struct task_struct *task;
cpuset_being_rebound = cs; /* causes mpol_dup() rebind */
-
guarantee_online_mems(cs, &newmems);
/*
@@ -2650,7 +2663,7 @@ void cpuset_update_tasks_nodemask(struct cpuset *cs)
migrate = is_memory_migrate(cs);
- mpol_rebind_mm(mm, &cs->mems_allowed);
+ mpol_rebind_mm(mm, &cs->effective_mems);
if (migrate)
cpuset_migrate_mm(mm, &cs->old_mems_allowed, &newmems);
else
@@ -3148,17 +3161,18 @@ static void cpuset_attach(struct cgroup_taskset *tset)
/*
* In the default hierarchy, enabling cpuset in the child cgroups
- * will trigger a number of cpuset_attach() calls with no change
- * in effective cpus and mems. In that case, we can optimize out
- * by skipping the task iteration and update.
+ * will trigger a cpuset_attach() call with no change in effective cpus
+ * and mems. In that case, we can optimize out by skipping the task
+ * iteration and update.
*/
- if (cpuset_v2() && !cpus_updated && !mems_updated) {
+ if (cpuset_v2()) {
cpuset_attach_nodemask_to = cs->effective_mems;
- goto out;
+ if (!cpus_updated && !mems_updated)
+ goto out;
+ } else {
+ guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
}
- guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
-
cgroup_taskset_for_each(task, css, tset)
cpuset_attach_task(cs, task);
@@ -3168,7 +3182,6 @@ static void cpuset_attach(struct cgroup_taskset *tset)
* if there is no change in effective_mems and CS_MEMORY_MIGRATE is
* not set.
*/
- cpuset_attach_nodemask_to = cs->effective_mems;
if (!is_memory_migrate(cs) && !mems_updated)
goto out;
@@ -3176,7 +3189,7 @@ static void cpuset_attach(struct cgroup_taskset *tset)
struct mm_struct *mm = get_task_mm(leader);
if (mm) {
- mpol_rebind_mm(mm, &cpuset_attach_nodemask_to);
+ mpol_rebind_mm(mm, &cs->effective_mems);
/*
* old_mems_allowed is the same with mems_allowed
--
2.54.0
^ permalink raw reply related
* [PATCH-next v6 0/6] cgroup/cpuset: Support multiple source/destination cpusets for cpuset_*attach()
From: Waiman Long @ 2026-06-04 15:02 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang, Waiman Long
v6:
- Make guarantee_online_mems() to only return cs->effective_mems with v2
in patch 1.
- Remove obsolete commit description text from patch 3.
- Add Reviewed-by tags.
- In patch 6, add WARN_ON_ONCE() test in cpuset_can_attach() to
confirm that cs != oldcs.
v5:
- Remove the WARN_ON() call as it can be triggered in a corner case.
- Instead of passing an attach_cpus_updated and attach_mems_updated
flags from cpuset_can_attach() to cpuset_attach(), re-evaluate the
flags at the beginning of cpuset_attach() based on data in the source &
destination cpusets in the singly linked lists to eliminate the
Time-of-Check to Time-of-Use (TOCTOU) race condition & simplify the
code changes.
- Add back the dropped optimization in patch 5.
v4:
- Add a new patch 1 to fix inconsistency in node mask usage in
cpuset_update_tasks_nodemask() and cpuset_attach() and adjust
the subsequent patches accordingly.
- Update patch 3 to set the update flags whenever the CPU or node
mask is updated to address issue reported by Sashiko.
- Update patch 5 to remove unneeded setting of old_mems_allowed as
well as calling schedule_flush_migrate_mm() if queue_task_work is
set.
Sashiko AI review of another cpuset patch had found that cpuset_attach()
and cpuset_can_attach() can be passed a cgroup_taskset with tasks
migrating from one source cpuset to multiple destination cpusets and
vice versa. Further testing of the cpuset code indicates that this is
indeed the case when the v2 cpuset controller is enabled or disabled.
Unfortunately, cpuset_attach() and cpuset_can_attach() still assume that
there will be one source and one destinaton cpuset which may result in
inocrrect behavior.
This patch series is created to fix this issue.
Patch 1 is to fix an inconsistency in the way node mask update is being
handled in cpuset_update_tasks_nodemask() and cpuset_attach() so that
they match each other.
Patches 2 and 3 are just preparatory patches to make the remaining
patches easier to review.
Patch 4 makes cpuset_attach_old_cs to track group leader for use by
cpuset_migrate_mm().
Patch 5 moves mpol_rebind_mm() and cpuset_migrate_mm() inside
cpuset_attach_task() to make CLONE_INTO_CGROUP flag of clone(2) works
more like moving task from one cpuset to another one, while also make
supporting multiple source and destination cpusets easier.
Patch 6 makes the necessary changes to enable the support of multiple
source and destination cpusets by keeping all the source and destination
cpusets found during task iterations in two singly linked lists for
source and destination cpusets respectively.
Waiman Long (6):
cgroup/cpuset: Fix node inconsistencies between
cpuset_update_tasks_nodemask() and cpuset_attach()
cgroup/cpuset: Add a cpuset_reserve_dl_bw() helper
cgroup/cpuset: Expand the scope of cpuset_can_attach_check()
cgroup/cpuset: Make cpuset_attach_old_cs track task group leaders
cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside
cpuset_attach_task()
cgroup/cpuset: Support multiple source/destination cpusets for
cpuset_*attach()
kernel/cgroup/cpuset-internal.h | 6 +
kernel/cgroup/cpuset.c | 424 +++++++++++++++++++++++---------
2 files changed, 311 insertions(+), 119 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH v3 2/4] mm/zswap: Implement proactive writeback
From: Shakeel Butt @ 2026-06-04 14:01 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Hao Jia, Johannes Weiner, mhocko, tj, mkoutny, roman.gushchin,
Nhat Pham, akpm, chengming.zhou, muchun.song, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <CAO9r8zM4SDdTgz9L2s1VfXL8K2VBjMD9ej2BTDxaGge1t2+quA@mail.gmail.com>
On Wed, Jun 03, 2026 at 10:36:07PM -0700, Yosry Ahmed wrote:
> > >> But doesn't it make more sense to specify the compressed size, which is
> > >> ultimately the amount of memory you actually want to reclaim.
> > >>
> > >
> > > I personally prefer compressed size to pre-compressed size. That's
> > > kinda what user cares about, no?
> > >
> > > One thing we can do is let users prescribe a compressed size, but
> > > internally, we can multiply that by the average compression ratio.
> > > That gives us a guesstimate of how many pages we need to reclaim, and
> > > you can follow the rest of your implementation as is (perhaps with
> > > short-circuit when we reach the goal with fewer pages reclaimed).
> >
> > Got it. I will change it to use the compressed size in the next version.
> >
> > Yosry, Nhat, should we continue using the zswap_writeback_only key to
> > trigger proactive writeback?
>
> I *really* want the memcg maintainers to chime in here, it's
> ultimately their call.
>
> Michal? Johannes? Shakeel? Roman? Anyone? :D
Sorry for the delay, I will take a look in a day or two.
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Hao Jia @ 2026-06-04 13:06 UTC (permalink / raw)
To: Yosry Ahmed
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <CAO9r8zPBH6-0SQ6-_ZOhTQeyu=rz4F=ugikCrU-JR_skm6fEWA@mail.gmail.com>
On 2026/6/4 13:34, Yosry Ahmed wrote:
>>>> For instance, suppose a parent memcg has two children, memcg1 and memcg2,
>>>> each with 200MB of zswap (100MB inactive). Triggering proactive writeback on
>>>> the parent memcg will exhaust memcg1's inactive zswap pages. After that,
>>>> even though memcg2 still has plenty of inactive zswap pages, it will
>>>> continue to write back memcg1's active zswap pages. Writing back active
>>>> zswap pages causes the user-space agent to prematurely abort the writeback
>>>> because it detects that certain memcg metrics have exceeded predefined
>>>> thresholds.
>>>
>>> This will only happen if the reclaim size is smaller than the batch
>>> size, right? Otherwise the kernel should reclaim more or less equally
>>> from both memcgs?
>>>
>>
>> I gave it some thought. Not using a cursor could lead to unfairness
>> issues with certain writeback sizes:
>>
>> - If the writeback size is an odd multiple of WB_BATCH (e.g.,
>> triggering a writeback of 3 * WB_BATCH), with 2 child cgroups, the
>> writeback ratio might end up being 2:1.
>> - If a memcg has 5 child cgroups and a writeback of 2 * WB_BATCH is
>> triggered, it might repeatedly write back from only the first 2 child
>> cgroups.
>>
>> Although setting a smaller WB_BATCH might mitigate this unfairness, it
>> could hurt writeback efficiency. Let's just use per-memcg cursors to
>> completely fix these corner cases.
>
> Exactly, the batch size should be small enough that any unfairness is
> not a problem. I would honestly just do batching without a per-memcg
> cursor, unless we have numbers to prove that the efficiency is
> affected when we use a small batch size. Let's only introduce
> complexity when needed please.
If you prefer not to use per-cgroup cursors, do we still need to keep
the global cursor (i.e., the root cgroup's cursor) zswap_next_shrink?
I found this part to be quite tricky when trying to reuse the main logic
of shrink_worker() in zswap_proactive_writeback().
Of course, I think we could also keep zswap_next_shrink and write a
small helper to check if it's the root cgroup, allowing us to use
different memcg iteration methods.
Thanks，
Hao
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-06-04 12:18 UTC (permalink / raw)
To: Balbir Singh
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, apopple, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <aiFSZfRlFPd7qlIw@parvat>
On Thu, Jun 04, 2026 at 08:35:19PM +1000, Balbir Singh wrote:
>
> My concern is that __GFP_PRIVATE is too wide, I wonder if we'll have a
> need to support N_MEMORY_PRIVATE may not be all homogeneous memory nodes.
> Very similar to how not all ZONE_DEVICE memory is homogenous.
>
Can you more precise about your definition of homogeneous here?
Are you saying not all memory on a private node will be homogeneous?
While possible, I would argue that you should not do this and
should instead prefer to use multiple nodes - 1 per memory class.
Are you saying not all private nodes will be homogenous?
I don't see the issue with this.
> >
> > Agreed, but also one which can be deferred and played with since it's
> > all kernel-internal. None of this should have UAPI implications, and we
> > need need to accept that we're going to get it wrong on the first try.
> >
>
> Agreed that we might get the design wrong, until we fix it up. I feel
> that __GFP_PRIVATE should be an evolution of the design to that point.
>
Possibly. If we can't guarantee isolation without __GFP_PRIVATE, then
we probably can't merge the baseline without it.
> > Because pagecache pages are associated with potentially many VMAs.
> >
> > The fault can be a soft fault or a hard fault. On soft fault - the page
> > was already present, and will simply fault into VMA without being
> > migrated.
> >
>
> Let's split this into two:
>
> 1. unmapped page cache is never impacted by mempolicy and should not
> end up on private memory nodes
> 2. For shared pages, mempolicy would be hard, but it would need to
> be on a set of nodes backed by private memory, depending on mbind()
> policy
>
... snip ...
>
> I'd need to think more about this. For now, my basic requirement would
> be that unmapped page cache should not come from/to private nodes.
>
This does not fully describe the problem.
A file can be opened and cached as unmapped page cache, and then mapped
at a later time - at which point the mapped copy would share the filemap
page cache page.
Worse, because it's file-backed, you can have the memory faulted onto
your remote node - reclaimed - and the faulted back in via the process
accessing the file via unmapped operations (read/write), at which point
you've had a silent migration occur.
Basically consider
Process A:
fd = open("myfile", ..., RO);
read(fd, ...); /* mm/filemap.c fills page cache */
Process B:
fd = open("myfile", ...);
mem = mmap(fd, ...);
mbind(mem, ..., private_node);
for page in mem:
int tmp = mem[page]; /* fault into vma */
The result of Process A running first is Process B thinks it has faulted
the memory onto private_node, but in reality it's taking soft faults and
just getting the filemap folio mapped in.
If you wanted mbind() support from the start, we would have to limit
applicability to anon memory only.
Shared anon memory is different, as there is a radix tree that deals
with a shared mempolicy state.
>
> I am open to this, I was coming from the blueprint approach of:
> - Let's mimic N_MEMORY with N_MEMORY_PRIVATE and then pick and choose
> what features to change or make specific to the implementation
>
N_MEMORY essentially states:
"This is normal memory touch it however you like"
N_MEMORY_PRIVATE (_MANAGED, w/e) says
"This is NOT normal memory, there are special rules here"
So, no, lets not mimic N_MEMORY. This is a "closed by default" design,
while N_MEMORY is an "open by default" design. This design choice is
explicit to make reasoning about these nodes feasible.
> > This is informed by a single use case / device.
> >
> > There are users / devices that don't want any UAPI for their memory,
> > but simply wish to re-utilize some subsection of mm/ (page_alloc,
> > reclaim, etc).
> >
>
> But then, why do they need NUMA nodes? Do we have a list of use cases?
>
So far i have collected:
- Network accelerators carrying their own memory for message buffers
- GPUs with semi-general-purpose working memory across coherent links
- Acceptionally slow distributed memory that you do not want fallback
allocations to (so you want to deliberately tier what lands there)
- Compressed memory (just another form of accelerator really) which
has *special access rules* (i.e. writes need to be controlled)
In most if not all of these cases, the right abstraction to reason about
where memory *should come from* IS a NUMA node.
- the network stack can be taught to check if the target device has a
node with memory and prefer that node over local memory
- accelerators can be given private nodes to manage memory using
core mm/ components, without worrying that general kernel operation
will put unrelated memory on those nodes or do things like migrate
your pages out from under you (unless your driver/service requested
that).
the tiering application should be somewhat obvious / trivial.
> >
> > I am trying to test whether, lacking __GFP_PRIVATE, any normal runtime
> > operations access private nodes removed from fallback lists are reached
> > via something like the possible / online nodemask.
> >
> > I remember, maybe a year ago, there were per-node allocations happening
> > during hotplug and that's why I originally proposed __GFP_PRIVATE, but
> > I'm trying to re-collect that data now.
> >
>
> Thanks, I look forward to the next set of patches. Let me know if I
> can help test what's on the list or if you want me to wait for the next
> round
>
Really I want to get the minimized set out the door so we can start
breaking this up by feature (reclaim, mempolicy, etc), because trying to
reason about it as a whole is infeasible - and I cannot be the single
arbiter of every use case (I simply do not have sufficient context).
I'm reworking it all as we speak.
~Gregory
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Balbir Singh @ 2026-06-04 10:35 UTC (permalink / raw)
To: Gregory Price
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, apopple, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <aiE5DZC8Io4SNI3H@gourry-fedora-PF4VCD3F>
On Thu, Jun 04, 2026 at 09:36:29AM +0100, Gregory Price wrote:
> On Thu, Jun 04, 2026 at 11:43:14AM +1000, Balbir Singh wrote:
> > On Wed, Jun 03, 2026 at 08:02:09AM +0100, Gregory Price wrote:
> > >
> > > Here is how the page allocator fallback lists and nodemasks interact:
> > >
> > > Fallbacks A: A B
> > > Fallbacks B: B A
> > > Fallbacks C: C A B (Private)
> > > Fallbacks D: D B A (Private)
> > >
> >
> > Do we want regular memory (N_MEMORY) in the fallback list of device private nodes?
> > The assumption is that we have ATS translation enabled? Assumiung A and
> > B are N_MEMORY here or am I misreading your illustraion?
> >
>
> If we don't have __GFP_PRIVATE, then probably not. This is a holdover
> from the current __GFP_PRIVATE branch so that if the preferred_nid=
> value is a private node (which is a hint, but not a hard control),
> there's a way for that allocation to land *somewhere*.
>
> __GFP_PRIVATE would say "Only allow access to private nodes if this
> flag is provided - otherwise treat that as unreachable and fall back".
>
> (__GFP_PRIVATE | __GFP_THISNODE) then does exactly what you expect (only
> allocate from specifically this private node and don't fall back).
>
> This has the added benefit of not causing OOM on allocation failure.
>
> Some would consider such a request a bug (i.e. that caller has a bad
> mask), but I find the premise of that statement to be flawwed if only
> because we do not have good controls over what ends up in a nodemask due
> to the existence of things like possible_nodes.
>
My concern is that __GFP_PRIVATE is too wide, I wonder if we'll have a
need to support N_MEMORY_PRIVATE may not be all homogeneous memory nodes.
Very similar to how not all ZONE_DEVICE memory is homogenous.
>
> > > If we wanted to change this behavior, realistically we'd be looking for
> > > a way to add specific nodes to certain fallback lists - rather than
> > > modify the nodemask interaction in some way.
> >
> > Yes, that is what we did with CDM, control the fallback for
> > N_MEMORY_PRIVATE, but there is a design decision to be made here.
> >
>
> Agreed, but also one which can be deferred and played with since it's
> all kernel-internal. None of this should have UAPI implications, and we
> need need to accept that we're going to get it wrong on the first try.
>
Agreed that we might get the design wrong, until we fix it up. I feel
that __GFP_PRIVATE should be an evolution of the design to that point.
> > > 2) full mempolicy support doesn't really make sense
> > >
> > > task mempolicy PROBABLY should never really touch private nodes,
> > > while VMA policy certainly can. Assuming we're able to support
> > > multi-private-node masks, none of the non-bind mempolicies even
> > > make sense for most private nodes (interleave? weighted interleave?)
> > >
> >
> > Yes, mostly, but is that baked into the design? If so, why?
> >
>
> "Baked in" in this case would mean:
>
> set_mempolicy(..., private_node) -> -EINVAL
> mbind(..., private_node) -> Success
>
> With appropriate documentation.
>
> This can be changed later if a reasonable design was agreed upon.
>
> > > 4) File VMA interactions don't entirely make sense with mbind
> > >
> > > In theory you might want:
> > >
> > > fd = open("somefile", ...);
> > > mem = mmap(fd, ...);
> > > mbind(mem, ..., private_node);
> > > for page in mem:
> > > mem[page_off] /* fault file into private memory */
> > >
> > > In reality: This does not work the way you want.
> >
> > Why not? Just curious about what you found?
> >
>
> Because pagecache pages are associated with potentially many VMAs.
>
> The fault can be a soft fault or a hard fault. On soft fault - the page
> was already present, and will simply fault into VMA without being
> migrated.
>
Let's split this into two:
1. unmapped page cache is never impacted by mempolicy and should not
end up on private memory nodes
2. For shared pages, mempolicy would be hard, but it would need to
be on a set of nodes backed by private memory, depending on mbind()
policy
> You can imagine the following
>
> Process A:
> fd = open("somefile", ...);
> mem = mmap(fd, ...);
> mbind(mem, ..., private_node_A);
> for page in mem:
> mem[page_off] /* fault file into private memory */
>
> Process B:
> fd = open("somefile", ...);
> mem = mmap(fd, ...);
> mbind(mem, ..., private_node_B);
> for page in mem:
> mem[page_off] /* fault file into private memory */
>
> If process A runs first, and assuming VMA mempolicy is respected for
> file backed allocation (note: it's not, see below) - then the second
> process will think the memory now lives on node B when it's already
> living on node A (pages are not migrated on fault).
>
> filemap page cache means file-backed pages are global resources.
>
> Re file-backed VMAs - see filemap_alloc_folio_noprof in mm/filemap.c
>
> struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order)
> {
> int n;
> struct folio *folio;
>
> if (cpuset_do_page_mem_spread()) {
> unsigned int cpuset_mems_cookie;
> do {
> cpuset_mems_cookie = read_mems_allowed_begin();
> n = cpuset_mem_spread_node();
> folio = __folio_alloc_node_noprof(gfp, order, n);
> } while (!folio && read_mems_allowed_retry(cpuset_mems_cookie));
>
> return folio;
> }
> return folio_alloc_noprof(gfp, order);
> }
>
> We'd have to hang a mempolicy off of the file and use fctl or something
> like this if we want a file to have a node preference.
I'd need to think more about this. For now, my basic requirement would
be that unmapped page cache should not come from/to private nodes.
>
> > >
> > > I went digging and we need a few mild extensions to allow
> > > migration on mbind to work for pagecache pages, and the fault
> > > path does not necessarily respect the vma mempolicy always.
> > >
> > > You also start getting into the question of "what happens when
> > > the node is out of memory and you don't have reclaim support?".
> >
> > Yes, we should discuss reclaim support, I think we should allow for
> > reclaim. It allows you to overcommit private memory the way we can
> > with regular memory.
> >
>
> Reclaim support is feasible, but again - crawl, walk, run.
>
> If we get the base private node infrastructure in place, we can break
> things like mempolicy and reclaim support into different work streams
> to enable support for these features.
>
> Different private node users will be interested in different
> combinations of mm/ service support.
>
> For example: compressed memory as a swap backend DOES NOT want explicit
> reclaim support - it will need to manage its own shrinker. This comes
> from requirements associated with that specific use case (which I do not
> want to get into here).
>
> That is why this series introduced the concept of NP_OPS_* - so that the
> owner (driver) of a private node (such as a CXL-enabled accelerator
> driver) can tell mm/ what services it should enable for that node.
I am open to this, I was coming from the blueprint approach of:
- Let's mimic N_MEMORY with N_MEMORY_PRIVATE and then pick and choose
what features to change or make specific to the implementation
>
> > >
> > > For all these reasons, I think the be mbind/mempolicy support with
> > > private nodes needs to be brought in with follow up work - not
> > > introduced as part of the baseline set.
> > >
> >
> > I am not opposed to the follow up work, but I feel mbind() should
> > be the fundamental work and user space API.
> >
>
> This is informed by a single use case / device.
>
> There are users / devices that don't want any UAPI for their memory,
> but simply wish to re-utilize some subsection of mm/ (page_alloc,
> reclaim, etc).
>
But then, why do they need NUMA nodes? Do we have a list of use cases?
> > >
> > > I am arguing for #1 - the community has argued for #2 and "fixing
> > > existing nodemask users". I think we can ship #2 and pivot to #1 if we
> > > find fixing existing users is infeasible or too much of a maintenance
> > > burden.
> >
> > Again happy to discuss this, I'd like to make sure we agree on the
> > design. I am wondering if there is any experimental data to choose
> > between 1 and 2.
> >
>
> I am trying to test whether, lacking __GFP_PRIVATE, any normal runtime
> operations access private nodes removed from fallback lists are reached
> via something like the possible / online nodemask.
>
> I remember, maybe a year ago, there were per-node allocations happening
> during hotplug and that's why I originally proposed __GFP_PRIVATE, but
> I'm trying to re-collect that data now.
>
Thanks, I look forward to the next set of patches. Let me know if I
can help test what's on the list or if you want me to wait for the next
round
Balbir
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-06-04 8:36 UTC (permalink / raw)
To: Balbir Singh
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, apopple, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <aiDVMgu0viTIml8H@parvat>
On Thu, Jun 04, 2026 at 11:43:14AM +1000, Balbir Singh wrote:
> On Wed, Jun 03, 2026 at 08:02:09AM +0100, Gregory Price wrote:
> >
> > Here is how the page allocator fallback lists and nodemasks interact:
> >
> > Fallbacks A: A B
> > Fallbacks B: B A
> > Fallbacks C: C A B (Private)
> > Fallbacks D: D B A (Private)
> >
>
> Do we want regular memory (N_MEMORY) in the fallback list of device private nodes?
> The assumption is that we have ATS translation enabled? Assumiung A and
> B are N_MEMORY here or am I misreading your illustraion?
>
If we don't have __GFP_PRIVATE, then probably not. This is a holdover
from the current __GFP_PRIVATE branch so that if the preferred_nid=
value is a private node (which is a hint, but not a hard control),
there's a way for that allocation to land *somewhere*.
__GFP_PRIVATE would say "Only allow access to private nodes if this
flag is provided - otherwise treat that as unreachable and fall back".
(__GFP_PRIVATE | __GFP_THISNODE) then does exactly what you expect (only
allocate from specifically this private node and don't fall back).
This has the added benefit of not causing OOM on allocation failure.
Some would consider such a request a bug (i.e. that caller has a bad
mask), but I find the premise of that statement to be flawwed if only
because we do not have good controls over what ends up in a nodemask due
to the existence of things like possible_nodes.
> > If we wanted to change this behavior, realistically we'd be looking for
> > a way to add specific nodes to certain fallback lists - rather than
> > modify the nodemask interaction in some way.
>
> Yes, that is what we did with CDM, control the fallback for
> N_MEMORY_PRIVATE, but there is a design decision to be made here.
>
Agreed, but also one which can be deferred and played with since it's
all kernel-internal. None of this should have UAPI implications, and we
need need to accept that we're going to get it wrong on the first try.
> > 2) full mempolicy support doesn't really make sense
> >
> > task mempolicy PROBABLY should never really touch private nodes,
> > while VMA policy certainly can. Assuming we're able to support
> > multi-private-node masks, none of the non-bind mempolicies even
> > make sense for most private nodes (interleave? weighted interleave?)
> >
>
> Yes, mostly, but is that baked into the design? If so, why?
>
"Baked in" in this case would mean:
set_mempolicy(..., private_node) -> -EINVAL
mbind(..., private_node) -> Success
With appropriate documentation.
This can be changed later if a reasonable design was agreed upon.
> > 4) File VMA interactions don't entirely make sense with mbind
> >
> > In theory you might want:
> >
> > fd = open("somefile", ...);
> > mem = mmap(fd, ...);
> > mbind(mem, ..., private_node);
> > for page in mem:
> > mem[page_off] /* fault file into private memory */
> >
> > In reality: This does not work the way you want.
>
> Why not? Just curious about what you found?
>
Because pagecache pages are associated with potentially many VMAs.
The fault can be a soft fault or a hard fault. On soft fault - the page
was already present, and will simply fault into VMA without being
migrated.
You can imagine the following
Process A:
fd = open("somefile", ...);
mem = mmap(fd, ...);
mbind(mem, ..., private_node_A);
for page in mem:
mem[page_off] /* fault file into private memory */
Process B:
fd = open("somefile", ...);
mem = mmap(fd, ...);
mbind(mem, ..., private_node_B);
for page in mem:
mem[page_off] /* fault file into private memory */
If process A runs first, and assuming VMA mempolicy is respected for
file backed allocation (note: it's not, see below) - then the second
process will think the memory now lives on node B when it's already
living on node A (pages are not migrated on fault).
filemap page cache means file-backed pages are global resources.
Re file-backed VMAs - see filemap_alloc_folio_noprof in mm/filemap.c
struct folio *filemap_alloc_folio_noprof(gfp_t gfp, unsigned int order)
{
int n;
struct folio *folio;
if (cpuset_do_page_mem_spread()) {
unsigned int cpuset_mems_cookie;
do {
cpuset_mems_cookie = read_mems_allowed_begin();
n = cpuset_mem_spread_node();
folio = __folio_alloc_node_noprof(gfp, order, n);
} while (!folio && read_mems_allowed_retry(cpuset_mems_cookie));
return folio;
}
return folio_alloc_noprof(gfp, order);
}
We'd have to hang a mempolicy off of the file and use fctl or something
like this if we want a file to have a node preference.
> >
> > I went digging and we need a few mild extensions to allow
> > migration on mbind to work for pagecache pages, and the fault
> > path does not necessarily respect the vma mempolicy always.
> >
> > You also start getting into the question of "what happens when
> > the node is out of memory and you don't have reclaim support?".
>
> Yes, we should discuss reclaim support, I think we should allow for
> reclaim. It allows you to overcommit private memory the way we can
> with regular memory.
>
Reclaim support is feasible, but again - crawl, walk, run.
If we get the base private node infrastructure in place, we can break
things like mempolicy and reclaim support into different work streams
to enable support for these features.
Different private node users will be interested in different
combinations of mm/ service support.
For example: compressed memory as a swap backend DOES NOT want explicit
reclaim support - it will need to manage its own shrinker. This comes
from requirements associated with that specific use case (which I do not
want to get into here).
That is why this series introduced the concept of NP_OPS_* - so that the
owner (driver) of a private node (such as a CXL-enabled accelerator
driver) can tell mm/ what services it should enable for that node.
> >
> > For all these reasons, I think the be mbind/mempolicy support with
> > private nodes needs to be brought in with follow up work - not
> > introduced as part of the baseline set.
> >
>
> I am not opposed to the follow up work, but I feel mbind() should
> be the fundamental work and user space API.
>
This is informed by a single use case / device.
There are users / devices that don't want any UAPI for their memory,
but simply wish to re-utilize some subsection of mm/ (page_alloc,
reclaim, etc).
> >
> > I am arguing for #1 - the community has argued for #2 and "fixing
> > existing nodemask users". I think we can ship #2 and pivot to #1 if we
> > find fixing existing users is infeasible or too much of a maintenance
> > burden.
>
> Again happy to discuss this, I'd like to make sure we agree on the
> design. I am wondering if there is any experimental data to choose
> between 1 and 2.
>
I am trying to test whether, lacking __GFP_PRIVATE, any normal runtime
operations access private nodes removed from fallback lists are reached
via something like the possible / online nodemask.
I remember, maybe a year ago, there were per-node allocations happening
during hotplug and that's why I originally proposed __GFP_PRIVATE, but
I'm trying to re-collect that data now.
~Gregory
^ permalink raw reply
* Re: [PATCH v3 2/4] mm/zswap: Implement proactive writeback
From: Yosry Ahmed @ 2026-06-04 5:36 UTC (permalink / raw)
To: Hao Jia, Johannes Weiner, shakeel.butt, mhocko, tj, mkoutny,
roman.gushchin
Cc: Nhat Pham, akpm, chengming.zhou, muchun.song, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <6db27a22-cc7a-9a94-db3f-c912fd39aa32@gmail.com>
> >> But doesn't it make more sense to specify the compressed size, which is
> >> ultimately the amount of memory you actually want to reclaim.
> >>
> >
> > I personally prefer compressed size to pre-compressed size. That's
> > kinda what user cares about, no?
> >
> > One thing we can do is let users prescribe a compressed size, but
> > internally, we can multiply that by the average compression ratio.
> > That gives us a guesstimate of how many pages we need to reclaim, and
> > you can follow the rest of your implementation as is (perhaps with
> > short-circuit when we reach the goal with fewer pages reclaimed).
>
> Got it. I will change it to use the compressed size in the next version.
>
> Yosry, Nhat, should we continue using the zswap_writeback_only key to
> trigger proactive writeback?
I *really* want the memcg maintainers to chime in here, it's
ultimately their call.
Michal? Johannes? Shakeel? Roman? Anyone? :D
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Yosry Ahmed @ 2026-06-04 5:34 UTC (permalink / raw)
To: Hao Jia
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <9898f83d-fae9-e284-6b85-c7f4089840a0@gmail.com>
> >> For instance, suppose a parent memcg has two children, memcg1 and memcg2,
> >> each with 200MB of zswap (100MB inactive). Triggering proactive writeback on
> >> the parent memcg will exhaust memcg1's inactive zswap pages. After that,
> >> even though memcg2 still has plenty of inactive zswap pages, it will
> >> continue to write back memcg1's active zswap pages. Writing back active
> >> zswap pages causes the user-space agent to prematurely abort the writeback
> >> because it detects that certain memcg metrics have exceeded predefined
> >> thresholds.
> >
> > This will only happen if the reclaim size is smaller than the batch
> > size, right? Otherwise the kernel should reclaim more or less equally
> > from both memcgs?
> >
>
> I gave it some thought. Not using a cursor could lead to unfairness
> issues with certain writeback sizes:
>
> - If the writeback size is an odd multiple of WB_BATCH (e.g.,
> triggering a writeback of 3 * WB_BATCH), with 2 child cgroups, the
> writeback ratio might end up being 2:1.
> - If a memcg has 5 child cgroups and a writeback of 2 * WB_BATCH is
> triggered, it might repeatedly write back from only the first 2 child
> cgroups.
>
> Although setting a smaller WB_BATCH might mitigate this unfairness, it
> could hurt writeback efficiency. Let's just use per-memcg cursors to
> completely fix these corner cases.
Exactly, the batch size should be small enough that any unfairness is
not a problem. I would honestly just do batching without a per-memcg
cursor, unless we have numbers to prove that the efficiency is
affected when we use a small batch size. Let's only introduce
complexity when needed please.
^ permalink raw reply
* [tj-cgroup:for-next] BUILD SUCCESS 3e0d2ffb1ae163aa8e97e4ddd7aafa9b7162b5d5
From: kernel test robot @ 2026-06-04 2:18 UTC (permalink / raw)
To: Tejun Heo; +Cc: cgroups
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
branch HEAD: 3e0d2ffb1ae163aa8e97e4ddd7aafa9b7162b5d5 Merge branch 'for-7.1-fixes' into for-next
elapsed time: 1724m
configs tested: 119
configs skipped: 5
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-15.2.0
alpha allyesconfig gcc-15.2.0
arc allmodconfig gcc-15.2.0
arc allnoconfig gcc-15.2.0
arc allyesconfig gcc-15.2.0
arc randconfig-001-20260604 clang-17
arc randconfig-002-20260604 clang-17
arm allnoconfig clang-23
arm allnoconfig gcc-15.2.0
arm allyesconfig gcc-15.2.0
arm randconfig-001-20260604 clang-17
arm randconfig-002-20260604 clang-17
arm randconfig-003-20260604 clang-17
arm randconfig-004-20260604 clang-17
arm64 allmodconfig clang-19
arm64 allnoconfig gcc-15.2.0
csky allmodconfig gcc-15.2.0
csky allnoconfig gcc-15.2.0
hexagon allmodconfig clang-17
hexagon allmodconfig gcc-15.2.0
hexagon allnoconfig clang-23
hexagon allnoconfig gcc-15.2.0
hexagon randconfig-001 gcc-11.5.0
hexagon randconfig-001-20260604 gcc-11.5.0
hexagon randconfig-002 gcc-11.5.0
hexagon randconfig-002-20260604 gcc-11.5.0
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 allnoconfig gcc-15.2.0
i386 allyesconfig gcc-14
i386 buildonly-randconfig-001-20260604 clang-20
i386 buildonly-randconfig-002-20260604 clang-20
i386 buildonly-randconfig-003-20260604 clang-20
i386 buildonly-randconfig-004-20260604 clang-20
i386 buildonly-randconfig-005-20260604 clang-20
i386 buildonly-randconfig-006-20260604 clang-20
i386 randconfig-011-20260604 gcc-14
i386 randconfig-012-20260604 gcc-14
i386 randconfig-013-20260604 gcc-14
i386 randconfig-014-20260604 gcc-14
i386 randconfig-015-20260604 gcc-14
i386 randconfig-016-20260604 gcc-14
i386 randconfig-017-20260604 gcc-14
loongarch allnoconfig clang-23
loongarch allnoconfig gcc-15.2.0
loongarch randconfig-001 gcc-11.5.0
loongarch randconfig-001-20260604 gcc-11.5.0
loongarch randconfig-002 gcc-11.5.0
loongarch randconfig-002-20260604 gcc-11.5.0
m68k allmodconfig gcc-15.2.0
m68k allnoconfig gcc-15.2.0
m68k allyesconfig gcc-15.2.0
microblaze allnoconfig gcc-15.2.0
microblaze allyesconfig gcc-15.2.0
mips allmodconfig gcc-15.2.0
mips allnoconfig gcc-15.2.0
mips allyesconfig gcc-15.2.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig clang-17
nios2 allnoconfig gcc-11.5.0
nios2 randconfig-001 gcc-11.5.0
nios2 randconfig-001-20260604 gcc-11.5.0
nios2 randconfig-002 gcc-11.5.0
nios2 randconfig-002-20260604 gcc-11.5.0
openrisc allmodconfig gcc-15.2.0
openrisc allnoconfig clang-17
openrisc allnoconfig gcc-15.2.0
openrisc defconfig gcc-16.1.0
parisc allmodconfig gcc-15.2.0
parisc allnoconfig clang-17
parisc allnoconfig gcc-15.2.0
parisc allyesconfig gcc-15.2.0
parisc defconfig gcc-16.1.0
powerpc allmodconfig gcc-15.2.0
powerpc allnoconfig clang-17
powerpc allnoconfig gcc-15.2.0
riscv allnoconfig clang-17
riscv allnoconfig gcc-15.2.0
riscv allyesconfig clang-16
riscv defconfig gcc-16.1.0
s390 allmodconfig clang-18
s390 allnoconfig clang-17
s390 allnoconfig clang-23
s390 allyesconfig gcc-15.2.0
s390 defconfig gcc-16.1.0
sh allmodconfig gcc-15.2.0
sh allnoconfig clang-17
sh allnoconfig gcc-15.2.0
sh allyesconfig gcc-15.2.0
sh defconfig gcc-14
sh urquell_defconfig gcc-16.1.0
sparc allnoconfig clang-17
sparc allnoconfig gcc-15.2.0
sparc defconfig gcc-16.1.0
sparc64 allmodconfig clang-23
sparc64 defconfig gcc-14
um allmodconfig clang-19
um allnoconfig clang-17
um allnoconfig clang-23
um allyesconfig gcc-14
um allyesconfig gcc-15.2.0
um defconfig gcc-14
um i386_defconfig gcc-14
um x86_64_defconfig gcc-14
x86_64 allmodconfig clang-20
x86_64 allnoconfig clang-17
x86_64 allnoconfig clang-20
x86_64 allyesconfig clang-20
x86_64 defconfig gcc-14
x86_64 kexec clang-22
x86_64 rhel-9.4 clang-22
x86_64 rhel-9.4-bpf gcc-14
x86_64 rhel-9.4-func clang-22
x86_64 rhel-9.4-kselftests clang-22
x86_64 rhel-9.4-kunit gcc-14
x86_64 rhel-9.4-ltp gcc-14
x86_64 rhel-9.4-rust clang-20
xtensa allnoconfig clang-17
xtensa allnoconfig gcc-15.2.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 2/4] mm/zswap: Implement proactive writeback
From: Hao Jia @ 2026-06-04 2:11 UTC (permalink / raw)
To: Nhat Pham, Yosry Ahmed
Cc: Johannes Weiner, akpm, tj, shakeel.butt, mhocko, mkoutny,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <CAKEwX=OhxUxRCEfvZMnWzXy=Fa4jgzL3DuP-RmaVzdK65m4bew@mail.gmail.com>
On 2026/6/4 02:14, Nhat Pham wrote:
> On Wed, Jun 3, 2026 at 10:58 AM Yosry Ahmed <yosry@kernel.org> wrote:
>>
>> On Wed, Jun 03, 2026 at 07:22:36PM +0800, Hao Jia wrote:
>>>
>>>
>>> On 2026/5/30 09:40, Yosry Ahmed wrote:
>>>> On Fri, May 29, 2026 at 12:58:09PM -0700, Nhat Pham wrote:
>>>>> On Tue, May 26, 2026 at 4:46 AM Hao Jia <jiahao.kernel@gmail.com> wrote:
>>>>>>
>>>>>> From: Hao Jia <jiahao1@lixiang.com>
>>>>>>
>>>>>> Zswap currently writes back pages to backing swap reactively, triggered
>>>>>> either by the shrinker or when the pool reaches its size limit. There is
>>>>>> no mechanism to control the amount of writeback for a specific memory
>>>>>> cgroup. However, users may want to proactively write back zswap pages,
>>>>>> e.g., to free up memory for other applications or to prepare for
>>>>>> memory-intensive workloads.
>>>>>>
>>>>>> Introduce a "zswap_writeback_only" key to the memory.reclaim cgroup
>>>>>> interface. When specified, this key bypasses standard memory reclaim
>>>>>> and exclusively performs proactive zswap writeback up to the requested
>>>>>> budget. If omitted, the default reclaim behavior remains unchanged.
>>>>>>
>>>>>> Example usage:
>>>>>> # Write back 100MB of pages from zswap to the backing swap
>>>>>> echo "100M zswap_writeback_only" > memory.reclaim
>>>>>
>>>>> Hmmm, so this 100MB is the pre-compression size? i.e if this 100 MB
>>>>> compresses to 25 MB, then you're only freeing 25 MB?
>>>>>
>>>>> I'm ok-ish with this, but can you document it?
>>>>
>>>> That's a good point. I think pre-compressed size doesn't make sense to
>>>> be honest. We should care about how much memory we are actually trying
>>>> to save by doing writeback here.
>>>>
>>>> The pre-compressed size is only useful in determining the blast radius,
>>>> how many actual pages are going to have slower page faults now. But
>>>> then, I don't think there's a reasonable way for userspace to decide
>>>> that.
>>>>
>>>> I understand passing in the compressed size is tricky because we need to
>>>> keep track of the size of the compressed pages we end up writing back,
>>>> but it should be doable.
>>>
>>> Agreed. Using pre-compressed size is probably easier to implement. IIRC,
>>> interfaces like ZRAM writeback_limit are also calculated using the
>>> pre-compressed size.
>>>
>>> I'll clarify this in the documentation in the next version.
>>>
>>>>
>>>> If we really want pre-compressed size here, then yes we need to make it
>>>> very clear, and I vote that we use a separate interface in this case
>>>> because memory.reclaim having different meanings for the amount of
>>>> memory written to it is extremely counter-intuitive.
>>>>
>>> Agree. This would indeed break the semantics of memory.reclaim. I will use a
>>> separate interface for proactive writeback in the next version.
>>
>> But doesn't it make more sense to specify the compressed size, which is
>> ultimately the amount of memory you actually want to reclaim.
>>
>
> I personally prefer compressed size to pre-compressed size. That's
> kinda what user cares about, no?
>
> One thing we can do is let users prescribe a compressed size, but
> internally, we can multiply that by the average compression ratio.
> That gives us a guesstimate of how many pages we need to reclaim, and
> you can follow the rest of your implementation as is (perhaps with
> short-circuit when we reach the goal with fewer pages reclaimed).
Got it. I will change it to use the compressed size in the next version.
Yosry, Nhat, should we continue using the zswap_writeback_only key to
trigger proactive writeback?
Thanks,
Hao
^ permalink raw reply
* Re: [PATCH v3 1/4] mm/zswap: Make shrink_worker writeback cursor per-memcg
From: Hao Jia @ 2026-06-04 1:58 UTC (permalink / raw)
To: Yosry Ahmed
Cc: akpm, tj, hannes, shakeel.butt, mhocko, mkoutny, nphamcs,
chengming.zhou, muchun.song, roman.gushchin, cgroups, linux-mm,
linux-kernel, linux-doc, Hao Jia
In-Reply-To: <aiBpibRNi0BcM1Zu@google.com>
On 2026/6/4 01:53, Yosry Ahmed wrote:
> On Wed, Jun 03, 2026 at 11:02:54AM +0800, Hao Jia wrote:
>>
>>
>> On 2026/6/3 07:19, Yosry Ahmed wrote:
>>>>>>>> Proactive writeback also wants a similar per-memcg cursor that is
>>>>>>>> scoped to the specified memcg, so that repeated invocations against
>>>>>>>> the same memcg make forward progress across its descendant memcgs
>>>>>>>> instead of restarting from the first child memcg each time.
>>>>>>>
>>>>>>> Is this a problem in practice?
>>>>>>>
>>>>>>> Is the concern the overhead of scanning memcgs repeatedly, or lack of
>>>>>>> fairness? I wonder if we should just do writeback in batches from all
>>>>>>> memcgs, similar to how reclaim does it, then evaluate at the end if we
>>>>>>> need to start over?
>>>>>>>
>>>>>>
>>>>>> Not using a per-cgroup cursor will cause issues for "repeated small-budget
>>>>>> calls" cases. For example, repeatedly triggering a 2MB writeback might
>>>>>> result in only writing back pages from the first few child memcgs every
>>>>>> time. In the worst-case scenario (where the writeback amount is less than
>>>>>> WB_BATCH), it might only ever write back from the first child memcg.
>>>>>
>>>>> Right, so a fairness concern?
>>>>>
>>>>> I wonder if we should just reclaim a batch from each memcg, then check
>>>>> if we reached the goal, otherwise start over. If the batch size is small
>>>>> enough that should work?
>>>>
>>>> Even with a small batch size, for small writeback requests triggered by
>>>> user-space (e.g., 2MB, which is batch size * N), it might still repeatedly
>>>> write back from only the first N child memcgs.
>>>
>>> Yes, I understand, I am asking if this is a problem in practice. For
>>> this to be a problem we'd need to trigger small writeback requests and
>>> have many memcgs.
>>>
>>>> This could cause the user-space agent to prematurely give up on zswap
>>>> writeback.
>>>
>>> Why? The kernel should not return before trying to writeback from all
>>> memcgs. If we scan the first N child memcgs and did not writeback
>>> enough, we should keep going, right?
>>>
>>
>> Yes, this issue is not caused by the kernel, but rather by our user-space
>> agent itself.
>>
>> For instance, suppose a parent memcg has two children, memcg1 and memcg2,
>> each with 200MB of zswap (100MB inactive). Triggering proactive writeback on
>> the parent memcg will exhaust memcg1's inactive zswap pages. After that,
>> even though memcg2 still has plenty of inactive zswap pages, it will
>> continue to write back memcg1's active zswap pages. Writing back active
>> zswap pages causes the user-space agent to prematurely abort the writeback
>> because it detects that certain memcg metrics have exceeded predefined
>> thresholds.
>
> This will only happen if the reclaim size is smaller than the batch
> size, right? Otherwise the kernel should reclaim more or less equally
> from both memcgs?
>
I gave it some thought. Not using a cursor could lead to unfairness
issues with certain writeback sizes:
- If the writeback size is an odd multiple of WB_BATCH (e.g.,
triggering a writeback of 3 * WB_BATCH), with 2 child cgroups, the
writeback ratio might end up being 2:1.
- If a memcg has 5 child cgroups and a writeback of 2 * WB_BATCH is
triggered, it might repeatedly write back from only the first 2 child
cgroups.
Although setting a smaller WB_BATCH might mitigate this unfairness, it
could hurt writeback efficiency. Let's just use per-memcg cursors to
completely fix these corner cases.
Thanks,
Hao
>> Of course, real-world scenarios are much more complex, and this kind of case
>> is extremely rare in our environment.
>>
>> That being said, your suggestion of using the global lock for the per-memcg
>> cursors makes the writeback fairer and would resolve these corner cases.
>
> Right, but I'd rather not do per-memcg cursors at all if we can avoid
> it. Will using batches help make reclaim fair over all memcgs without a
> cursor?
>
> We can always add the cursor later if needed.
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Balbir Singh @ 2026-06-04 1:43 UTC (permalink / raw)
To: Gregory Price
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, apopple, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <ah_RcTU8SpQG7hab@gourry-fedora-PF4VCD3F>
On Wed, Jun 03, 2026 at 08:02:09AM +0100, Gregory Price wrote:
> On Wed, Jun 03, 2026 at 03:00:01PM +1000, Balbir Singh wrote:
> > On Tue, Jun 02, 2026 at 09:57:48AM +0100, Gregory Price wrote:
> > > On Tue, Jun 02, 2026 at 12:16:50PM +1000, Balbir Singh wrote:
> > > >
> > > > I was think we wouldn't need explicit flags and that allocations would
> > > > happen from user space using __GFP_THISNODE to the node or via a nodemask
> > > > based on nodes of interest. Is there a reason to add this flag, a system
> > > > might have more than one source of N_MEMORY_PRIVATE?
> > > >
> > >
> > > There's a few things to unpack here. I discussed this many times on
> > > list and at LSF, but to reiterate.
> > >
> > > 1) __GFP_THISNODE is insufficient to enforce isolation and otherwise
> > > not particularly useful. Additionally, from userland, it's not
> > > something you can actually set.
> >
> > I was thinking mbind()/mempolicy() is how we get to it. It already
> > accepts a nodemask.
> >
>
> First let me say: I want to enable mbind access to these nodes.
>
> But let me caveat: I think that needs more time to develop, and
> in the meantime, we can enable the /dev/xxx pattern somewhat trivially.
>
> First let me address a few things about mbind/mempolicy and how it
> interacts with page_alloc.c, I gave this overview at LSF but I don't
> remember if I posted it in any of my follow ups.
>
>
> 1) Fallback lists are filtered by nodemask, the nodemask does not replace
> the fallback list.
>
> Here is how the page allocator fallback lists and nodemasks interact:
>
> Fallbacks A: A B
> Fallbacks B: B A
> Fallbacks C: C A B (Private)
> Fallbacks D: D B A (Private)
>
Do we want regular memory (N_MEMORY) in the fallback list of device private nodes?
The assumption is that we have ATS translation enabled? Assumiung A and
B are N_MEMORY here or am I misreading your illustraion?
> Lets say you pass:
>
> alloc_pages_node(C, ..., nodemask(A,C,D))
>
> So we get
>
> Fallback(C,A,B) & nodemask(A,C,D) -> iterate(C,A)
>
> If we wanted to change this behavior, realistically we'd be looking for
> a way to add specific nodes to certain fallback lists - rather than
> modify the nodemask interaction in some way.
Yes, that is what we did with CDM, control the fallback for
N_MEMORY_PRIVATE, but there is a design decision to be made here.
>
> I think this is out of scope for the first iteration - so supporting
> anything other than mbind() from the start is just pointless.
>
> The only feasible mempolicy you can apply is single-node bind, so
> realistically you can only support mbind.
>
>
> 2) full mempolicy support doesn't really make sense
>
> task mempolicy PROBABLY should never really touch private nodes,
> while VMA policy certainly can. Assuming we're able to support
> multi-private-node masks, none of the non-bind mempolicies even
> make sense for most private nodes (interleave? weighted interleave?)
>
Yes, mostly, but is that baked into the design? If so, why?
> I haven't worked through all the implications of a task policy having
> a private node attached, but the longer I think about it, the less it
> makes sense to just support this outright.
>
>
> 3) Introducing mbind support is not just a simple nodemask on a VMA,
> It also implies migration, cgroup/cpuset, and UAPI interactions.
>
> a) migration:
>
> mbind/mempolicy can and will engage migration when it is called
> with certain flags. Migration has subtle LRU interactions, but
> the patch set I have at least allows this to work.
>
> b) cgroup/cpuset:
>
> cpuset.mems rebinding will cause private nodes to be quietly
> rebound to non-private nodes within a nodemask.
>
> c) between A and B - we really want MPOL_F_STATIC to be required
> for mbind to be applied to private node so that it is never
> forcefully remapped.
>
> That's a UAPI semantic change specific for private nodes we
> should really take time to consider.
>
>
> 4) File VMA interactions don't entirely make sense with mbind
>
> In theory you might want:
>
> fd = open("somefile", ...);
> mem = mmap(fd, ...);
> mbind(mem, ..., private_node);
> for page in mem:
> mem[page_off] /* fault file into private memory */
>
> In reality: This does not work the way you want.
Why not? Just curious about what you found?
>
> I went digging and we need a few mild extensions to allow
> migration on mbind to work for pagecache pages, and the fault
> path does not necessarily respect the vma mempolicy always.
>
> You also start getting into the question of "what happens when
> the node is out of memory and you don't have reclaim support?".
Yes, we should discuss reclaim support, I think we should allow for
reclaim. It allows you to overcommit private memory the way we can
with regular memory.
> The OOM implications jump out at you pretty aggressively.
>
> Moreover other tasks can force the page cache pages to be moved
> as well. So the programming model here just kind of sucks.
>
> Works great for anon memory though :]
>
> For all these reasons, I think the be mbind/mempolicy support with
> private nodes needs to be brought in with follow up work - not
> introduced as part of the baseline set.
>
I am not opposed to the follow up work, but I feel mbind() should
be the fundamental work and user space API.
> > >
> > > for node in possible_nodes:
> > > alloc_pages_node(private_node, __GFP_THISNODE)
> > >
> > > In fact it's the opposite semantic of what we want.
> > > THISNODE says: "Do not fallback back to OTHER nodes".
> > >
> >
> > That's why we need to control the fallback nodes carefully for
> > N_MEMORY_PRIVATE
> >
>
> My point is that __GFP_THISNODE is not actually useful.
>
> If we go by nodemask, submitting a single-node nodemask is the
> equivalent of an empty fallback list.
>
> If we gate access to a private node by __GFP_THISNODE... this is the
> same as just providing a single-node nodelist (putting aside the OOM
> implications for a moment).
>
> And it doesn't even buy you any new filtering ability against existing
> nodemask iterators that may already utilize __GFP_THISNODE. i.e.
>
> for node in online_nodes:
> alloc_pages_node(node, __GFP_THISNODE, ...)
> /* Alloc per-node resources */
>
> This pattern is undesirable, but completely valid.
>
> So overloading/requiring __GFP_THISNODE is just not useful.
>
> I will follow up soon with a new version that limits the private node
> interface to just nodemask and fallback list controls.
>
> I need to test a few more things related to removing normal nodes from
> private node fallbacks before I feel comfortable shipping without
> __GFP_PRIVATE.
>
> > > The semantic we want is "Do not allow allocations from private
> > > nodes UNLESS we specifically request" (__GFP_PRIVATE).
> > >
> > > __GFP_THISNODE does not actually buy you anything here, AND it's
> > > worse, in the scenario where a private node makes its way into the
> > > preferred slot (via possible_nodes or some other nodemask), the
> > > allocator cannot fall back to a node it can access.
> > >
> > > __GFP_THISNODE cannot be overloaded to do anything useful here.
> >
> > Let me clarify, I meant to say, let's use a nodemask for allocation
> > and __GFP_THISNODE gets us to the node we desire, if that is the only
> > node. My earlier comment might not have been clear.
> >
>
> My point was that __GFP_THISNODE is pointless and reduces to providing a
> single node nodemask anyway.
>
> The contention over __GFP_PRIVATE is a bit ideological - do we want:
>
> 1) A hard guarantee that allocations to a private node are controlled
> (__GFP_PRIVATE implies the caller knows what it's doing)
>
> or
>
> 2) A soft guarantee (fallback list isolation only), and needing to
> deal with undesired behavior that's "not technically a bug"
> associated with existing users of global nodemasks (possible,
> online, etc).
>
> I am arguing for #1 - the community has argued for #2 and "fixing
> existing nodemask users". I think we can ship #2 and pivot to #1 if we
> find fixing existing users is infeasible or too much of a maintenance
> burden.
Again happy to discuss this, I'd like to make sure we agree on the
design. I am wondering if there is any experimental data to choose
between 1 and 2.
>
> >
> > Why not use mbind() API's? Do we want to gate allocation/privileges
> > via a /dev?
> >
>
> We want to eventually enable it, but we really need to treat these
> extensions as a separate step from the base so that the UAPI
> implications are given proper scrutiny.
>
> In the short term, /dev/xxx and driver-local/service-local control
> of a node is still very useful.
>
> For example, for my compressed memory work, I have found that if
> implemented as a swap backend - the kernel can manage the node without
> any UAPI implications at all :].
>
> A driver managing memory on a private node could do the same.
>
> ~Gregory
Thanks for the detailed answers, happy to iterate and experiment on
the design with you, my opinions come from way back when we tried
to do CDM (in it's first iteration)
Balbir
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-03 20:09 UTC (permalink / raw)
To: Yosry Ahmed
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAO9r8zP+PkgRzXJcFv+3i2pKFQdLt78Ax1s1DY0qNaiUo7ySqA@mail.gmail.com>
On Wed, Jun 3, 2026 at 12:35 PM Yosry Ahmed <yosry@kernel.org> wrote:
>
> > > > > All that being said, perhaps I am too out of touch with the code to
> > > > > realize it's simply not possible.
> > > > >
> > > > > Honestly, if the main reason we can't have a single swap table for vswap
> > > > > is saving 8 bytes on the reverse mapping, it sounds like a weak-ish
> > > > > argument, even if we can't optimize the reverse mapping away. But maybe
> > > > > I am also out of touch with RAM prices :)
> > > >
> > > > In terms of the space overhead I do agree, FWIW :)
> > > >
> > > > I think the other concern is the indirection overhead with going
> > > > through the xarray for every swap operation, hence the per-CPU vswap
> > > > cluster lookup caching idea:
> > > >
> > > > https://lore.kernel.org/all/20260505153854.1612033-23-nphamcs@gmail.com/
> > >
> > > Right, but we should already avoid the xarray with the swap table
> > > design, right? We just have one swap table pointing to another
> > > essentially?
> >
> > Hmmm, I don't quite follow your suggestion here.
> >
> > For normal swap devices, we organize the space into clusters, and
> > maintain them in various lists (free, nonfull, full etc.). The only
> > difference with a vswap device is we do not have a free list, and have
> > the clusters themselves dynamically allocated.
> >
> > If we're using vswap, we will incur the xarray overhead. There's no
> > avoiding that if we want a dynamic indirection layer. We can of course
> > revisit this data structure design later.
> >
> > So yes, it will be one swap table (vswap cluster) pointing to another
> > swap table (pswap cluster). But to get to the first swap table, you
> > will have to go through xarray still.
>
> Why the xarray? Don't page tables (and shmem page cache) just point
> directly to the vswap entry the same way they point to swap entries
> today?
>
> *looks at the code*
>
> Oh, it's to find the actual cluster because the vswap file can be
> sparse? Hmm yeah I guess we can revisit the data structure here later,
Less sparsity, and more dynamicity :) It might be dense for all we
know - we just don't really know (or want to figure out) the size
statically.
> but IIRC xarrays aren't particularly good for sparse data. Maybe it's
> usually not sprase in practice.
>
> Maybe a maple tree? :)
Maybe :)
>
> > > > If folks like it, what I can do is have CONFIG_ZSWAP depends on
> > > > CONFIG_VSWAP, removes all the non-vswap logic, and call it a day? :)
> > > > Then, on the swap allocation side, if vswap allocation fail and zswap
> > > > writeback is disabled, we can error out early.
> > >
> > > Hmm maybe we can keep it around for now and do that after vswap
> > > stabilizes? It ultimately depend on how much complexity we maintain by
> > > allowing both.
> > >
> > > I think another problem is 32-bit, technically zswap can be used on
> > > 32-bit now, right? So vswap not supporitng 32-bit is a problem.
> >
> > Ah shoot I forgot about that. Hmmm.
> >
> > It's not impossible to make vswap support 32-bit. I did that for v6
> > after all. It just needs extra fields because we have fewer bits to
> > leverage in pointers etc., complicating the logic a bit. Follow-up
> > work? :)
>
> Yeah we can do that, but it's a blocker for zswap only using vswap.
Yeah we can table that. FWIW, if you enable vswap, then zswap should
go through vswap already. It's just code complexity (hopefully for a
short while).
>
> > > General question (for both zswap and general swap code), would a boot
> > > param make implementation simpler? Right now we seem to key off the swap
> > > device having the "vswap" flag, would it help if it was a runtime
> > > constant?
> >
> > Hmmm, even if it's a runtime constant, both branches still have to be
> > there, no? Does the boot param simplify it somehow?
>
> Maybe it doesn't simplify the code, but if the branching causes
> performance overhead we can use static keys. I guess we can still use
> static keys per-swapfile, but it would be more complicated.
>
> Anyway, not super important now.
Ahhh I see what you mean. Yeah we can optimize this later.
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Yosry Ahmed @ 2026-06-03 19:35 UTC (permalink / raw)
To: Nhat Pham
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAKEwX=P95D7wNpWhEAXQpeNPM6eQa2mEZE8Srzfpct=-=Q40tg@mail.gmail.com>
On Wed, Jun 3, 2026 at 12:26 PM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Wed, Jun 3, 2026 at 11:58 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > > > I assume the main reason here is to avoid the extra overhead if
> > > > everything uses vswap, which would mainly be the reverse mapping
> > > > overhead? I guess there's also some simplicity that comes from reusing
> > > > the swap info infra as a whole, including the swap table.
> > >
> > > Yeah it helps a lot that we don't have to rewrite the whole allocator
> > > and swap entry reference counting logic again :)
> >
> > I specifically meant using a full swap info thing for the physical swap
> > device even when it's behind vswap. That seems like an overkill, and we
> > don't need things like the swap entry reference coutning. We probably
> > just need a bitmap and a reverse mapping.
> >
> > So I am assuming the main reason why we are not doing that (at least for
> > now) is simplicity?
>
> Mostly.
>
> FWIW, we're pretty close to full deduplication. Right now, physical
> swap clusters have a couple of fields that are not needed when they're
> backing a vswap cluster:
>
> 1. The main swap table (which houses swap cache, swap shadow, and
> reference counting): I repurpose it for the rmap :) It's an array of
> unsigned long, which works for rmap.
>
> 2. memcg_table: still duplicated, but I think I can make sure this is
> not allocated if physical swap clusters only back vswap entries. I
> have a prototype that I'm testing for this.
>
> 3. The zeromap field: this is actually not allocated in 64 bit
> architecture, IIUC, which is what I'm gating CONFIG_VSWAP on. If we
> extend vswap to supporting 32 bits, this can also be dynamically
> allocated.
>
> 4. Extend table - this is for the swap count overfills, and already
> dynamically allocated.
I see.
> > > > All that being said, perhaps I am too out of touch with the code to
> > > > realize it's simply not possible.
> > > >
> > > > Honestly, if the main reason we can't have a single swap table for vswap
> > > > is saving 8 bytes on the reverse mapping, it sounds like a weak-ish
> > > > argument, even if we can't optimize the reverse mapping away. But maybe
> > > > I am also out of touch with RAM prices :)
> > >
> > > In terms of the space overhead I do agree, FWIW :)
> > >
> > > I think the other concern is the indirection overhead with going
> > > through the xarray for every swap operation, hence the per-CPU vswap
> > > cluster lookup caching idea:
> > >
> > > https://lore.kernel.org/all/20260505153854.1612033-23-nphamcs@gmail.com/
> >
> > Right, but we should already avoid the xarray with the swap table
> > design, right? We just have one swap table pointing to another
> > essentially?
>
> Hmmm, I don't quite follow your suggestion here.
>
> For normal swap devices, we organize the space into clusters, and
> maintain them in various lists (free, nonfull, full etc.). The only
> difference with a vswap device is we do not have a free list, and have
> the clusters themselves dynamically allocated.
>
> If we're using vswap, we will incur the xarray overhead. There's no
> avoiding that if we want a dynamic indirection layer. We can of course
> revisit this data structure design later.
>
> So yes, it will be one swap table (vswap cluster) pointing to another
> swap table (pswap cluster). But to get to the first swap table, you
> will have to go through xarray still.
Why the xarray? Don't page tables (and shmem page cache) just point
directly to the vswap entry the same way they point to swap entries
today?
*looks at the code*
Oh, it's to find the actual cluster because the vswap file can be
sparse? Hmm yeah I guess we can revisit the data structure here later,
but IIRC xarrays aren't particularly good for sparse data. Maybe it's
usually not sprase in practice.
Maybe a maple tree? :)
> > > If folks like it, what I can do is have CONFIG_ZSWAP depends on
> > > CONFIG_VSWAP, removes all the non-vswap logic, and call it a day? :)
> > > Then, on the swap allocation side, if vswap allocation fail and zswap
> > > writeback is disabled, we can error out early.
> >
> > Hmm maybe we can keep it around for now and do that after vswap
> > stabilizes? It ultimately depend on how much complexity we maintain by
> > allowing both.
> >
> > I think another problem is 32-bit, technically zswap can be used on
> > 32-bit now, right? So vswap not supporitng 32-bit is a problem.
>
> Ah shoot I forgot about that. Hmmm.
>
> It's not impossible to make vswap support 32-bit. I did that for v6
> after all. It just needs extra fields because we have fewer bits to
> leverage in pointers etc., complicating the logic a bit. Follow-up
> work? :)
Yeah we can do that, but it's a blocker for zswap only using vswap.
> > General question (for both zswap and general swap code), would a boot
> > param make implementation simpler? Right now we seem to key off the swap
> > device having the "vswap" flag, would it help if it was a runtime
> > constant?
>
> Hmmm, even if it's a runtime constant, both branches still have to be
> there, no? Does the boot param simplify it somehow?
Maybe it doesn't simplify the code, but if the branching causes
performance overhead we can use static keys. I guess we can still use
static keys per-swapfile, but it would be more complicated.
Anyway, not super important now.
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Nhat Pham @ 2026-06-03 19:26 UTC (permalink / raw)
To: Yosry Ahmed
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <aiB2sHqxcBAJrTkP@google.com>
On Wed, Jun 3, 2026 at 11:58 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> > > I assume the main reason here is to avoid the extra overhead if
> > > everything uses vswap, which would mainly be the reverse mapping
> > > overhead? I guess there's also some simplicity that comes from reusing
> > > the swap info infra as a whole, including the swap table.
> >
> > Yeah it helps a lot that we don't have to rewrite the whole allocator
> > and swap entry reference counting logic again :)
>
> I specifically meant using a full swap info thing for the physical swap
> device even when it's behind vswap. That seems like an overkill, and we
> don't need things like the swap entry reference coutning. We probably
> just need a bitmap and a reverse mapping.
>
> So I am assuming the main reason why we are not doing that (at least for
> now) is simplicity?
Mostly.
FWIW, we're pretty close to full deduplication. Right now, physical
swap clusters have a couple of fields that are not needed when they're
backing a vswap cluster:
1. The main swap table (which houses swap cache, swap shadow, and
reference counting): I repurpose it for the rmap :) It's an array of
unsigned long, which works for rmap.
2. memcg_table: still duplicated, but I think I can make sure this is
not allocated if physical swap clusters only back vswap entries. I
have a prototype that I'm testing for this.
3. The zeromap field: this is actually not allocated in 64 bit
architecture, IIUC, which is what I'm gating CONFIG_VSWAP on. If we
extend vswap to supporting 32 bits, this can also be dynamically
allocated.
4. Extend table - this is for the swap count overfills, and already
dynamically allocated.
>
> > >
> > > I don't like that the code bifurcates for vswap vs. normal swap entries
> > > though. Not sure if this is an issue that can be fixed with proper
> > > abstractions to hide it, or if the design needs modifications. I was
> > > honestly really hoping we don't end up with this. I was hoping that the
> > > physical swap device no longer uses a full swap table and all, and
> > > everything goes through vswap.
> > >
> > > I hoping that if redirection isn't needed (e.g. zswap is disabled),
> > > vswap can directly encode the physical swap slot so that the reverse
> > > mapping isn't needed -- so we avoid the overhead without keeping the
> > > physical swap device using a fully-fledged swap table.
> >
> > Can you expand on "vswap can directly encode the physical swap slot"?
> > I'm not sure I follow here.
>
> I meant that if redirection is not needed (e.g. zswap is disabled), then
> instead of having a vswap device pointing at a physical swap device, we
> can just the data (e.g. phyiscal swap slot) in the vswap device
> directly. Then we don't need a full swap info thing and swap table for
> the physical swap device.
>
> This directly ties into my question above, about why we have a
> fully-fledged swap info thing for the physical swap device when using
> vswap.
See above.
>
> > >
> > > All that being said, perhaps I am too out of touch with the code to
> > > realize it's simply not possible.
> > >
> > > Honestly, if the main reason we can't have a single swap table for vswap
> > > is saving 8 bytes on the reverse mapping, it sounds like a weak-ish
> > > argument, even if we can't optimize the reverse mapping away. But maybe
> > > I am also out of touch with RAM prices :)
> >
> > In terms of the space overhead I do agree, FWIW :)
> >
> > I think the other concern is the indirection overhead with going
> > through the xarray for every swap operation, hence the per-CPU vswap
> > cluster lookup caching idea:
> >
> > https://lore.kernel.org/all/20260505153854.1612033-23-nphamcs@gmail.com/
>
> Right, but we should already avoid the xarray with the swap table
> design, right? We just have one swap table pointing to another
> essentially?
Hmmm, I don't quite follow your suggestion here.
For normal swap devices, we organize the space into clusters, and
maintain them in various lists (free, nonfull, full etc.). The only
difference with a vswap device is we do not have a free list, and have
the clusters themselves dynamically allocated.
If we're using vswap, we will incur the xarray overhead. There's no
avoiding that if we want a dynamic indirection layer. We can of course
revisit this data structure design later.
So yes, it will be one swap table (vswap cluster) pointing to another
swap table (pswap cluster). But to get to the first swap table, you
will have to go through xarray still.
>
> > >
> > > I at least hope that, the current design is not painting us into a
> > > corner (e.g. through userspace interfaces), and we can still achieve a
> > > vswap-for-all implementation in the future (maybe that's what you have
> > > in mind already?).
> >
> > That's still my plan. Operationally speaking, I want to make this
> > completely transparent to users, with minimal to no performance
> > overhead.
>
> So if CONFIG_VSWAP is set all swap devices are vswap by default, right?
> Would it help with testing if it's controlled by a boot param?
>
> >
> > The next action item is to optimize for vswap-on-fast-swapfile case -
> > that was Kairui's main concerns regarding performance. I spent a lot
> > of time perfing and fixing issues for this case in v6. The issues with
> > the most egregious effects and simplest fix (vswap-less
> > swap-cache-only check for e.g) are already fixed in this new design,
> > and eventually I will move the rest (lookup caching) and more to here.
>
> So is the end goal to have vswap be the default rather than a special
> swap device? It would certainly help to include some details about that.
That is my preference - I did allude to it in "Runtime enable/disable
of the vswap device" follow-up section :) I'm a sucker for unified
paths. It kinda depends on whether we can optimize most of vswap
overhead away though - if not, then we have to maintain both paths.
Kairui, how do you feel about this?
>
> > >
> > > Aside from the swap code, the only sticking point for me is the logic
> > > bifurcation in zswap. Why does zswap need to handle vswap vs. not vswap?
> > > I thought the point of the design is to use vswap when zswap is used,
> > > and otherwise use a normal swap table. In a way, one of the goals is to
> > > make zswap a first class swap citizen, but it doesn't seem like we are
> > > achieving that?
> >
> > We already have all the machinery to make zswap completely
> > independent. Right now, if you use vswap, you'll skip the zswap's
> > internal xarray entirely, and just store a zswap entry in the virtual
> > swap cluster's vtable.
> >
> > I just haven't removed the old code for 2 reasons:
> >
> > 1. Reduce the delta on this RFC, to ease the burden for reviewers (and
> > definitely not because I'm lazy :P)
> >
> > 2. The only other practical reason is so that we can let users compile
> > with !CONFIG_VSWAP and still uses zswap on top of the old swapfile
> > setup during the transition/experimentation period for now.
> >
> > But logically and conceptually speaking, there is no reason I can come
> > up with to use zswap on without vswap. The CPU indirection overhead is
> > already partially there (since zswap uses an xarray) and further
> > optimized (cluster loopup caching etc.), as well as the space overhead
> > (vswap replaces the zswap xarray). I actually wrote a whole paragraph
> > about how we should always go for vswap if we're using zswap, but then
> > decide to remove it since there's no code for it yet.
> >
> > If folks like it, what I can do is have CONFIG_ZSWAP depends on
> > CONFIG_VSWAP, removes all the non-vswap logic, and call it a day? :)
> > Then, on the swap allocation side, if vswap allocation fail and zswap
> > writeback is disabled, we can error out early.
>
> Hmm maybe we can keep it around for now and do that after vswap
> stabilizes? It ultimately depend on how much complexity we maintain by
> allowing both.
>
> I think another problem is 32-bit, technically zswap can be used on
> 32-bit now, right? So vswap not supporitng 32-bit is a problem.
Ah shoot I forgot about that. Hmmm.
It's not impossible to make vswap support 32-bit. I did that for v6
after all. It just needs extra fields because we have fewer bits to
leverage in pointers etc., complicating the logic a bit. Follow-up
work? :)
>
> General question (for both zswap and general swap code), would a boot
> param make implementation simpler? Right now we seem to key off the swap
> device having the "vswap" flag, would it help if it was a runtime
> constant?
Hmmm, even if it's a runtime constant, both branches still have to be
there, no? Does the boot param simplify it somehow?
^ permalink raw reply
* Re: [RFC PATCH 0/5] mm, swap: Virtual Swap Space (Swap Table Edition)
From: Yosry Ahmed @ 2026-06-03 19:00 UTC (permalink / raw)
To: Nhat Pham
Cc: kasong, Liam.Howlett, akpm, apopple, axelrasmussen, baohua,
baolin.wang, bhe, byungchul, cgroups, chengming.zhou, chrisl,
corbet, david, dev.jain, gourry, hannes, hughd, jannh,
joshua.hahnjy, lance.yang, lenb, linux-doc, linux-kernel,
linux-mm, linux-pm, lorenzo.stoakes, matthew.brost, mhocko,
muchun.song, npache, pavel, peterx, peterz, pfalcato, rafael,
rakie.kim, roman.gushchin, rppt, ryan.roberts, shakeel.butt,
shikemeng, surenb, tglx, vbabka, weixugc, ying.huang, yosry.ahmed,
yuanchu, zhengqi.arch, ziy, kernel-team, riel, haowenchao22
In-Reply-To: <CAKEwX=MZQJLHNNU0tUqnihdhdPdVd19KhC-HtJxfbQ_d8OezzQ@mail.gmail.com>
> > > I don't like that the code bifurcates for vswap vs. normal swap entries
> > > though. Not sure if this is an issue that can be fixed with proper
> > > abstractions to hide it, or if the design needs modifications. I was
> > > honestly really hoping we don't end up with this. I was hoping that the
> > > physical swap device no longer uses a full swap table and all, and
> > > everything goes through vswap.
> > >
> > > I hoping that if redirection isn't needed (e.g. zswap is disabled),
> > > vswap can directly encode the physical swap slot so that the reverse
> > > mapping isn't needed -- so we avoid the overhead without keeping the
> > > physical swap device using a fully-fledged swap table.
> >
> > Can you expand on "vswap can directly encode the physical swap slot"?
> > I'm not sure I follow here.
> >
> > >
> > > All that being said, perhaps I am too out of touch with the code to
> > > realize it's simply not possible.
> > >
> > > Honestly, if the main reason we can't have a single swap table for vswap
> > > is saving 8 bytes on the reverse mapping, it sounds like a weak-ish
> > > argument, even if we can't optimize the reverse mapping away. But maybe
> > > I am also out of touch with RAM prices :)
> >
> > In terms of the space overhead I do agree, FWIW :)
> >
> > I think the other concern is the indirection overhead with going
> > through the xarray for every swap operation, hence the per-CPU vswap
> > cluster lookup caching idea:
> >
> > https://lore.kernel.org/all/20260505153854.1612033-23-nphamcs@gmail.com/
> >
> > >
> > > I at least hope that, the current design is not painting us into a
> > > corner (e.g. through userspace interfaces), and we can still achieve a
> > > vswap-for-all implementation in the future (maybe that's what you have
> > > in mind already?).
> >
> > That's still my plan. Operationally speaking, I want to make this
> > completely transparent to users, with minimal to no performance
> > overhead.
>
> I do want to add that, even without achieving this, the current design
> already enables a lot of use cases. I think it is a good compromise to
> maintain both virtual and directly mapped physical swap entries for
> now, and revisit the conversation of whether we can afford a mandatory
> vswap layer once all the optimizations have been done :)
>
> We should strive to simplify the codebase, and it will naturally
> happen when the original overhead concern is no longer there. A
> swap-related example: a few years ago, everyone thought swap slot
> cache was needed. But then, Kairui optimized the swap allocator's lock
> contention issue away, and that swap slot cache is suddenly redundant.
> That finally allowed us to get rid of it. Similar thing happened (or
> is happening?) with the SWP_SYNCHRONOUS_IO swapcache-skipping
> heuristics.
I agree, I just want to make sure we have a line of sight (or at least
no blockers) to having a unified vswap layer in the future.
^ 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