* [PATCH 8/8] block, bfq: don't grab queue_lock to initialize bfq
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
The request_queue is frozen and quiesced while the elevator init_sched()
method runs, so queue_lock is not needed for BFQ cgroup initialization.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-iosched.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 42ccfd0c6140..5cabee2d4e7c 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -7207,14 +7207,11 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_queue *eq)
bfqd = kzalloc_node(sizeof(*bfqd), GFP_KERNEL, q->node);
if (!bfqd)
return -ENOMEM;
eq->elevator_data = bfqd;
-
- spin_lock_irq(&q->queue_lock);
q->elevator = eq;
- spin_unlock_irq(&q->queue_lock);
/*
* Our fallback bfqq if bfq_find_alloc_queue() runs into OOM issues.
* Grab a permanent reference to it, so that the normal code flow
* will not attempt to free it.
@@ -7243,11 +7240,10 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_queue *eq)
bfqd->num_actuators = 1;
/*
* If the disk supports multiple actuators, copy independent
* access ranges from the request queue structure.
*/
- spin_lock_irq(&q->queue_lock);
if (ia_ranges) {
/*
* Check if the disk ia_ranges size exceeds the current bfq
* actuator limit.
*/
@@ -7269,11 +7265,10 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_queue *eq)
/* Otherwise use single-actuator dev info */
if (bfqd->num_actuators == 1) {
bfqd->sector[0] = 0;
bfqd->nr_sectors[0] = get_capacity(q->disk);
}
- spin_unlock_irq(&q->queue_lock);
INIT_LIST_HEAD(&bfqd->dispatch);
hrtimer_setup(&bfqd->idle_slice_timer, bfq_idle_slice_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_REL);
--
2.51.0
^ permalink raw reply related
* [PATCH 7/8] mm/page_io: don't nest queue_lock under rcu in bio_associate_blkg_from_page()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
Take a css reference under RCU, drop RCU, and then associate the bio with
the blkg. This avoids nesting queue_lock under RCU and prepares to protect
blkcg with blkcg_mutex instead of queue_lock.
Use css_tryget() instead of css_tryget_online() so swap writeback for
pages charged to a dying memcg still passes the dying css to
bio_associate_blkg_from_css(). That preserves the existing closest-live
ancestor fallback instead of charging those bios to the root blkg.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
mm/page_io.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/page_io.c b/mm/page_io.c
index 70cea9e24d2f..3b54c60c278e 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -315,12 +315,17 @@ static void bio_associate_blkg_from_page(struct bio *bio, struct folio *folio)
return;
rcu_read_lock();
memcg = folio_memcg(folio);
css = cgroup_e_css(memcg->css.cgroup, &io_cgrp_subsys);
- bio_associate_blkg_from_css(bio, css);
+ if (!css || !css_tryget(css))
+ css = NULL;
rcu_read_unlock();
+
+ bio_associate_blkg_from_css(bio, css);
+ if (css)
+ css_put(css);
}
#else
#define bio_associate_blkg_from_page(bio, folio) do { } while (0)
#endif /* CONFIG_MEMCG && CONFIG_BLK_CGROUP */
--
2.51.0
^ permalink raw reply related
* [PATCH 6/8] blk-cgroup: don't nest queue_lock under blkcg->lock in blkcg_destroy_blkgs()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
The correct lock order is q->queue_lock before blkcg->lock, and in order
to prevent deadlock from blkcg_destroy_blkgs(), trylock is used for
q->queue_lock while blkcg->lock is already held, this is hacky.
Refactor blkcg_destroy_blkgs() to hold blkcg->lock only long enough to
get the first blkg and then release it. Then take q->queue_lock and
blkcg->lock in the correct order to destroy the blkg. This is a very cold
path, so the extra lock/unlock cycles are acceptable.
Also prepare to convert protecting blkcg with blkcg_mutex instead of
queue_lock.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 8c9ca52a54f4..d1f69a23c9d6 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1289,10 +1289,25 @@ struct list_head *blkcg_get_cgwb_list(struct cgroup_subsys_state *css)
*
* 3. Once the blkcg ref count goes to zero, blkcg_css_free() is called.
* This finally frees the blkcg.
*/
+static struct blkcg_gq *blkcg_get_first_blkg(struct blkcg *blkcg)
+{
+ struct blkcg_gq *blkg = NULL;
+
+ spin_lock_irq(&blkcg->lock);
+ if (!hlist_empty(&blkcg->blkg_list)) {
+ blkg = hlist_entry(blkcg->blkg_list.first, struct blkcg_gq,
+ blkcg_node);
+ blkg_get(blkg);
+ }
+ spin_unlock_irq(&blkcg->lock);
+
+ return blkg;
+}
+
/**
* blkcg_destroy_blkgs - responsible for shooting down blkgs
* @blkcg: blkcg of interest
*
* blkgs should be removed while holding both q and blkcg locks. As blkcg lock
@@ -1302,36 +1317,28 @@ struct list_head *blkcg_get_cgwb_list(struct cgroup_subsys_state *css)
*
* This is the blkcg counterpart of ioc_release_fn().
*/
static void blkcg_destroy_blkgs(struct blkcg *blkcg)
{
- might_sleep();
+ struct blkcg_gq *blkg;
- spin_lock_irq(&blkcg->lock);
+ might_sleep();
- while (!hlist_empty(&blkcg->blkg_list)) {
- struct blkcg_gq *blkg = hlist_entry(blkcg->blkg_list.first,
- struct blkcg_gq, blkcg_node);
+ while ((blkg = blkcg_get_first_blkg(blkcg))) {
struct request_queue *q = blkg->q;
- if (need_resched() || !spin_trylock(&q->queue_lock)) {
- /*
- * Given that the system can accumulate a huge number
- * of blkgs in pathological cases, check to see if we
- * need to rescheduling to avoid softlockup.
- */
- spin_unlock_irq(&blkcg->lock);
- cond_resched();
- spin_lock_irq(&blkcg->lock);
- continue;
- }
+ spin_lock_irq(&q->queue_lock);
+ spin_lock(&blkcg->lock);
blkg_destroy(blkg);
- spin_unlock(&q->queue_lock);
- }
- spin_unlock_irq(&blkcg->lock);
+ spin_unlock(&blkcg->lock);
+ spin_unlock_irq(&q->queue_lock);
+
+ blkg_put(blkg);
+ cond_resched();
+ }
}
/**
* blkcg_pin_online - pin online state
* @blkcg_css: blkcg of interest
--
2.51.0
^ permalink raw reply related
* [PATCH 5/8] blk-cgroup: don't nest queue_lock under rcu in bio_associate_blkg()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
If a bio is already associated with a blkg, the blkcg is already pinned
until the bio is done, so there is no need for RCU protection. Otherwise,
protect blkcg_css() with RCU independently. Prepare to protect blkcg with
blkcg_mutex instead of queue_lock.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index e2896d582235..8c9ca52a54f4 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -2186,20 +2186,24 @@ void bio_associate_blkg(struct bio *bio)
struct cgroup_subsys_state *css;
if (blk_op_is_passthrough(bio->bi_opf))
return;
- rcu_read_lock();
-
- if (bio->bi_blkg)
+ if (bio->bi_blkg) {
css = bio_blkcg_css(bio);
- else
+ bio_associate_blkg_from_css(bio, css);
+ } else {
+ rcu_read_lock();
css = blkcg_css();
+ if (!css_tryget_online(css))
+ css = NULL;
+ rcu_read_unlock();
- bio_associate_blkg_from_css(bio, css);
-
- rcu_read_unlock();
+ bio_associate_blkg_from_css(bio, css);
+ if (css)
+ css_put(css);
+ }
}
EXPORT_SYMBOL_GPL(bio_associate_blkg);
/**
* bio_clone_blkg_association - clone blkg association from src to dst bio
--
2.51.0
^ permalink raw reply related
* [PATCH 4/8] blk-cgroup: don't nest queue_lock under rcu in blkg_lookup_create()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
Change this in two steps:
1) hold rcu lock and do blkg_lookup() from fast path;
2) hold queue_lock directly from slow path, and don't nest it under rcu
lock;
Prepare to convert protecting blkcg with blkcg_mutex instead of
queue_lock.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 57 +++++++++++++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 21 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 46fc65050c38..e2896d582235 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -466,26 +466,21 @@ static struct blkcg_gq *blkg_create(struct blkcg *blkcg, struct gendisk *disk,
static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
struct gendisk *disk)
{
struct request_queue *q = disk->queue;
struct blkcg_gq *blkg;
- unsigned long flags;
-
- WARN_ON_ONCE(!rcu_read_lock_held());
- blkg = blkg_lookup(blkcg, q);
- if (blkg)
- return blkg;
-
- spin_lock_irqsave(&q->queue_lock, flags);
+ rcu_read_lock();
blkg = blkg_lookup(blkcg, q);
if (blkg) {
if (blkcg != &blkcg_root &&
blkg != rcu_dereference(blkcg->blkg_hint))
rcu_assign_pointer(blkcg->blkg_hint, blkg);
- goto found;
+ rcu_read_unlock();
+ return blkg;
}
+ rcu_read_unlock();
/*
* Create blkgs walking down from blkcg_root to @blkcg, so that all
* non-root blkgs have access to their parents. Returns the closest
* blkg to the intended blkg should blkg_create() fail.
@@ -513,12 +508,10 @@ static struct blkcg_gq *blkg_lookup_create(struct blkcg *blkcg,
}
if (pos == blkcg)
break;
}
-found:
- spin_unlock_irqrestore(&q->queue_lock, flags);
return blkg;
}
static void blkg_destroy(struct blkcg_gq *blkg)
{
@@ -2098,10 +2091,22 @@ void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta)
return;
blkcg_scale_delay(blkg, now);
atomic64_add(delta, &blkg->delay_nsec);
}
+static inline struct blkcg_gq *blkg_lookup_tryget(struct blkcg_gq *blkg)
+{
+retry:
+ if (blkg_tryget(blkg))
+ return blkg;
+
+ blkg = blkg->parent;
+ if (blkg)
+ goto retry;
+
+ return NULL;
+}
/**
* blkg_tryget_closest - try and get a blkg ref on the closet blkg
* @bio: target bio
* @css: target css
*
@@ -2110,24 +2115,34 @@ void blkcg_add_delay(struct blkcg_gq *blkg, u64 now, u64 delta)
* up taking a reference on or %NULL if no reference was taken.
*/
static inline struct blkcg_gq *blkg_tryget_closest(struct bio *bio,
struct cgroup_subsys_state *css)
{
- struct blkcg_gq *blkg, *ret_blkg = NULL;
+ struct request_queue *q = bio->bi_bdev->bd_queue;
+ struct blkcg *blkcg = css_to_blkcg(css);
+ struct blkcg_gq *blkg;
rcu_read_lock();
- blkg = blkg_lookup_create(css_to_blkcg(css), bio->bi_bdev->bd_disk);
- while (blkg) {
- if (blkg_tryget(blkg)) {
- ret_blkg = blkg;
- break;
- }
- blkg = blkg->parent;
- }
+ blkg = blkg_lookup(blkcg, q);
+ if (likely(blkg))
+ blkg = blkg_lookup_tryget(blkg);
rcu_read_unlock();
- return ret_blkg;
+ if (blkg)
+ return blkg;
+
+ /*
+ * Fast path failed, we're probably issuing IO in this cgroup the first
+ * time, hold lock to create new blkg.
+ */
+ spin_lock_irq(&q->queue_lock);
+ blkg = blkg_lookup_create(blkcg, bio->bi_bdev->bd_disk);
+ if (blkg)
+ blkg = blkg_lookup_tryget(blkg);
+ spin_unlock_irq(&q->queue_lock);
+
+ return blkg;
}
/**
* bio_associate_blkg_from_css - associate a bio with a specified css
* @bio: target bio
--
2.51.0
^ permalink raw reply related
* [PATCH 3/8] blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
With previous modification to delay freeing policy data after an RCU grace
period, prfill() can run under RCU instead of taking queue_lock. However,
policy teardown can still clear blkg->pd[plid] after blkcg_print_blkgs()
observes the policy enabled bit.
Load policy data once with READ_ONCE() and skip the blkg if teardown
already cleared it. Do the same in recursive stat walks for descendant
blkgs. Remove the stale BFQ debug queue_lock assertion because
blkcg_print_blkgs() no longer calls prfill() with queue_lock held. This
also lets ioc_qos_prfill() and ioc_cost_model_prfill() use IRQ-safe
ioc->lock locking without re-enabling IRQs while queue_lock is still held.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 8 +++++---
block/blk-cgroup-rwstat.c | 15 +++++++++------
block/blk-cgroup.c | 22 +++++++++++++---------
block/blk-cgroup.h | 6 +++---
block/blk-iocost.c | 8 ++++----
5 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 56f60e36c799..904d9e0d9029 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -1146,20 +1146,22 @@ static u64 bfqg_prfill_stat_recursive(struct seq_file *sf,
struct blkcg_gq *blkg = pd_to_blkg(pd);
struct blkcg_gq *pos_blkg;
struct cgroup_subsys_state *pos_css;
u64 sum = 0;
- lockdep_assert_held(&blkg->q->queue_lock);
-
rcu_read_lock();
blkg_for_each_descendant_pre(pos_blkg, pos_css, blkg) {
+ struct blkg_policy_data *pd;
struct bfq_stat *stat;
if (!pos_blkg->online)
continue;
- stat = (void *)blkg_to_pd(pos_blkg, &blkcg_policy_bfq) + off;
+ pd = blkg_to_pd(pos_blkg, &blkcg_policy_bfq);
+ if (!pd)
+ continue;
+ stat = (void *)pd + off;
sum += bfq_stat_read(stat) + atomic64_read(&stat->aux_cnt);
}
rcu_read_unlock();
return __blkg_prfill_u64(sf, pd, sum);
diff --git a/block/blk-cgroup-rwstat.c b/block/blk-cgroup-rwstat.c
index a55fb0c53558..aae910713814 100644
--- a/block/blk-cgroup-rwstat.c
+++ b/block/blk-cgroup-rwstat.c
@@ -99,26 +99,29 @@ void blkg_rwstat_recursive_sum(struct blkcg_gq *blkg, struct blkcg_policy *pol,
{
struct blkcg_gq *pos_blkg;
struct cgroup_subsys_state *pos_css;
unsigned int i;
- lockdep_assert_held(&blkg->q->queue_lock);
+ WARN_ON_ONCE(!rcu_read_lock_held());
memset(sum, 0, sizeof(*sum));
- rcu_read_lock();
blkg_for_each_descendant_pre(pos_blkg, pos_css, blkg) {
struct blkg_rwstat *rwstat;
if (!pos_blkg->online)
continue;
- if (pol)
- rwstat = (void *)blkg_to_pd(pos_blkg, pol) + off;
- else
+ if (pol) {
+ struct blkg_policy_data *pd = blkg_to_pd(pos_blkg, pol);
+
+ if (!pd)
+ continue;
+ rwstat = (void *)pd + off;
+ } else {
rwstat = (void *)pos_blkg + off;
+ }
for (i = 0; i < BLKG_RWSTAT_NR; i++)
sum->cnt[i] += blkg_rwstat_read_counter(rwstat, i);
}
- rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(blkg_rwstat_recursive_sum);
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b55c43f72bcb..46fc65050c38 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -699,13 +699,13 @@ const char *blkg_dev_name(struct blkcg_gq *blkg)
* @data: data to be passed to @prfill
* @show_total: to print out sum of prfill return values or not
*
* This function invokes @prfill on each blkg of @blkcg if pd for the
* policy specified by @pol exists. @prfill is invoked with @sf, the
- * policy data and @data and the matching queue lock held. If @show_total
- * is %true, the sum of the return values from @prfill is printed with
- * "Total" label at the end.
+ * policy data and @data under RCU read lock. If @show_total is %true, the
+ * sum of the return values from @prfill is printed with "Total" label at the
+ * end.
*
* This is to be used to construct print functions for
* cftype->read_seq_string method.
*/
void blkcg_print_blkgs(struct seq_file *sf, struct blkcg *blkcg,
@@ -717,14 +717,18 @@ void blkcg_print_blkgs(struct seq_file *sf, struct blkcg *blkcg,
struct blkcg_gq *blkg;
u64 total = 0;
rcu_read_lock();
hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) {
- spin_lock_irq(&blkg->q->queue_lock);
- if (blkcg_policy_enabled(blkg->q, pol))
- total += prfill(sf, blkg->pd[pol->plid], data);
- spin_unlock_irq(&blkg->q->queue_lock);
+ struct blkg_policy_data *pd;
+
+ if (!blkcg_policy_enabled(blkg->q, pol))
+ continue;
+
+ pd = blkg_to_pd(blkg, pol);
+ if (pd)
+ total += prfill(sf, pd, data);
}
rcu_read_unlock();
if (show_total)
seq_printf(sf, "Total %llu\n", (unsigned long long)total);
@@ -1591,11 +1595,11 @@ static void blkcg_policy_teardown_pds(struct request_queue *q,
if (pd) {
if (pd->online && pol->pd_offline_fn)
pol->pd_offline_fn(pd);
pd->online = false;
pol->pd_free_fn(pd);
- blkg->pd[pol->plid] = NULL;
+ WRITE_ONCE(blkg->pd[pol->plid], NULL);
}
spin_unlock(&blkcg->lock);
}
}
@@ -1683,11 +1687,11 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
spin_lock(&blkg->blkcg->lock);
pd->blkg = blkg;
pd->plid = pol->plid;
- blkg->pd[pol->plid] = pd;
+ WRITE_ONCE(blkg->pd[pol->plid], pd);
if (pol->pd_init_fn)
pol->pd_init_fn(pd);
if (pol->pd_online_fn)
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index fd206d1fa3c9..5402b4ff6f3f 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -279,13 +279,13 @@ static inline struct blkcg_gq *blkg_lookup(struct blkcg *blkcg,
* @pol: policy of interest
*
* Return pointer to private data associated with the @blkg-@pol pair.
*/
static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg,
- struct blkcg_policy *pol)
+ const struct blkcg_policy *pol)
{
- return blkg ? blkg->pd[pol->plid] : NULL;
+ return blkg ? READ_ONCE(blkg->pd[pol->plid]) : NULL;
}
static inline struct blkcg_policy_data *blkcg_to_cpd(struct blkcg *blkcg,
struct blkcg_policy *pol)
{
@@ -488,11 +488,11 @@ static inline int blkcg_activate_policy(struct gendisk *disk,
const struct blkcg_policy *pol) { return 0; }
static inline void blkcg_deactivate_policy(struct gendisk *disk,
const struct blkcg_policy *pol) { }
static inline struct blkg_policy_data *blkg_to_pd(struct blkcg_gq *blkg,
- struct blkcg_policy *pol) { return NULL; }
+ const struct blkcg_policy *pol) { return NULL; }
static inline struct blkcg_gq *pd_to_blkg(struct blkg_policy_data *pd) { return NULL; }
static inline void blkg_get(struct blkcg_gq *blkg) { }
static inline void blkg_put(struct blkcg_gq *blkg) { }
static inline void blk_cgroup_bio_start(struct bio *bio) { }
static inline bool blk_cgroup_mergeable(struct request *rq, struct bio *bio) { return true; }
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index c136b1f46fcc..1f3f6e0f8901 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3188,11 +3188,11 @@ static u64 ioc_qos_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
struct ioc *ioc = pd_to_iocg(pd)->ioc;
if (!dname)
return 0;
- spin_lock(&ioc->lock);
+ spin_lock_irq(&ioc->lock);
seq_printf(sf, "%s enable=%d ctrl=%s rpct=%u.%02u rlat=%u wpct=%u.%02u wlat=%u min=%u.%02u max=%u.%02u\n",
dname, ioc->enabled, ioc->user_qos_params ? "user" : "auto",
ioc->params.qos[QOS_RPPM] / 10000,
ioc->params.qos[QOS_RPPM] % 10000 / 100,
ioc->params.qos[QOS_RLAT],
@@ -3201,11 +3201,11 @@ static u64 ioc_qos_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
ioc->params.qos[QOS_WLAT],
ioc->params.qos[QOS_MIN] / 10000,
ioc->params.qos[QOS_MIN] % 10000 / 100,
ioc->params.qos[QOS_MAX] / 10000,
ioc->params.qos[QOS_MAX] % 10000 / 100);
- spin_unlock(&ioc->lock);
+ spin_unlock_irq(&ioc->lock);
return 0;
}
static int ioc_qos_show(struct seq_file *sf, void *v)
{
@@ -3386,18 +3386,18 @@ static u64 ioc_cost_model_prfill(struct seq_file *sf,
u64 *u = ioc->params.i_lcoefs;
if (!dname)
return 0;
- spin_lock(&ioc->lock);
+ spin_lock_irq(&ioc->lock);
seq_printf(sf, "%s ctrl=%s model=linear "
"rbps=%llu rseqiops=%llu rrandiops=%llu "
"wbps=%llu wseqiops=%llu wrandiops=%llu\n",
dname, ioc->user_cost_model ? "user" : "auto",
u[I_LCOEF_RBPS], u[I_LCOEF_RSEQIOPS], u[I_LCOEF_RRANDIOPS],
u[I_LCOEF_WBPS], u[I_LCOEF_WSEQIOPS], u[I_LCOEF_WRANDIOPS]);
- spin_unlock(&ioc->lock);
+ spin_unlock_irq(&ioc->lock);
return 0;
}
static int ioc_cost_model_show(struct seq_file *sf, void *v)
{
--
2.51.0
^ permalink raw reply related
* [PATCH 2/8] blk-cgroup: delay freeing policy data after rcu grace period
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
Currently blkcg_print_blkgs() must hold RCU to iterate blkgs from a
blkcg, and prfill() must hold queue_lock to prevent policy data from
being freed by policy deactivation. As a consequence, queue_lock has to
be nested under RCU from blkcg_print_blkgs().
Delay freeing policy data until after an RCU grace period so prfill() can
be protected by RCU alone.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 9 ++++++++-
block/blk-cgroup.h | 2 ++
block/blk-iocost.c | 14 ++++++++++++--
block/blk-iolatency.c | 10 +++++++++-
block/blk-throttle.c | 13 +++++++++++--
5 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index f765e767d36a..56f60e36c799 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -548,17 +548,24 @@ static void bfq_pd_init(struct blkg_policy_data *pd)
bfqg->active_entities = 0;
bfqg->num_queues_with_pending_reqs = 0;
bfqg->rq_pos_tree = RB_ROOT;
}
-static void bfq_pd_free(struct blkg_policy_data *pd)
+static void bfqg_release(struct rcu_head *rcu)
{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
struct bfq_group *bfqg = pd_to_bfqg(pd);
bfqg_put(bfqg);
}
+static void bfq_pd_free(struct blkg_policy_data *pd)
+{
+ call_rcu(&pd->rcu_head, bfqg_release);
+}
+
static void bfq_pd_reset_stats(struct blkg_policy_data *pd)
{
struct bfq_group *bfqg = pd_to_bfqg(pd);
bfqg_stats_reset(&bfqg->stats);
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 1cce3294634d..fd206d1fa3c9 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -138,10 +138,12 @@ static inline struct blkcg *css_to_blkcg(struct cgroup_subsys_state *css)
struct blkg_policy_data {
/* the blkg and policy id this per-policy data belongs to */
struct blkcg_gq *blkg;
int plid;
bool online;
+
+ struct rcu_head rcu_head;
};
/*
* Policies that need to keep per-blkcg data which is independent from any
* request_queue associated to it should implement cpd_alloc/free_fn()
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 0cca88a366dc..c136b1f46fcc 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3024,10 +3024,20 @@ static void ioc_pd_init(struct blkg_policy_data *pd)
spin_lock_irqsave(&ioc->lock, flags);
weight_updated(iocg, &now);
spin_unlock_irqrestore(&ioc->lock, flags);
}
+static void iocg_release(struct rcu_head *rcu)
+{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
+ struct ioc_gq *iocg = pd_to_iocg(pd);
+
+ free_percpu(iocg->pcpu_stat);
+ kfree(iocg);
+}
+
static void ioc_pd_free(struct blkg_policy_data *pd)
{
struct ioc_gq *iocg = pd_to_iocg(pd);
struct ioc *ioc = iocg->ioc;
unsigned long flags;
@@ -3048,12 +3058,12 @@ static void ioc_pd_free(struct blkg_policy_data *pd)
spin_unlock_irqrestore(&ioc->lock, flags);
hrtimer_cancel(&iocg->waitq_timer);
}
- free_percpu(iocg->pcpu_stat);
- kfree(iocg);
+
+ call_rcu(&pd->rcu_head, iocg_release);
}
static void ioc_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
{
struct ioc_gq *iocg = pd_to_iocg(pd);
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 53e8dd2dfa8a..c79056410cd9 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -1026,17 +1026,25 @@ static void iolatency_pd_offline(struct blkg_policy_data *pd)
iolatency_set_min_lat_nsec(blkg, 0);
iolatency_clear_scaling(blkg);
}
-static void iolatency_pd_free(struct blkg_policy_data *pd)
+static void iolat_release(struct rcu_head *rcu)
{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
struct iolatency_grp *iolat = pd_to_lat(pd);
+
free_percpu(iolat->stats);
kfree(iolat);
}
+static void iolatency_pd_free(struct blkg_policy_data *pd)
+{
+ call_rcu(&pd->rcu_head, iolat_release);
+}
+
static struct cftype iolatency_files[] = {
{
.name = "latency",
.flags = CFTYPE_NOT_ON_ROOT,
.seq_show = iolatency_print_limit,
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index cabf91f0d0dc..0f89fb03cdb6 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -351,20 +351,29 @@ static void throtl_pd_online(struct blkg_policy_data *pd)
* Update has_rules[] after a new group is brought online.
*/
tg_update_has_rules(tg);
}
-static void throtl_pd_free(struct blkg_policy_data *pd)
+static void tg_release(struct rcu_head *rcu)
{
+ struct blkg_policy_data *pd =
+ container_of(rcu, struct blkg_policy_data, rcu_head);
struct throtl_grp *tg = pd_to_tg(pd);
- timer_delete_sync(&tg->service_queue.pending_timer);
blkg_rwstat_exit(&tg->stat_bytes);
blkg_rwstat_exit(&tg->stat_ios);
kfree(tg);
}
+static void throtl_pd_free(struct blkg_policy_data *pd)
+{
+ struct throtl_grp *tg = pd_to_tg(pd);
+
+ timer_delete_sync(&tg->service_queue.pending_timer);
+ call_rcu(&pd->rcu_head, tg_release);
+}
+
static struct throtl_grp *
throtl_rb_first(struct throtl_service_queue *parent_sq)
{
struct rb_node *n;
--
2.51.0
^ permalink raw reply related
* [PATCH 1/8] blk-cgroup: protect iterating blkgs with blkcg->lock in blkcg_print_stat()
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
In-Reply-To: <cover.1780621988.git.yukuai@fygo.io>
From: Yu Kuai <yukuai@fygo.io>
blkcg_print_one_stat() will be called for each blkg:
- access blkg->iostat, which is freed from rcu callback
blkg_free_workfn();
- access policy data from pd_stat_fn(), which is freed from
pd_free_fn(), while pd_free_fn() can be called by removing blkcg or
deactivating policy;
Take blkcg->lock while iterating so the blkgs stay online and both
blkg->iostat and policy data for activated policies stay valid. Use
irq-safe locking because blkcg->lock can be nested under q->queue_lock,
which is used from IRQ completion paths.
Prepare to convert protecting blkgs from request_queue with mutex.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index c75b2a103bbc..b55c43f72bcb 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1241,17 +1241,14 @@ static int blkcg_print_stat(struct seq_file *sf, void *v)
if (!seq_css(sf)->parent)
blkcg_fill_root_iostats();
else
css_rstat_flush(&blkcg->css);
- rcu_read_lock();
- hlist_for_each_entry_rcu(blkg, &blkcg->blkg_list, blkcg_node) {
- spin_lock_irq(&blkg->q->queue_lock);
+ guard(spinlock_irq)(&blkcg->lock);
+ hlist_for_each_entry(blkg, &blkcg->blkg_list, blkcg_node)
blkcg_print_one_stat(blkg, sf);
- spin_unlock_irq(&blkg->q->queue_lock);
- }
- rcu_read_unlock();
+
return 0;
}
static struct cftype blkcg_files[] = {
{
--
2.51.0
^ permalink raw reply related
* [PATCH 0/8] blk-cgroup: remove queue_lock nesting from blkcg paths
From: Yu Kuai @ 2026-06-08 3:42 UTC (permalink / raw)
To: nilay, tom.leiming, bvanassche, tj, josef, axboe, yukuai
Cc: akpm, chrisl, kasong, shikemeng, nphamcs, bhe, baohua,
youngjun.park, cgroups, linux-block, linux-kernel, linux-mm
From: Yu Kuai <yukuai@fygo.io>
Hi,
This series is the follow-up blk-cgroup locking cleanup on top of the
earlier blkg-list protection fixes, and prepares blk-cgroup to stop using
q->queue_lock as the global blkg lifetime/iteration lock.
The current queue_lock based protection is hard to maintain because
queue_lock is used from hardirq and softirq completion paths, while some
blkcg cgroup file paths also need to iterate blkgs, print policy data, or
create blkgs from RCU-protected contexts. This series first tightens the
blkcg-side lifetime rules:
- blkcg_print_stat() iterates blkgs under blkcg->lock with IRQs disabled.
- policy data freeing is delayed past an RCU grace period.
- blkcg_print_blkgs(), blkg lookup/create, bio association, page-IO
association, blkg destruction, and BFQ initialization stop nesting
queue_lock under RCU or blkcg->lock.
Using blkcg->lock and RCU for blkcg-owned lists/data keeps the lock order
local to blk-cgroup and avoids extending queue_lock into cgroup file
iteration paths. It also makes the subsequent conversion to q->blkcg_mutex
possible without carrying forward queue_lock's interrupt-context
constraints.
Yu Kuai (8):
blk-cgroup: protect iterating blkgs with blkcg->lock in
blkcg_print_stat()
blk-cgroup: delay freeing policy data after rcu grace period
blk-cgroup: don't nest queue_lock under rcu in blkcg_print_blkgs()
blk-cgroup: don't nest queue_lock under rcu in blkg_lookup_create()
blk-cgroup: don't nest queue_lock under rcu in bio_associate_blkg()
blk-cgroup: don't nest queue_lock under blkcg->lock in
blkcg_destroy_blkgs()
mm/page_io: don't nest queue_lock under rcu in
bio_associate_blkg_from_page()
block, bfq: don't grab queue_lock to initialize bfq
block/bfq-cgroup.c | 17 ++++-
block/bfq-iosched.c | 5 --
block/blk-cgroup-rwstat.c | 15 ++--
block/blk-cgroup.c | 151 ++++++++++++++++++++++----------------
block/blk-cgroup.h | 8 +-
block/blk-iocost.c | 22 ++++--
block/blk-iolatency.c | 10 ++-
block/blk-throttle.c | 13 +++-
mm/page_io.c | 7 +-
9 files changed, 158 insertions(+), 90 deletions(-)
base-commit: b23df513de562739af61fa61ba80ef5e8059a636
--
2.51.0
^ permalink raw reply
* Re: [PATCH 2/5] bfq: protect q->blkg_list iteration in bfq_end_wr_async() with blkcg_mutex
From: yu kuai @ 2026-06-08 2:48 UTC (permalink / raw)
To: Nilay Shroff, Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Bart Van Assche, linux-block,
cgroups, linux-kernel, yukuai
In-Reply-To: <a532857a-16d5-4bef-bbd1-3bc080363182@linux.ibm.com>
Hi,
在 2026/6/5 1:31, Nilay Shroff 写道:
> 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.
Yes, this is done in another huge patchset, because currently blkg_create()
is protected by queue_lock and can be called under rcu, code refactor will
be required.
I'll send the first set soon.
>
> Thanks,
> --Nilay
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] cgroup/dmem: accept only one region per limit write
From: Tejun Heo @ 2026-06-07 7:22 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Natalie Vock, Eric Chanudet, Maxime Ripard, Johannes Weiner,
Michal Koutný, cgroups, dri-devel, linux-kernel,
Albert Esteve
In-Reply-To: <f00e7771-cd70-4c86-9fac-149897e02b12@lankhorst.se>
On Sat, Jun 06, 2026 at 11:44:10PM +0200, Maarten Lankhorst wrote:
> > As I see it, we could go down one of two paths:
> > 1. We go ahead with the patch as proposed, and I make sure that the users I know of adapt. Could be a bit icky wrt. "do not break userspace" rules, but since the already use non-merged UAPIs in one place, you can argue that these users kind of have to expect breakage.
> > 2. We use the old handling allowing multiple lines for dmem.min and dmem.low only. This preserves compatibility but uglifies the code by quite a bit.
> >
> > All things considered, I think I personally would prefer going with 1. and taking the patch as proposed and just having one codepath handling every limit file. Just highlighting this so we don't do it on accident.
> >
> > [1] https://patchwork.freedesktop.org/series/163183/
> >
>
> I prefer option 1 as well, but would like an ack from one of the core cgroup maintainers too,
> and what Maxime's opinion on this as well.
Yeah, if at all possible, please drop the multi region write support if at
all possible. This shouldn't have gotten in and yeah it's on the boundary
but the fact that users need external patches works in our favor - both
qualitatively and quantatively (the usage is likely tiny).
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH-next v6 1/6] cgroup/cpuset: Fix node inconsistencies between cpuset_update_tasks_nodemask() and cpuset_attach()
From: Ridong Chen @ 2026-06-07 3:29 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang
In-Reply-To: <e9257ecd-00b2-4767-838e-b335c6df6a39@redhat.com>
On 6/6/2026 1:09 AM, Waiman Long wrote:
> On 6/5/26 3:48 AM, Ridong Chen wrote:
>>
>> On 6/4/2026 11:02 PM, Waiman Long wrote:
>>> Whenever memory node mask is changed, there are 4 places where the node
>>> mask has to be updated or used.
>>> 1) task's node mask via cpuset_change_task_nodemask()
>>> 2) memory policy binding via mpol_rebind_mm()
>>> 3) if memory migration is enabled, migrate from old_mems_allowed to
>>> the new node mask via cpuset_migrate_mm().
>>> 4) setting old_mems_allowed
>>>
>>> These memory actions are done in cpuset_update_tasks_nodemask() and
>>> cpuset_attach(). However there are inconsistencies in what node masks
>>> are being used in these 2 functions.
>>>
>>> In cpuset_update_tasks_nodemask(),
>>> - cpuset_change_task_nodemask(): guarantee_online_mems()
>>> - mpol_rebind_mm(): mems_allowed
>>> - cpuset_migrate_mm(): guarantee_online_mems()
>>> - old_mems_allowed: guarantee_online_mems()
>>>
>>> In cpuset_attach(),
>>> - cpuset_change_task_nodemask(): guarantee_online_mems()
>>> - mpol_rebind_mm(): effective_mems
>>> - cpuset_migrate_mm(): effective_mems
>>> - old_mems_allowed: effective_mems
>>>
>>> These inconsistencies dates back to quite a long time ago and it is
>>> hard to say what should be the correct values.
>>>
>>> The guarantee_online_mems() function returns a node mask from current or
>>> an ancestor cpuset that is a subset of node_states[N_MEMORY]. Nodes in
>>> node_states[N_MEMORY] are all online, i.e. in node_states[N_ONLINE].
>>> However, node in node_states[N_ONLINE] may not have memory. So
>>> node_states[N_MEMORY] should be a subset of node_states[N_ONLINE].
>>>
>>> The guarantee_online_mems() function should only be useful for v1 where
>>> mems_allowed is the same as effective_mems. With v2, the memory nodes
>>> in effective_mems should always be a subset of node_states[N_MEMORY].
>>> The only time that may not be true is when a memory hot-unplug operation
>>> is in progress and a memory node is removed from node_states[N_MEMORY]
>>> but not yet reflected in effective_mems as cpuset_handle_hotplug()
>>> has not yet been called from cpuset_track_online_nodes(). When
>>> cpuset_handle_hotplug() is called later, the memory node setting
>>> of the relevant cpusets and tasks will be updated. So replacing the
>>> guarantee_online_mems() call by just using cs->effective_mems should
>>> be fine.
>>>
>> The message should be updated.
> Could you be more specific about what message are you referring to?
After this patch, guarantee_online_mems() for v2 simply returns
cs->effective_mems directly. So the paragraph:
"The guarantee_online_mems() function should only be useful for v1
where mems_allowed is the same as effective_mems."
is no longer accurate — guarantee_online_mems() is now useful for
both v1 and v2, just with different behavior. For v2 it returns
effective_mems directly; for v1 it walks up the hierarchy to find
an ancestor with online memory nodes.
Similarly, "So replacing the guarantee_online_mems() call by just
using cs->effective_mems should be fine" describes the old reasoning,
but the patch actually keeps guarantee_online_mems() calls in place
and makes the function itself return effective_mems for v2. The commit
message should reflect what the patch actually does.
>>
>>> 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);
>>> -
>> Nit.
>>
>> I prefer:
>>
>> guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
>> if (cpuset_v2() && !cpus_updated && !mems_updated)
>> goto out;
>
> This is fixed in patch 7. I may have to send a new version of the patch
> 7 integrated back into individual patches.
>
> Cheers,
> Longman
>
>>> 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
>> Other than that, looks good to me.
>>
>> Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
>>
>
--
Best regards,
Ridong
^ permalink raw reply
* Re: [PATCH] cgroup/cpuset: Support multiple source/destination cpusets using pids pattern
From: Ridong Chen @ 2026-06-07 3:22 UTC (permalink / raw)
To: Waiman Long; +Cc: cgroups, Tejun Heo, Johannes Weiner, linux-kernel
In-Reply-To: <110456d5-7164-4032-ae4f-81a97ed96504@redhat.com>
On 6/6/2026 1:15 AM, Waiman Long wrote:
> On 6/5/26 3:35 AM, Ridong Chen wrote:
>>
>> On 6/4/2026 2:47 AM, Waiman Long wrote:
>>> On 6/3/26 6:26 AM, Ridong Chen wrote:
>>>> The current cpuset_can_attach() and cpuset_attach() functions assume
>>>> task
>>>> migration is from one source cpuset to one destination cpuset. This
>>>> can be
>>>> wrong in several scenarios:
>>>> - Moving a multi-threaded process with threads in different cpusets
>>>> - Disabling the cpuset controller (many children to one parent)
>>>> - Enabling the cpuset controller (one parent to many children)
>>>>
>>>> Fix this by adopting the pids subsystem's per-task accounting pattern.
>>>> In cpuset_can_attach(), use task_cs(task) to get the correct source
>>>> cpuset
>>>> for each task (like pids_can_attach uses task_css), adjust
>>>> nr_deadline_tasks
>>>> and reserve DL bandwidth per-task, and increment attach_in_progress
>>>> per-task
>>>> on the destination cpuset. In cpuset_attach(), handle destination
>>>> cpuset
>>>> changes within the task iteration loop.
>>>>
>>>> A shared helper cpuset_undo_attach() reverses the per-task
>>>> operations for
>>>> both partial rollback in cpuset_can_attach() and full reversal in
>>>> cpuset_cancel_attach().
>>>>
>>>> When multiple source cpusets are detected in can_attach(), set
>>>> attach_many_sources so that cpuset_attach() forces cpus_updated and
>>>> mems_updated to true, ensuring all tasks get properly updated
>>>> regardless
>>>> of which source cpuset cpuset_attach_old_cs points to.
>>>>
>>>> This eliminates the need for nr_migrate_dl_tasks, sum_migrate_dl_bw,
>>>> and
>>>> dl_bw_cpu fields in struct cpuset.
>>>>
>>>> Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default
>>>> hierarchy")
>>>> Signed-off-by: Ridong Chen <ridong.chen@linux.dev>
>>> It is not a problem doing per-task DL BW allocation and eliminating the
>>> *dl_bw* fields. However, updating nr_deadline_tasks before it is
>>> committed can be problematic.
>>>
>> Good to hear that.
>>
>>> nr_deadline_tasks is used in dl_rebuild_rd_accounting() which is called
>>> by partition_sched_domains_locked(). After the release of cpuset_mutex
>>> at the end of cpuset_can_attach() and before cpuset_attach() or
>>> cpuset_cancel_attach() is called, it is possible
>>> that partition_sched_domains_locked() can be called
>>> and dl_rebuild_rd_accounting() is not getting the right DL BW accounting
>>> information. So unless there is a way to confirm that this situation
>>> cannot happen, we can't change nr_deadline_tasks before the attach is
>>> commited.
>>>
>> We can keep the nr_migrate_dl_tasks field and update nr_deadline_tasks
>> once migration is complete. I think this will be much simpler than
>> fixing the issue using lists.
>>
> But we still need to track the set of source and destination cpusets to
> commit or cancel the change. Doing it task-by-task will add code in the
> cpuset_attach() and cpuset_cancel_attach() to check if a task is a DL
> task and act accordingly. So we are just trading task-by-task code with
> code to handle the lists.
>
I resend a patch [1] that keeps nr_migrate_dl_tasks but eliminates
sum_migrate_dl_bw, dl_bw_cpu, attach_node, and attach_llist_head from
the cpuset structure by doing per-task dl_bw_alloc directly in
cpuset_can_attach().
I just offer a way to discuss whether we can solve this issue in a
simpler way.
[1]
https://lore.kernel.org/cgroups/20260602023203.248077-1-longman@redhat.com/T/#mb2c6a3ae44f34f571db5dffa888212eaeaaea17a
--
Best regards,
Ridong
^ permalink raw reply
* [PATCH] cgroup/cpuset: Support multiple source/destination cpusets using pids pattern
From: Ridong Chen @ 2026-06-07 3:12 UTC (permalink / raw)
To: Waiman Long
Cc: cgroups, Tejun Heo, Ridong Chen, Johannes Weiner, linux-kernel
In-Reply-To: <110456d5-7164-4032-ae4f-81a97ed96504@redhat.com>
When the cpuset controller is enabled/disabled in a parent cgroup, tasks
from multiple child cpusets need to be migrated. The current code only
handles a single source/destination pair.
Support multiple source/destination cpusets by adopting the per-task
processing pattern similar to the pids controller:
1) Perform per-task DL bandwidth reservation (dl_bw_alloc) directly in
cpuset_can_attach() instead of batching into sum_migrate_dl_bw. This
eliminates the sum_migrate_dl_bw and dl_bw_cpu fields from the cpuset
struct.
2) Track attach_in_progress per-task per-destination cpuset to properly
guard all involved cpusets from having their cpus/mems zeroed.
3) Use a shared cpuset_undo_attach() helper for both rollback-on-error
in cpuset_can_attach() and for cpuset_cancel_attach().
4) Detect many-source migrations and force cpus_updated/mems_updated
to true so all tasks get properly updated during attach.
5) Defer nr_deadline_tasks updates to cpuset_attach() (after migration
is committed) to avoid a race with dl_rebuild_rd_accounting() that
could see inconsistent values between can_attach and attach.
Signed-off-by: Ridong Chen <ridong.chen@linux.dev>
---
kernel/cgroup/cpuset-internal.h | 7 --
kernel/cgroup/cpuset.c | 167 ++++++++++++++++----------------
2 files changed, 84 insertions(+), 90 deletions(-)
diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index f7aaf01f7cd5..8f32cb97eb94 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -167,13 +167,6 @@ struct cpuset {
*/
int nr_deadline_tasks;
int nr_migrate_dl_tasks;
- /* DL bandwidth that needs destination reservation for this attach. */
- u64 sum_migrate_dl_bw;
- /*
- * CPU used for temporary DL bandwidth allocation during attach;
- * -1 if no DL bandwidth was allocated in the current attach.
- */
- int dl_bw_cpu;
/* Invalid partition error code, not lock protected */
enum prs_errcode prs_err;
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e52a5a40d607..a6d96a39cdb1 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -288,7 +288,6 @@ struct cpuset top_cpuset = {
.flags = BIT(CS_CPU_EXCLUSIVE) |
BIT(CS_MEM_EXCLUSIVE) | BIT(CS_SCHED_LOAD_BALANCE),
.partition_root_state = PRS_ROOT,
- .dl_bw_cpu = -1,
};
/**
@@ -580,8 +579,6 @@ static struct cpuset *dup_or_alloc_cpuset(struct cpuset *cs)
if (!trial)
return NULL;
- trial->dl_bw_cpu = -1;
-
/* Setup cpumask pointer array */
cpumask_var_t *pmask[4] = {
&trial->cpus_allowed,
@@ -3026,31 +3023,36 @@ static int cpuset_can_attach_check(struct cpuset *cs, struct cpuset *oldcs,
return 0;
}
-static int cpuset_reserve_dl_bw(struct cpuset *cs)
+/*
+ * Undo DL bandwidth reservations and attach_in_progress increments done
+ * in cpuset_can_attach(). Used for both rollback on error and cancel_attach.
+ * If @stop_at is non-NULL, undo only for tasks before @stop_at in the tset.
+ */
+static void cpuset_undo_attach(struct cgroup_taskset *tset,
+ struct task_struct *stop_at)
{
- int cpu, ret;
-
- if (!cs->sum_migrate_dl_bw)
- return 0;
+ struct cgroup_subsys_state *css;
+ struct task_struct *task;
- cpu = cpumask_any_and(cpu_active_mask, cs->effective_cpus);
- if (unlikely(cpu >= nr_cpu_ids))
- return -EINVAL;
+ cgroup_taskset_for_each(task, css, tset) {
+ struct cpuset *cs = css_cs(css);
- ret = dl_bw_alloc(cpu, cs->sum_migrate_dl_bw);
- if (ret)
- return ret;
+ if (task == stop_at)
+ break;
- cs->dl_bw_cpu = cpu;
- return 0;
+ if (dl_task(task)) {
+ cs->nr_migrate_dl_tasks--;
+ if (dl_task_needs_bw_move(task, cs->effective_cpus)) {
+ int cpu = cpumask_any_and(cpu_active_mask,
+ cs->effective_cpus);
+ dl_bw_free(cpu, task->dl.dl_bw);
+ }
+ }
+ dec_attach_in_progress_locked(cs);
+ }
}
-static void reset_migrate_dl_data(struct cpuset *cs)
-{
- cs->nr_migrate_dl_tasks = 0;
- cs->sum_migrate_dl_bw = 0;
- cs->dl_bw_cpu = -1;
-}
+static bool attach_many_sources;
/* Called by cgroups to determine if a cpuset is usable; cpuset_mutex held */
static int cpuset_can_attach(struct cgroup_taskset *tset)
@@ -3067,90 +3069,73 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
cs = css_cs(css);
mutex_lock(&cpuset_mutex);
+ attach_many_sources = false;
/* Check to see if task is allowed in the cpuset */
ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
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) {
+ struct cpuset *newcs = css_cs(css);
+ struct cpuset *new_oldcs = task_cs(task);
+
+ if (newcs != cs || new_oldcs != oldcs) {
+ if (new_oldcs != oldcs)
+ attach_many_sources = true;
+ cs = newcs;
+ oldcs = new_oldcs;
+ ret = cpuset_can_attach_check(cs, oldcs,
+ &setsched_check);
+ if (ret)
+ goto out_rollback;
+ }
+
ret = task_can_attach(task);
if (ret)
- goto out_unlock;
+ goto out_rollback;
- /* 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)
- goto out_unlock;
+ goto out_rollback;
}
if (dl_task(task)) {
- /*
- * Count all migrating DL tasks for cpuset task accounting.
- * Only tasks that need a root-domain bandwidth move
- * contribute to sum_migrate_dl_bw.
- */
cs->nr_migrate_dl_tasks++;
- if (dl_task_needs_bw_move(task, cs->effective_cpus))
- cs->sum_migrate_dl_bw += task->dl.dl_bw;
+ if (dl_task_needs_bw_move(task, cs->effective_cpus)) {
+ int cpu = cpumask_any_and(cpu_active_mask,
+ cs->effective_cpus);
+ if (unlikely(cpu >= nr_cpu_ids)) {
+ ret = -EINVAL;
+ goto out_rollback;
+ }
+ ret = dl_bw_alloc(cpu, task->dl.dl_bw);
+ if (ret)
+ goto out_rollback;
+ }
}
- }
-
- ret = cpuset_reserve_dl_bw(cs);
-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++;
}
+ goto out_unlock;
+
+out_rollback:
+ cpuset_undo_attach(tset, task);
+
+out_unlock:
mutex_unlock(&cpuset_mutex);
return ret;
}
static void cpuset_cancel_attach(struct cgroup_taskset *tset)
{
- struct cgroup_subsys_state *css;
- struct cpuset *cs;
-
- cgroup_taskset_first(tset, &css);
- 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);
-
+ cpuset_undo_attach(tset, NULL);
mutex_unlock(&cpuset_mutex);
}
@@ -3232,8 +3217,15 @@ static void cpuset_attach(struct cgroup_taskset *tset)
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);
+ if (attach_many_sources) {
+ attach_cpus_updated = true;
+ attach_mems_updated = true;
+ } else {
+ 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
@@ -3250,20 +3242,29 @@ static void cpuset_attach(struct cgroup_taskset *tset)
}
cgroup_taskset_for_each(task, css, tset)
- cpuset_attach_task(cs, task);
+ cpuset_attach_task(css_cs(css), 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);
- }
+ /*
+ * Update nr_deadline_tasks now that migration is committed.
+ * nr_migrate_dl_tasks was accumulated per-dst in can_attach but
+ * nr_deadline_tasks is deferred to here to avoid a race with
+ * dl_rebuild_rd_accounting() between can_attach and attach.
+ */
+ cgroup_taskset_for_each(task, css, tset) {
+ struct cpuset *dst_cs = css_cs(css);
- dec_attach_in_progress_locked(cs);
+ if (dst_cs->nr_migrate_dl_tasks) {
+ dst_cs->nr_deadline_tasks += dst_cs->nr_migrate_dl_tasks;
+ oldcs->nr_deadline_tasks -= dst_cs->nr_migrate_dl_tasks;
+ dst_cs->nr_migrate_dl_tasks = 0;
+ }
+ dec_attach_in_progress_locked(dst_cs);
+ }
mutex_unlock(&cpuset_mutex);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] cgroup/dmem: accept only one region per limit write
From: Maarten Lankhorst @ 2026-06-06 21:44 UTC (permalink / raw)
To: Natalie Vock, Eric Chanudet, Maxime Ripard, Tejun Heo,
Johannes Weiner, Michal Koutný
Cc: cgroups, dri-devel, linux-kernel, Albert Esteve
In-Reply-To: <271b1c16-3c3c-4a1e-b09e-c4361c63814c@gmx.de>
Hey,
On 6/6/26 18:31, Natalie Vock wrote:
> On 6/6/26 00:44, Eric Chanudet wrote:
>> Accept only one "region value" pair entry for the dmem.max, dmem.min,
>> dmem.low files.>
>> This changes the UAPI that otherwise accepted multiple lines for setting
>> multiple entries in one write. No existing user is known to rely on
>> writing multiple regions in a single write.
>
> Ugh, shoot.
>
> For dmem.low specifically, there already are some userspace thingies floating around that may write more than one region/value pairs.
>
> These thingies all depend on that one patchset for dmemcg protection that I should really get around to merging[1]. Since the userspace utilities depend on not-yet-merged patches, they sort of have to expect stuff changing under their belts, so I wouldn't really consider those users a blocker by necessity.
>
> As I see it, we could go down one of two paths:
> 1. We go ahead with the patch as proposed, and I make sure that the users I know of adapt. Could be a bit icky wrt. "do not break userspace" rules, but since the already use non-merged UAPIs in one place, you can argue that these users kind of have to expect breakage.
> 2. We use the old handling allowing multiple lines for dmem.min and dmem.low only. This preserves compatibility but uglifies the code by quite a bit.
>
> All things considered, I think I personally would prefer going with 1. and taking the patch as proposed and just having one codepath handling every limit file. Just highlighting this so we don't do it on accident.
>
> [1] https://patchwork.freedesktop.org/series/163183/
>
I prefer option 1 as well, but would like an ack from one of the core cgroup maintainers too,
and what Maxime's opinion on this as well.
Kind regards,
~Maarten Lankhorst
> Some more review comments inline.
>
>>
>> Processing multiple regions in dmemcg_limit_write() could quietly change
>> first limits before failing on a later one and returning an error to the
>> writer, with no indication some changes occurred.
>>
>> Signed-off-by: Eric Chanudet <echanude@redhat.com>
>> ---
>> Follow up from discussions on a previous thread[1].
>> If Albert's series[2] lands, I can cleanup and prepare some kunits for
>> these as well.
>> [1] https://lore.kernel.org/all/158bc103-7f99-4df4-8d3b-2da9b04ac0ed@lankhorst.se/
>> [2] https://lore.kernel.org/all/20260519-kunit_cgroups-v4-1-f6c2f498fae4@redhat.com/
>> ---
>> kernel/cgroup/dmem.c | 70 +++++++++++++++++++---------------------------------
>> 1 file changed, 26 insertions(+), 44 deletions(-)
>>
>> diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
>> index 6430c7ce1e0372f59f1313163fb7630ce49ac1ef..113ee88e276296bccb4def546adf5cc175d7f0be 100644
>> --- a/kernel/cgroup/dmem.c
>> +++ b/kernel/cgroup/dmem.c
>> @@ -734,57 +734,39 @@ static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
>> void (*apply)(struct dmem_cgroup_pool_state *, u64))
>> {
>> struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
>> - int err = 0;
>> -
>> - while (buf && !err) {
>> - struct dmem_cgroup_pool_state *pool = NULL;
>> - char *options, *region_name;
>> - struct dmem_cgroup_region *region;
>> - u64 new_limit;
>> -
>> - options = buf;
>> - buf = strchr(buf, '\n');
>> - if (buf)
>> - *buf++ = '\0';
>> -
>> - options = strstrip(options);
>> -
>> - /* eat empty lines */
>> - if (!options[0])
>> - continue;
>> -
>> - region_name = strsep(&options, " \t");
>> - if (!region_name[0])
>> - continue;
>> -
>> - if (!options || !*options)
>> - return -EINVAL;
>> + struct dmem_cgroup_pool_state *pool;
>> + struct dmem_cgroup_region *region;
>> + char *region_name;
>> + u64 new_limit;
>> + int err;
>> - rcu_read_lock();
>> - region = dmemcg_get_region_by_name(region_name);
>> - rcu_read_unlock();
>> + buf = strstrip(buf);
>> + region_name = strsep(&buf, " \t");
>> + if (!region_name[0] || !buf)
>
> If buf is NULL, isn't strsep(&buf, ...) also NULL? region_name[0] would therefore be a NULL pointer deref. Flipping the order of the logical or should be enough to prevent this.
>
>> + return -EINVAL;
>> - if (!region)
>> - return -EINVAL;
>> + rcu_read_lock();
>> + region = dmemcg_get_region_by_name(region_name);
>> + rcu_read_unlock();
>> + if (!region)
>> + return -EINVAL;
>> - err = dmemcg_parse_limit(options, &new_limit);
>> - if (err < 0)
>> - goto out_put;
>> + buf = strstrip(buf);
>
> Do we start allowing extra spaces between region and limit as well? Would also be fine by me, it doesn't break anything, just highlighting that it's a change in behavior. Should perhaps be documented in the commit message, too.
>
> Also, you should be able to use skip_spaces() here for an equivalent result. I'm not strongly opinionated on either way, but using skip_spaces() indicates more clearly that this can only remove spaces at the start.
>
> Best,
> Natalie
>
>> + err = dmemcg_parse_limit(buf, &new_limit);
>> + if (err < 0)
>> + goto out_put;
>> - pool = get_cg_pool_unlocked(dmemcs, region);
>> - if (IS_ERR(pool)) {
>> - err = PTR_ERR(pool);
>> - goto out_put;
>> - }
>> + pool = get_cg_pool_unlocked(dmemcs, region);
>> + if (IS_ERR(pool)) {
>> + err = PTR_ERR(pool);
>> + goto out_put;
>> + }
>> - /* And commit */
>> - apply(pool, new_limit);
>> - dmemcg_pool_put(pool);
>> + apply(pool, new_limit);
>> + dmemcg_pool_put(pool);
>> out_put:
>> - kref_put(®ion->ref, dmemcg_free_region);
>> - }
>> -
>> + kref_put(®ion->ref, dmemcg_free_region);
>> return err ?: nbytes;
>> }
>>
>> ---
>> base-commit: 640c57d6ca1346a1c2363a3f473b405af979e046
>> change-id: 20260605-cgroup-dmem-write-single-region-9bf05b6d995d
>>
>> Best regards,
>
^ permalink raw reply
* Re: [PATCH] cgroup/dmem: accept only one region per limit write
From: Natalie Vock @ 2026-06-06 16:31 UTC (permalink / raw)
To: Eric Chanudet, Maarten Lankhorst, Maxime Ripard, Tejun Heo,
Johannes Weiner, Michal Koutný
Cc: cgroups, dri-devel, linux-kernel, Albert Esteve
In-Reply-To: <20260605-cgroup-dmem-write-single-region-v1-1-9137f296579c@redhat.com>
On 6/6/26 00:44, Eric Chanudet wrote:
> Accept only one "region value" pair entry for the dmem.max, dmem.min,
> dmem.low files.>
> This changes the UAPI that otherwise accepted multiple lines for setting
> multiple entries in one write. No existing user is known to rely on
> writing multiple regions in a single write.
Ugh, shoot.
For dmem.low specifically, there already are some userspace thingies
floating around that may write more than one region/value pairs.
These thingies all depend on that one patchset for dmemcg protection
that I should really get around to merging[1]. Since the userspace
utilities depend on not-yet-merged patches, they sort of have to expect
stuff changing under their belts, so I wouldn't really consider those
users a blocker by necessity.
As I see it, we could go down one of two paths:
1. We go ahead with the patch as proposed, and I make sure that the
users I know of adapt. Could be a bit icky wrt. "do not break userspace"
rules, but since the already use non-merged UAPIs in one place, you can
argue that these users kind of have to expect breakage.
2. We use the old handling allowing multiple lines for dmem.min and
dmem.low only. This preserves compatibility but uglifies the code by
quite a bit.
All things considered, I think I personally would prefer going with 1.
and taking the patch as proposed and just having one codepath handling
every limit file. Just highlighting this so we don't do it on accident.
[1] https://patchwork.freedesktop.org/series/163183/
Some more review comments inline.
>
> Processing multiple regions in dmemcg_limit_write() could quietly change
> first limits before failing on a later one and returning an error to the
> writer, with no indication some changes occurred.
>
> Signed-off-by: Eric Chanudet <echanude@redhat.com>
> ---
> Follow up from discussions on a previous thread[1].
> If Albert's series[2] lands, I can cleanup and prepare some kunits for
> these as well.
> [1] https://lore.kernel.org/all/158bc103-7f99-4df4-8d3b-2da9b04ac0ed@lankhorst.se/
> [2] https://lore.kernel.org/all/20260519-kunit_cgroups-v4-1-f6c2f498fae4@redhat.com/
> ---
> kernel/cgroup/dmem.c | 70 +++++++++++++++++++---------------------------------
> 1 file changed, 26 insertions(+), 44 deletions(-)
>
> diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
> index 6430c7ce1e0372f59f1313163fb7630ce49ac1ef..113ee88e276296bccb4def546adf5cc175d7f0be 100644
> --- a/kernel/cgroup/dmem.c
> +++ b/kernel/cgroup/dmem.c
> @@ -734,57 +734,39 @@ static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
> void (*apply)(struct dmem_cgroup_pool_state *, u64))
> {
> struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
> - int err = 0;
> -
> - while (buf && !err) {
> - struct dmem_cgroup_pool_state *pool = NULL;
> - char *options, *region_name;
> - struct dmem_cgroup_region *region;
> - u64 new_limit;
> -
> - options = buf;
> - buf = strchr(buf, '\n');
> - if (buf)
> - *buf++ = '\0';
> -
> - options = strstrip(options);
> -
> - /* eat empty lines */
> - if (!options[0])
> - continue;
> -
> - region_name = strsep(&options, " \t");
> - if (!region_name[0])
> - continue;
> -
> - if (!options || !*options)
> - return -EINVAL;
> + struct dmem_cgroup_pool_state *pool;
> + struct dmem_cgroup_region *region;
> + char *region_name;
> + u64 new_limit;
> + int err;
>
> - rcu_read_lock();
> - region = dmemcg_get_region_by_name(region_name);
> - rcu_read_unlock();
> + buf = strstrip(buf);
> + region_name = strsep(&buf, " \t");
> + if (!region_name[0] || !buf)
If buf is NULL, isn't strsep(&buf, ...) also NULL? region_name[0] would
therefore be a NULL pointer deref. Flipping the order of the logical or
should be enough to prevent this.
> + return -EINVAL;
>
> - if (!region)
> - return -EINVAL;
> + rcu_read_lock();
> + region = dmemcg_get_region_by_name(region_name);
> + rcu_read_unlock();
> + if (!region)
> + return -EINVAL;
>
> - err = dmemcg_parse_limit(options, &new_limit);
> - if (err < 0)
> - goto out_put;
> + buf = strstrip(buf);
Do we start allowing extra spaces between region and limit as well?
Would also be fine by me, it doesn't break anything, just highlighting
that it's a change in behavior. Should perhaps be documented in the
commit message, too.
Also, you should be able to use skip_spaces() here for an equivalent
result. I'm not strongly opinionated on either way, but using
skip_spaces() indicates more clearly that this can only remove spaces at
the start.
Best,
Natalie
> + err = dmemcg_parse_limit(buf, &new_limit);
> + if (err < 0)
> + goto out_put;
>
> - pool = get_cg_pool_unlocked(dmemcs, region);
> - if (IS_ERR(pool)) {
> - err = PTR_ERR(pool);
> - goto out_put;
> - }
> + pool = get_cg_pool_unlocked(dmemcs, region);
> + if (IS_ERR(pool)) {
> + err = PTR_ERR(pool);
> + goto out_put;
> + }
>
> - /* And commit */
> - apply(pool, new_limit);
> - dmemcg_pool_put(pool);
> + apply(pool, new_limit);
> + dmemcg_pool_put(pool);
>
> out_put:
> - kref_put(®ion->ref, dmemcg_free_region);
> - }
> -
> + kref_put(®ion->ref, dmemcg_free_region);
>
> return err ?: nbytes;
> }
>
> ---
> base-commit: 640c57d6ca1346a1c2363a3f473b405af979e046
> change-id: 20260605-cgroup-dmem-write-single-region-9bf05b6d995d
>
> Best regards,
^ permalink raw reply
* [syzbot] Monthly cgroups report (Jun 2026)
From: syzbot @ 2026-06-06 12:32 UTC (permalink / raw)
To: cgroups, linux-kernel, syzkaller-bugs
Hello cgroups maintainers/developers,
This is a 31-day syzbot report for the cgroups subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/cgroups
During the period, 1 new issues were detected and 0 were fixed.
In total, 3 issues are still open and 49 have already been fixed.
There are also 2 low-priority issues.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 3833 Yes possible deadlock in console_flush_all (4)
https://syzkaller.appspot.com/bug?extid=d10e9d53059eb8aed654
<2> 39 No INFO: task hung in cgroup_subtree_control_write (2)
https://syzkaller.appspot.com/bug?extid=bb2e19a1190a556c01b1
<3> 32 No INFO: rcu detected stall in clone3 (6)
https://syzkaller.appspot.com/bug?extid=774c2dfaebdf78f984c5
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* [syzbot] [cgroups?] INFO: task hung in cgroup_subtree_control_write (2)
From: syzbot @ 2026-06-06 4:40 UTC (permalink / raw)
To: cgroups, hannes, linux-kernel, mkoutny, syzkaller-bugs, tj
Hello,
syzbot found the following issue on:
HEAD commit: f7af91adc230 Add linux-next specific files for 20260528
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=10fba1b6580000
kernel config: https://syzkaller.appspot.com/x/.config?x=8540f3ea107c5da4
dashboard link: https://syzkaller.appspot.com/bug?extid=bb2e19a1190a556c01b1
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/201366128b18/disk-f7af91ad.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/18bd352284cf/vmlinux-f7af91ad.xz
kernel image: https://storage.googleapis.com/syzbot-assets/83c27fa66016/bzImage-f7af91ad.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+bb2e19a1190a556c01b1@syzkaller.appspotmail.com
INFO: task syz.4.3305:20367 blocked for more than 143 seconds.
Tainted: G L syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz.4.3305 state:D stack:28920 pid:20367 tgid:20365 ppid:18600 task_flags:0x400040 flags:0x00080002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5402 [inline]
__schedule+0x16f9/0x5500 kernel/sched/core.c:7204
__schedule_loop kernel/sched/core.c:7283 [inline]
schedule+0x164/0x360 kernel/sched/core.c:7298
cgroup_lock_and_drain_offline+0x516/0x650 kernel/cgroup/cgroup.c:3248
cgroup_kn_lock_live+0x120/0x230 kernel/cgroup/cgroup.c:1699
cgroup_subtree_control_write+0x4b3/0x10a0 kernel/cgroup/cgroup.c:3550
cgroup_file_write+0x331/0x8f0 kernel/cgroup/cgroup.c:4289
kernfs_fop_write_iter+0x3b0/0x540 fs/kernfs/file.c:352
new_sync_write fs/read_write.c:595 [inline]
vfs_write+0x629/0xba0 fs/read_write.c:688
ksys_write+0x156/0x270 fs/read_write.c:740
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7ffad184ce59
RSP: 002b:00007ffacfa7d028 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007ffad1ac6090 RCX: 00007ffad184ce59
RDX: 0000000000000005 RSI: 0000200000000040 RDI: 0000000000000006
RBP: 00007ffad18e2d6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffad1ac6128 R14: 00007ffad1ac6090 R15: 00007ffe4a0d9fe8
</TASK>
Showing all locks held in the system:
1 lock held by khungtaskd/39:
#0: ffffffff8e1cac60 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:300 [inline]
#0: ffffffff8e1cac60 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:838 [inline]
#0: ffffffff8e1cac60 (rcu_read_lock){....}-{1:3}, at: debug_show_all_locks+0x2e/0x180 kernel/locking/lockdep.c:6777
2 locks held by getty/5367:
#0: ffff888037c8b0a0 (&tty->ldisc_sem){++++}-{0:0}, at: tty_ldisc_ref_wait+0x25/0x70 drivers/tty/tty_ldisc.c:243
#1: ffffc90003cc62e0 (&ldata->atomic_read_lock){+.+.}-{4:4}, at: n_tty_read+0x462/0x13a0 drivers/tty/n_tty.c:2211
3 locks held by kworker/0:6/5762:
#0: ffff88813fe16538 ((wq_completion)events_power_efficient){+.+.}-{0:0}, at: process_one_work+0x897/0x1630 kernel/workqueue.c:3293
#1: ffffc9000550fc40 ((gc_work).work){+.+.}-{0:0}, at: process_one_work+0x8be/0x1630 kernel/workqueue.c:3294
#2: ffffffff8ececcf8 ("ratelimiter_table_lock"){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
#2: ffffffff8ececcf8 ("ratelimiter_table_lock"){+.+.}-{3:3}, at: wg_ratelimiter_gc_entries+0x5d/0x480 drivers/net/wireguard/ratelimiter.c:63
2 locks held by kworker/u8:39/16177:
#0: ffff88813fe54138 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x897/0x1630 kernel/workqueue.c:3293
#1: ffffc90007a0fc40 (connector_reaper_work){+.+.}-{0:0}, at: process_one_work+0x8be/0x1630 kernel/workqueue.c:3294
3 locks held by syz.4.3305/20367:
#0: ffff88803b51c328 (&f->f_pos_lock){+.+.}-{4:4}, at: fdget_pos+0x252/0x320 fs/file.c:1260
#1: ffff8880353ea480 (sb_writers#9){.+.+}-{0:0}, at: file_start_write include/linux/fs.h:2733 [inline]
#1: ffff8880353ea480 (sb_writers#9){.+.+}-{0:0}, at: vfs_write+0x22d/0xba0 fs/read_write.c:684
#2: ffff888034fed478 (&of->mutex){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x1df/0x540 fs/kernfs/file.c:343
2 locks held by syz-executor/20485:
#0: ffffffff8e900c38 (tomoyo_ss){.+.+}-{0:0}, at: srcu_lock_acquire include/linux/srcu.h:187 [inline]
#0: ffffffff8e900c38 (tomoyo_ss){.+.+}-{0:0}, at: srcu_read_lock include/linux/srcu.h:294 [inline]
#0: ffffffff8e900c38 (tomoyo_ss){.+.+}-{0:0}, at: tomoyo_read_lock security/tomoyo/common.h:1112 [inline]
#0: ffffffff8e900c38 (tomoyo_ss){.+.+}-{0:0}, at: tomoyo_check_open_permission+0x1d3/0x470 security/tomoyo/file.c:772
#1: ffff88813feaad58 (&n->list_lock){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
#1: ffff88813feaad58 (&n->list_lock){+.+.}-{3:3}, at: get_partial_node_bulk mm/slub.c:3752 [inline]
#1: ffff88813feaad58 (&n->list_lock){+.+.}-{3:3}, at: __refill_objects_node+0x89/0x620 mm/slub.c:7071
1 lock held by syz.6.3865/22401:
#0: ffff888040d99d90 (mapping.invalidate_lock#2){++++}-{4:4}, at: filemap_invalidate_lock_shared include/linux/fs.h:1089 [inline]
#0: ffff888040d99d90 (mapping.invalidate_lock#2){++++}-{4:4}, at: filemap_fault+0xa92/0x1470 mm/filemap.c:3571
5 locks held by syz.7.3869/22407:
#0: ffff88803f089fc8 (&net->xfrm.xfrm_cfg_mutex){+.+.}-{4:4}, at: xfrm_netlink_rcv+0x6a/0x90 net/xfrm/xfrm_user.c:3545
#1: ffff88803de8c928 (nlk_cb_mutex-XFRM){+.+.}-{4:4}, at: __netlink_dump_start+0xfe/0x7e0 net/netlink/af_netlink.c:2410
#2: ffff88813feaad58 (&n->list_lock){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
#2: ffff88813feaad58 (&n->list_lock){+.+.}-{3:3}, at: get_partial_node_bulk mm/slub.c:3752 [inline]
#2: ffff88813feaad58 (&n->list_lock){+.+.}-{3:3}, at: __refill_objects_node+0x89/0x620 mm/slub.c:7071
#3: ffff8880b863b8a0 (&rq->__lock){-...}-{2:2}, at: finish_lock_switch kernel/sched/core.c:5136 [inline]
#3: ffff8880b863b8a0 (&rq->__lock){-...}-{2:2}, at: finish_task_switch+0x15f/0xbe0 kernel/sched/core.c:5257
#4: ffff8880b8628418 (hrtimer_bases.lock){-...}-{2:2}, at: lock_hrtimer_base kernel/time/hrtimer.c:191 [inline]
#4: ffff8880b8628418 (hrtimer_bases.lock){-...}-{2:2}, at: hrtimer_start_range_ns+0x8c/0x3f0 kernel/time/hrtimer.c:1501
=============================================
NMI backtrace for cpu 1
CPU: 1 UID: 0 PID: 39 Comm: khungtaskd Tainted: G L syzkaller #0 PREEMPT_{RT,(full)}
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/09/2026
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
nmi_cpu_backtrace+0x274/0x2d0 lib/nmi_backtrace.c:122
nmi_trigger_cpumask_backtrace+0x17a/0x380 lib/nmi_backtrace.c:65
trigger_all_cpu_backtrace include/linux/nmi.h:162 [inline]
__sys_info lib/sys_info.c:157 [inline]
sys_info+0x135/0x170 lib/sys_info.c:165
check_hung_uninterruptible_tasks kernel/hung_task.c:353 [inline]
watchdog+0xfd3/0x1030 kernel/hung_task.c:561
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Sending NMI from CPU 1 to CPUs 0:
NMI backtrace for cpu 0
CPU: 0 UID: 0 PID: 22407 Comm: syz.7.3869 Tainted: G L syzkaller #0 PREEMPT_{RT,(full)}
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/09/2026
RIP: 0010:__bfs kernel/locking/lockdep.c:1755 [inline]
RIP: 0010:__bfs_backwards kernel/locking/lockdep.c:1862 [inline]
RIP: 0010:check_irq_usage kernel/locking/lockdep.c:2798 [inline]
RIP: 0010:check_prev_add kernel/locking/lockdep.c:3171 [inline]
RIP: 0010:check_prevs_add kernel/locking/lockdep.c:3286 [inline]
RIP: 0010:validate_chain kernel/locking/lockdep.c:3910 [inline]
RIP: 0010:__lock_acquire+0x1821/0x2d10 kernel/locking/lockdep.c:5239
Code: 00 0f 83 8a 01 00 00 4e 8b 24 fd a0 69 a2 95 ff c0 25 ff 0f 00 00 89 05 9d cb 01 14 4d 85 e4 0f 84 b2 01 00 00 49 8b 44 24 10 <48> 85 c0 0f 84 3c 0d 00 00 8b 0d 88 cb 01 14 39 48 5c 0f 84 67 ff
RSP: 0018:ffffc9001429e7e0 EFLAGS: 00000082
RAX: ffffffff934677a8 RBX: 00000000000003cd RCX: ffffffff9657db68
RDX: ffffffff934d1bd8 RSI: ffff888041d1cac8 RDI: 00000000000003cd
RBP: 9e3923e9df32e17b R08: ffffc9001429e7a8 R09: 0000000000000020
R10: ffffc9001429e9e8 R11: ffffffff81a17080 R12: ffffffff9657db68
R13: ffff888041d1cac8 R14: ffff888041d1be00 R15: 000000000000007c
FS: 00007fdb4f43e6c0(0000) GS:ffff888125c7e000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f03764ae3e3 CR3: 000000007a326000 CR4: 00000000003526f0
DR0: 0000000000000009 DR1: 00000000000000c9 DR2: 00000000000000f3
DR3: 0000000000000002 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5870
__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:132 [inline]
_raw_spin_lock_irqsave+0x40/0x60 kernel/locking/spinlock.c:166
debug_object_deactivate+0x9a/0x250 lib/debugobjects.c:900
debug_hrtimer_deactivate kernel/time/hrtimer.c:490 [inline]
remove_and_enqueue_same_base kernel/time/hrtimer.c:1252 [inline]
__hrtimer_start_range_ns kernel/time/hrtimer.c:1402 [inline]
hrtimer_start_range_ns_common+0x3f7/0xba0 kernel/time/hrtimer.c:1481
hrtimer_start_range_ns+0x111/0x3f0 kernel/time/hrtimer.c:1503
hrtimer_start include/linux/hrtimer.h:223 [inline]
hrtick_cond_restart kernel/sched/core.c:933 [inline]
hrtick_schedule_exit kernel/sched/core.c:995 [inline]
finish_lock_switch kernel/sched/core.c:5138 [inline]
finish_task_switch+0x3cd/0xbe0 kernel/sched/core.c:5257
context_switch kernel/sched/core.c:5405 [inline]
__schedule+0x1701/0x5500 kernel/sched/core.c:7204
preempt_schedule_common+0x82/0xd0 kernel/sched/core.c:7385
preempt_schedule_thunk+0x16/0x40 arch/x86/entry/thunk.S:12
class_preempt_destructor include/linux/preempt.h:468 [inline]
raw_spin_unlock_irqrestore_wake include/linux/sched/wake_q.h:102 [inline]
rtlock_slowlock kernel/locking/rtmutex.c:1919 [inline]
rtlock_lock kernel/locking/spinlock_rt.c:43 [inline]
__rt_spin_lock kernel/locking/spinlock_rt.c:49 [inline]
rt_spin_lock+0x316/0x400 kernel/locking/spinlock_rt.c:57
spin_lock include/linux/spinlock_rt.h:45 [inline]
get_partial_node_bulk mm/slub.c:3752 [inline]
__refill_objects_node+0x89/0x620 mm/slub.c:7071
refill_objects+0x62/0x3d0 mm/slub.c:7203
refill_sheaf mm/slub.c:2827 [inline]
__pcs_replace_empty_main+0x373/0x720 mm/slub.c:4665
alloc_from_pcs mm/slub.c:4763 [inline]
slab_alloc_node mm/slub.c:4897 [inline]
__do_kmalloc_node mm/slub.c:5308 [inline]
__kmalloc_node_track_caller_noprof+0x60b/0x7e0 mm/slub.c:5412
kmalloc_reserve net/core/skbuff.c:635 [inline]
__alloc_skb+0x2c1/0x7d0 net/core/skbuff.c:713
alloc_skb include/linux/skbuff.h:1382 [inline]
netlink_dump+0x1d8/0xe10 net/netlink/af_netlink.c:2296
__netlink_dump_start+0x5cb/0x7e0 net/netlink/af_netlink.c:2446
netlink_dump_start include/linux/netlink.h:341 [inline]
xfrm_user_rcv_msg+0x951/0xc40 net/xfrm/xfrm_user.c:3503
netlink_rcv_skb+0x232/0x4b0 net/netlink/af_netlink.c:2556
xfrm_netlink_rcv+0x79/0x90 net/xfrm/xfrm_user.c:3546
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x780/0x920 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
sock_sendmsg_nosec+0x13a/0x180 net/socket.c:797
__sock_sendmsg net/socket.c:812 [inline]
____sys_sendmsg+0x55c/0x870 net/socket.c:2716
___sys_sendmsg+0x2a5/0x360 net/socket.c:2770
__sys_sendmsg net/socket.c:2802 [inline]
__do_sys_sendmsg net/socket.c:2807 [inline]
__se_sys_sendmsg net/socket.c:2805 [inline]
__x64_sys_sendmsg+0x1c3/0x2a0 net/socket.c:2805
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fdb511ece59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fdb4f43e028 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007fdb51465fa0 RCX: 00007fdb511ece59
RDX: 0000000000000000 RSI: 0000200000000040 RDI: 0000000000000004
RBP: 00007fdb51282d6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fdb51466038 R14: 00007fdb51465fa0 R15: 00007fff24b1b738
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* [PATCH] cgroup/dmem: accept only one region per limit write
From: Eric Chanudet @ 2026-06-05 22:44 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Natalie Vock, Tejun Heo,
Johannes Weiner, Michal Koutný
Cc: cgroups, dri-devel, linux-kernel, Albert Esteve, Eric Chanudet
Accept only one "region value" pair entry for the dmem.max, dmem.min,
dmem.low files.
This changes the UAPI that otherwise accepted multiple lines for setting
multiple entries in one write. No existing user is known to rely on
writing multiple regions in a single write.
Processing multiple regions in dmemcg_limit_write() could quietly change
first limits before failing on a later one and returning an error to the
writer, with no indication some changes occurred.
Signed-off-by: Eric Chanudet <echanude@redhat.com>
---
Follow up from discussions on a previous thread[1].
If Albert's series[2] lands, I can cleanup and prepare some kunits for
these as well.
[1] https://lore.kernel.org/all/158bc103-7f99-4df4-8d3b-2da9b04ac0ed@lankhorst.se/
[2] https://lore.kernel.org/all/20260519-kunit_cgroups-v4-1-f6c2f498fae4@redhat.com/
---
kernel/cgroup/dmem.c | 70 +++++++++++++++++++---------------------------------
1 file changed, 26 insertions(+), 44 deletions(-)
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 6430c7ce1e0372f59f1313163fb7630ce49ac1ef..113ee88e276296bccb4def546adf5cc175d7f0be 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -734,57 +734,39 @@ static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
void (*apply)(struct dmem_cgroup_pool_state *, u64))
{
struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
- int err = 0;
-
- while (buf && !err) {
- struct dmem_cgroup_pool_state *pool = NULL;
- char *options, *region_name;
- struct dmem_cgroup_region *region;
- u64 new_limit;
-
- options = buf;
- buf = strchr(buf, '\n');
- if (buf)
- *buf++ = '\0';
-
- options = strstrip(options);
-
- /* eat empty lines */
- if (!options[0])
- continue;
-
- region_name = strsep(&options, " \t");
- if (!region_name[0])
- continue;
-
- if (!options || !*options)
- return -EINVAL;
+ struct dmem_cgroup_pool_state *pool;
+ struct dmem_cgroup_region *region;
+ char *region_name;
+ u64 new_limit;
+ int err;
- rcu_read_lock();
- region = dmemcg_get_region_by_name(region_name);
- rcu_read_unlock();
+ buf = strstrip(buf);
+ region_name = strsep(&buf, " \t");
+ if (!region_name[0] || !buf)
+ return -EINVAL;
- if (!region)
- return -EINVAL;
+ rcu_read_lock();
+ region = dmemcg_get_region_by_name(region_name);
+ rcu_read_unlock();
+ if (!region)
+ return -EINVAL;
- err = dmemcg_parse_limit(options, &new_limit);
- if (err < 0)
- goto out_put;
+ buf = strstrip(buf);
+ err = dmemcg_parse_limit(buf, &new_limit);
+ if (err < 0)
+ goto out_put;
- pool = get_cg_pool_unlocked(dmemcs, region);
- if (IS_ERR(pool)) {
- err = PTR_ERR(pool);
- goto out_put;
- }
+ pool = get_cg_pool_unlocked(dmemcs, region);
+ if (IS_ERR(pool)) {
+ err = PTR_ERR(pool);
+ goto out_put;
+ }
- /* And commit */
- apply(pool, new_limit);
- dmemcg_pool_put(pool);
+ apply(pool, new_limit);
+ dmemcg_pool_put(pool);
out_put:
- kref_put(®ion->ref, dmemcg_free_region);
- }
-
+ kref_put(®ion->ref, dmemcg_free_region);
return err ?: nbytes;
}
---
base-commit: 640c57d6ca1346a1c2363a3f473b405af979e046
change-id: 20260605-cgroup-dmem-write-single-region-9bf05b6d995d
Best regards,
--
Eric Chanudet <echanude@redhat.com>
^ permalink raw reply related
* Re: [PATCH] cgroup/cpuset: Support multiple source/destination cpusets using pids pattern
From: Waiman Long @ 2026-06-05 17:15 UTC (permalink / raw)
To: Ridong Chen; +Cc: cgroups, Tejun Heo, Johannes Weiner, linux-kernel
In-Reply-To: <d708fb7a-d12f-40da-95ca-fbc6d0552f07@linux.dev>
On 6/5/26 3:35 AM, Ridong Chen wrote:
>
> On 6/4/2026 2:47 AM, Waiman Long wrote:
>> On 6/3/26 6:26 AM, Ridong Chen wrote:
>>> The current cpuset_can_attach() and cpuset_attach() functions assume task
>>> migration is from one source cpuset to one destination cpuset. This
>>> can be
>>> wrong in several scenarios:
>>> - Moving a multi-threaded process with threads in different cpusets
>>> - Disabling the cpuset controller (many children to one parent)
>>> - Enabling the cpuset controller (one parent to many children)
>>>
>>> Fix this by adopting the pids subsystem's per-task accounting pattern.
>>> In cpuset_can_attach(), use task_cs(task) to get the correct source
>>> cpuset
>>> for each task (like pids_can_attach uses task_css), adjust
>>> nr_deadline_tasks
>>> and reserve DL bandwidth per-task, and increment attach_in_progress
>>> per-task
>>> on the destination cpuset. In cpuset_attach(), handle destination cpuset
>>> changes within the task iteration loop.
>>>
>>> A shared helper cpuset_undo_attach() reverses the per-task operations for
>>> both partial rollback in cpuset_can_attach() and full reversal in
>>> cpuset_cancel_attach().
>>>
>>> When multiple source cpusets are detected in can_attach(), set
>>> attach_many_sources so that cpuset_attach() forces cpus_updated and
>>> mems_updated to true, ensuring all tasks get properly updated regardless
>>> of which source cpuset cpuset_attach_old_cs points to.
>>>
>>> This eliminates the need for nr_migrate_dl_tasks, sum_migrate_dl_bw, and
>>> dl_bw_cpu fields in struct cpuset.
>>>
>>> Fixes: 4ec22e9c5a90 ("cpuset: Enable cpuset controller in default
>>> hierarchy")
>>> Signed-off-by: Ridong Chen <ridong.chen@linux.dev>
>> It is not a problem doing per-task DL BW allocation and eliminating the
>> *dl_bw* fields. However, updating nr_deadline_tasks before it is
>> committed can be problematic.
>>
> Good to hear that.
>
>> nr_deadline_tasks is used in dl_rebuild_rd_accounting() which is called
>> by partition_sched_domains_locked(). After the release of cpuset_mutex
>> at the end of cpuset_can_attach() and before cpuset_attach() or
>> cpuset_cancel_attach() is called, it is possible
>> that partition_sched_domains_locked() can be called
>> and dl_rebuild_rd_accounting() is not getting the right DL BW accounting
>> information. So unless there is a way to confirm that this situation
>> cannot happen, we can't change nr_deadline_tasks before the attach is
>> commited.
>>
> We can keep the nr_migrate_dl_tasks field and update nr_deadline_tasks
> once migration is complete. I think this will be much simpler than
> fixing the issue using lists.
>
But we still need to track the set of source and destination cpusets to
commit or cancel the change. Doing it task-by-task will add code in the
cpuset_attach() and cpuset_cancel_attach() to check if a task is a DL
task and act accordingly. So we are just trading task-by-task code with
code to handle the lists.
Cheers,
Longman
^ permalink raw reply
* Re: [PATCH-next v6 1/6] cgroup/cpuset: Fix node inconsistencies between cpuset_update_tasks_nodemask() and cpuset_attach()
From: Waiman Long @ 2026-06-05 17:09 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
Peter Zijlstra
Cc: cgroups, linux-kernel, Aaron Tomlin, Guopeng Zhang
In-Reply-To: <fb65a2f6-8ab0-43d0-b9e7-d223ec1a671d@linux.dev>
On 6/5/26 3:48 AM, Ridong Chen wrote:
>
> On 6/4/2026 11:02 PM, Waiman Long wrote:
>> Whenever memory node mask is changed, there are 4 places where the node
>> mask has to be updated or used.
>> 1) task's node mask via cpuset_change_task_nodemask()
>> 2) memory policy binding via mpol_rebind_mm()
>> 3) if memory migration is enabled, migrate from old_mems_allowed to
>> the new node mask via cpuset_migrate_mm().
>> 4) setting old_mems_allowed
>>
>> These memory actions are done in cpuset_update_tasks_nodemask() and
>> cpuset_attach(). However there are inconsistencies in what node masks
>> are being used in these 2 functions.
>>
>> In cpuset_update_tasks_nodemask(),
>> - cpuset_change_task_nodemask(): guarantee_online_mems()
>> - mpol_rebind_mm(): mems_allowed
>> - cpuset_migrate_mm(): guarantee_online_mems()
>> - old_mems_allowed: guarantee_online_mems()
>>
>> In cpuset_attach(),
>> - cpuset_change_task_nodemask(): guarantee_online_mems()
>> - mpol_rebind_mm(): effective_mems
>> - cpuset_migrate_mm(): effective_mems
>> - old_mems_allowed: effective_mems
>>
>> These inconsistencies dates back to quite a long time ago and it is
>> hard to say what should be the correct values.
>>
>> The guarantee_online_mems() function returns a node mask from current or
>> an ancestor cpuset that is a subset of node_states[N_MEMORY]. Nodes in
>> node_states[N_MEMORY] are all online, i.e. in node_states[N_ONLINE].
>> However, node in node_states[N_ONLINE] may not have memory. So
>> node_states[N_MEMORY] should be a subset of node_states[N_ONLINE].
>>
>> The guarantee_online_mems() function should only be useful for v1 where
>> mems_allowed is the same as effective_mems. With v2, the memory nodes
>> in effective_mems should always be a subset of node_states[N_MEMORY].
>> The only time that may not be true is when a memory hot-unplug operation
>> is in progress and a memory node is removed from node_states[N_MEMORY]
>> but not yet reflected in effective_mems as cpuset_handle_hotplug()
>> has not yet been called from cpuset_track_online_nodes(). When
>> cpuset_handle_hotplug() is called later, the memory node setting
>> of the relevant cpusets and tasks will be updated. So replacing the
>> guarantee_online_mems() call by just using cs->effective_mems should
>> be fine.
>>
> The message should be updated.
Could you be more specific about what message are you referring to?
>
>> 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);
>> -
> Nit.
>
> I prefer:
>
> guarantee_online_mems(cs, &cpuset_attach_nodemask_to);
> if (cpuset_v2() && !cpus_updated && !mems_updated)
> goto out;
This is fixed in patch 7. I may have to send a new version of the patch
7 integrated back into individual patches.
Cheers,
Longman
>> 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
> Other than that, looks good to me.
>
> Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
>
^ permalink raw reply
* Re: [PATCH 0/6 v3] mm/memcontrol, page_counter: move stock from mem_cgroup to page_counter
From: Joshua Hahn @ 2026-06-05 17:07 UTC (permalink / raw)
To: Joshua Hahn, Johannes Weiner, Michal Hocko
Cc: Roman Gushchin, Shakeel Butt, Muchun Song, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, cgroups,
linux-mm, linux-kernel, kernel-team
In-Reply-To: <20260605153603.234296-1-joshua.hahnjy@gmail.com>
On Fri, 5 Jun 2026 08:35:56 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> Memcg currently keeps a "stock" of 64 pages per-cpu to cache pre-charged
> allocations, allowing small allocations to avoid walking the expensive
> mem_cgroup hierarchy traversal and atomic operations on each charge.
> This design introduces a fastpath, but there is room for improvement:
>
> 1. Currently, each CPU tracks up to 7 (NR_MEMCG_STOCK) mem_cgroups. When
> more than 7 mem_cgroups are actively charging on a single CPU, a
> random victim is evicted and its associated stock is drained.
>
> 2. Stock management is tightly coupled to struct mem_cgroup, which makes
> it difficult to add a new page_counter to mem_cgroup and have
> multiple sources of stock management, which is required when trying
> to introduce fastpaths to multiple hard limit checks.
>
> This series moves the per-cpu stock down into the page_counter which
> consolidates stock limit checking and page_counter limit checking into
> page_counter_try_charge. This eliminates the 7-memcg-per-cpu slot limit,
> the random evictions (drain & refill), and slot traversal.
Hello reviewers,
I was hoping to receive some input on a point that Sashiko raises.
The draining work we do per-cpu uses work_on_cpu(), which does
schedule_work_on() and flush_work() on the system_percpu_wq, which is
not WQ_MEM_RECLAIM. And drain_all_stock() runs from try_charge_memcg()
on the reclaim path, so it actually triggers the check_flush_dependency()
since a wq_mem_reclaim is flushing a !wq_mem_reclaim.
In my testing, I haven't seen this become an issue. The flushing work
and draining only takes a local_lock() and does atomic operations,
and it never allocates, so there is no question on whether we can make
forward progress.
But this does slip up the WARN_ON since this is not obvious to the system.
I see three options here:
1. Trust that this is OK, and document that we can alwyas make forward
progress.
2. Keep the draining work synchronous, but queue and flush on memcg_wq
marked WQ_MEM_RECLAIM instead of just using work_on_cpu(). This would
add 2 words per-cpu-memcg for the work struct backpointer.
3. Go back to asynchronous, which would get rid of all the synchronous
concerns, but add an additional 2 words per-cpu-memcg for the work
struct backpointer here as well.
What do you think is the right decision here? I was thinking about this
quite a bit recently but just decided to send it out, but I think I should
have asked for upstream opinion sooner...
I would prefer to keep the memory footprint of this series minimal, and
opting to do things synchronously helped achieve this goal since we can
get rid of the backpointers. But I think this is beginning to show up as
a tradeoff, so I would really appreciate any input on what seems to be
the best decision here.
Thank you very much for your time!
Joshua
^ permalink raw reply
* Re: [PATCH v2 2/2] cgroup/dmem: add dmem.memcg control file for double-charging to memcg
From: Maarten Lankhorst @ 2026-06-05 15:53 UTC (permalink / raw)
To: Eric Chanudet
Cc: Michal Koutný, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, Maxime Ripard,
Natalie Vock, Tejun Heo, Jonathan Corbet, Shuah Khan, cgroups,
linux-mm, linux-kernel, dri-devel, T.J. Mercier,
Christian König, Maxime Ripard, Albert Esteve, Dave Airlie,
linux-doc
In-Reply-To: <aiLVbQPxK1qI1h4p@x1nano>
Hey,
On 6/5/26 17:42, Eric Chanudet wrote:
> On Fri, Jun 05, 2026 at 01:27:09PM +0200, Maarten Lankhorst wrote:
>> Hey,
>>
>> On 5/26/26 18:59, Eric Chanudet wrote:
>>> On Fri, May 22, 2026 at 05:26:16PM +0200, Michal Koutný wrote:
>>>> Hello Eric.
>>>>
>>>> On Tue, May 19, 2026 at 11:59:02AM -0400, Eric Chanudet <echanude@redhat.com> wrote:
>>>>> Add a root-only cgroupfs file "dmem.memcg" that lets an administrator
>>>>> configure whether allocations in a dmem region should also be charged to
>>>>> the memory controller.
>>>>
>>>> This kinda makes sense as it is not unlike io.cost.* device
>>>> configurators.
>>>>
>>>> Just for my better understanding -- will there be a space for userspace
>>>> to switch this? (No charged dmem allocations happen before responsible
>>>> userspace runs, so that the attribute remains unlocked.)
>>>>
>>>> (I'm rather indifferent about the actual double charging/non-charging
>>>> matter.)
>>>
>>> Yes, this is intended to be configured before the user space stack that
>>> would start allocating things is started. Once it has started (and tried
>>> to charge something), the configuration is locked
>>>
>>>>
>>>>>
>>>>> To handle inheritance, dmem adds a depends_on the memory controller,
>>>>> unless MEMCG isn't configured in.
>>>>>
>>>>> Double-charging is disabled by default. Once a charge is attempted, the
>>>>> setting is locked to prevent inconsistent accounting by a small 4-state
>>>>> machine (off, on, locked off, locked on).
>>>>>
>>>>> The memcg to charge is derived from the pool's cgroup, since the pool
>>>>> holds a reference to the dmem cgroup state that keeps the cgroup alive
>>>>> until it gets uncharged.
>>>>>
>>>>> Signed-off-by: Eric Chanudet <echanude@redhat.com>
>>>>> ---
>>>>> Documentation/admin-guide/cgroup-v2.rst | 23 +++++
>>>>> kernel/cgroup/dmem.c | 158 +++++++++++++++++++++++++++++++-
>>>>> 2 files changed, 178 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
>>>>> index 6efd0095ed995b1550317662bc1b56c7a7f3db23..1d2fa55ddf0faa17baa916a8914d3033e8e42359 100644
>>>>> --- a/Documentation/admin-guide/cgroup-v2.rst
>>>>> +++ b/Documentation/admin-guide/cgroup-v2.rst
>>>>> @@ -2828,6 +2828,29 @@ DMEM Interface Files
>>>>> drm/0000:03:00.0/vram0 12550144
>>>>> drm/0000:03:00.0/stolen 8650752
>>>>>
>>>>> + dmem.memcg
>>>>> + A readwrite nested-keyed file that exists only on the root
>>>>> + cgroup.
>>>>
>>>> Strictly speaking this is not nested-keyed but flat keyed [1],
>>>
>>> Indeed,
>>>
>>>> which leads me to realization that this is the first instance of a boolean.
>>>> All in call, such a composition comes to my mind (latter is RO):
>>>>
>>>> drm/0000:03:00.0/vram0 enable=0|1 locked=0|1
>>>>
>>>
>>> So per[1] 1 key, 2 sub-keys (enable RW, locked RO), that looks better
>>> and match the documentation, thanks!
>>>
>>>>
>>>>
>>>>> +static ssize_t dmem_cgroup_memcg_write(struct kernfs_open_file *of, char *buf,
>>>>> + size_t nbytes, loff_t off)
>>>>> +{
>>>>> + while (buf) {
>>>>> + struct dmem_cgroup_region *region;
>>>>> + char *options, *name;
>>>>> + bool flag;
>>>>> +
>>>>> + options = buf;
>>>>> + buf = strchr(buf, '\n');
>>>>> + if (buf)
>>>>> + *buf++ = '\0';
>>>>
>>>> I recall there was a discussion about accepting only a single device per
>>>> write(2) (at the same time I see this idiom is still present in other
>>>> dmem.* files, so this is nothing to change in _this_ patch).
>>>
>>> I would second that. When setting say dmem.max for 2 regions, with a
>>> typo on the second, the first one is set, but write still get EINVAL.
>>>
>>> Also, I just notice dmemcg_limit_write() returns EINVAL if the region is
>>> not found (this patch returns ENODEV).
>>>
>>>>
>>>> Thanks,
>>>> Michal
>>>>
>>>> [1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#format
>>>
>>>
>>>
>>
>> Perhaps a bit late, but before we start adding this UAPI we should enforce a
>> single region per write?
>
> I can send that separately, although that is a UAPI change. Is there any
> user that would be affected?
>
> This series is hung on charging memcg using memory objects from the
> context of dmem, when at that level of abstraction it doesn't have
> access to the underlying pieces that were allocated.
It's a uapi change, but I see more and more interest in the development and usage of dmemcg.
I believe it's better to fix this before users (perhaps accidentally) start to rely on this behavior.
Kind regards,
~Maarten Lankhorst
^ permalink raw reply
* Re: [PATCH v2 2/2] cgroup/dmem: add dmem.memcg control file for double-charging to memcg
From: Eric Chanudet @ 2026-06-05 15:42 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Michal Koutný, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, Maxime Ripard,
Natalie Vock, Tejun Heo, Jonathan Corbet, Shuah Khan, cgroups,
linux-mm, linux-kernel, dri-devel, T.J. Mercier,
Christian König, Maxime Ripard, Albert Esteve, Dave Airlie,
linux-doc
In-Reply-To: <158bc103-7f99-4df4-8d3b-2da9b04ac0ed@lankhorst.se>
On Fri, Jun 05, 2026 at 01:27:09PM +0200, Maarten Lankhorst wrote:
> Hey,
>
> On 5/26/26 18:59, Eric Chanudet wrote:
> > On Fri, May 22, 2026 at 05:26:16PM +0200, Michal Koutný wrote:
> >> Hello Eric.
> >>
> >> On Tue, May 19, 2026 at 11:59:02AM -0400, Eric Chanudet <echanude@redhat.com> wrote:
> >>> Add a root-only cgroupfs file "dmem.memcg" that lets an administrator
> >>> configure whether allocations in a dmem region should also be charged to
> >>> the memory controller.
> >>
> >> This kinda makes sense as it is not unlike io.cost.* device
> >> configurators.
> >>
> >> Just for my better understanding -- will there be a space for userspace
> >> to switch this? (No charged dmem allocations happen before responsible
> >> userspace runs, so that the attribute remains unlocked.)
> >>
> >> (I'm rather indifferent about the actual double charging/non-charging
> >> matter.)
> >
> > Yes, this is intended to be configured before the user space stack that
> > would start allocating things is started. Once it has started (and tried
> > to charge something), the configuration is locked
> >
> >>
> >>>
> >>> To handle inheritance, dmem adds a depends_on the memory controller,
> >>> unless MEMCG isn't configured in.
> >>>
> >>> Double-charging is disabled by default. Once a charge is attempted, the
> >>> setting is locked to prevent inconsistent accounting by a small 4-state
> >>> machine (off, on, locked off, locked on).
> >>>
> >>> The memcg to charge is derived from the pool's cgroup, since the pool
> >>> holds a reference to the dmem cgroup state that keeps the cgroup alive
> >>> until it gets uncharged.
> >>>
> >>> Signed-off-by: Eric Chanudet <echanude@redhat.com>
> >>> ---
> >>> Documentation/admin-guide/cgroup-v2.rst | 23 +++++
> >>> kernel/cgroup/dmem.c | 158 +++++++++++++++++++++++++++++++-
> >>> 2 files changed, 178 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> >>> index 6efd0095ed995b1550317662bc1b56c7a7f3db23..1d2fa55ddf0faa17baa916a8914d3033e8e42359 100644
> >>> --- a/Documentation/admin-guide/cgroup-v2.rst
> >>> +++ b/Documentation/admin-guide/cgroup-v2.rst
> >>> @@ -2828,6 +2828,29 @@ DMEM Interface Files
> >>> drm/0000:03:00.0/vram0 12550144
> >>> drm/0000:03:00.0/stolen 8650752
> >>>
> >>> + dmem.memcg
> >>> + A readwrite nested-keyed file that exists only on the root
> >>> + cgroup.
> >>
> >> Strictly speaking this is not nested-keyed but flat keyed [1],
> >
> > Indeed,
> >
> >> which leads me to realization that this is the first instance of a boolean.
> >> All in call, such a composition comes to my mind (latter is RO):
> >>
> >> drm/0000:03:00.0/vram0 enable=0|1 locked=0|1
> >>
> >
> > So per[1] 1 key, 2 sub-keys (enable RW, locked RO), that looks better
> > and match the documentation, thanks!
> >
> >>
> >>
> >>> +static ssize_t dmem_cgroup_memcg_write(struct kernfs_open_file *of, char *buf,
> >>> + size_t nbytes, loff_t off)
> >>> +{
> >>> + while (buf) {
> >>> + struct dmem_cgroup_region *region;
> >>> + char *options, *name;
> >>> + bool flag;
> >>> +
> >>> + options = buf;
> >>> + buf = strchr(buf, '\n');
> >>> + if (buf)
> >>> + *buf++ = '\0';
> >>
> >> I recall there was a discussion about accepting only a single device per
> >> write(2) (at the same time I see this idiom is still present in other
> >> dmem.* files, so this is nothing to change in _this_ patch).
> >
> > I would second that. When setting say dmem.max for 2 regions, with a
> > typo on the second, the first one is set, but write still get EINVAL.
> >
> > Also, I just notice dmemcg_limit_write() returns EINVAL if the region is
> > not found (this patch returns ENODEV).
> >
> >>
> >> Thanks,
> >> Michal
> >>
> >> [1] https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#format
> >
> >
> >
>
> Perhaps a bit late, but before we start adding this UAPI we should enforce a
> single region per write?
I can send that separately, although that is a UAPI change. Is there any
user that would be affected?
This series is hung on charging memcg using memory objects from the
context of dmem, when at that level of abstraction it doesn't have
access to the underlying pieces that were allocated.
Best,
>
> Kind regards,
> ~Maarten Lankhorst
>
--
Eric Chanudet
^ permalink raw reply
* Re: [PATCH 0/6 v3] mm/memcontrol, page_counter: move stock from mem_cgroup to page_counter
From: Joshua Hahn @ 2026-06-05 15:41 UTC (permalink / raw)
To: Joshua Hahn
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, cgroups, linux-mm, linux-kernel, kernel-team
In-Reply-To: <20260605153603.234296-1-joshua.hahnjy@gmail.com>
On Fri, 5 Jun 2026 08:35:56 -0700 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> Memcg currently keeps a "stock" of 64 pages per-cpu to cache pre-charged
> allocations, allowing small allocations to avoid walking the expensive
> mem_cgroup hierarchy traversal and atomic operations on each charge.
> This design introduces a fastpath, but there is room for improvement:
>
> 1. Currently, each CPU tracks up to 7 (NR_MEMCG_STOCK) mem_cgroups. When
> more than 7 mem_cgroups are actively charging on a single CPU, a
> random victim is evicted and its associated stock is drained.
>
> 2. Stock management is tightly coupled to struct mem_cgroup, which makes
> it difficult to add a new page_counter to mem_cgroup and have
> multiple sources of stock management, which is required when trying
> to introduce fastpaths to multiple hard limit checks.
>
> This series moves the per-cpu stock down into the page_counter which
> consolidates stock limit checking and page_counter limit checking into
> page_counter_try_charge. This eliminates the 7-memcg-per-cpu slot limit,
> the random evictions (drain & refill), and slot traversal.
Sorry, two things that I forgot to add as reviewers' notes:
- There was a previous v3, but that was just a rebase, so I wasn't sure
how to name that / this. I decided to name this one v3, since the
last one didn't have any changes at all. I apologize in case there are
any confusions.
- I think it is quite late in the merge cycle, this is intended for the
next cycle, not this one.
Thank you!
^ 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