* [PATCH v4 1/2] writeback: let foreign flushes reach dying cgwbs
2026-09-30 19:35 [PATCH v4 0/2] writeback: let foreign flushes reach dying cgwbs Liz Fong-Jones
@ 2026-09-30 19:35 ` Liz Fong-Jones
2026-10-01 18:43 ` Tejun Heo
2026-09-30 19:35 ` [PATCH v4 2/2] writeback: switch a replaced cgwb's inodes to its successor Liz Fong-Jones
1 sibling, 1 reply; 5+ messages in thread
From: Liz Fong-Jones @ 2026-09-30 19:35 UTC (permalink / raw)
To: Christian Brauner, Jan Kara, Tejun Heo
Cc: Alexander Viro, Jens Axboe, Andrew Morton, Johannes Weiner,
Roman Gushchin, Shakeel Butt, Xin Yin, linux-fsdevel, linux-mm,
cgroups, linux-kernel, ian, Liz Fong-Jones
When a container is replaced and the new one keeps appending to files
the old one left dirty, the new one can stall in balance_dirty_pages()
for seconds to minutes, with little CPU use and its write lag growing
linearly before it recovers. This matches a 30-60s stall after each
deploy of a container at Honeycomb that reads from Kafka and writes
columnar files to a host volume.
Trigger: a cgroup dirties files and is removed while they are still
dirty, and a sibling keeps appending to the same files under a parent
memory limit. cgwb_kill() takes the removed memcg's wbs out of
bdi->cgwb_tree, but the inodes stay attached to them, so the sibling's
foreign flushes fail with -ENOENT in wb_get_lookup().
Keep killed wbs in bdi->cgwb_tree until they are released, so that
cgroup_writeback_by_id() finds them through the regular lookup. The
creation paths skip dying wbs, and a new wb takes over the slot of a
dying one when a live memcg's blkcg association changes; release only
removes a wb that still owns its slot. cgwb_create() also refuses to
link a new wb for a dying memcg, so a removed memcg's dirty wb can't be
replaced before wb_memcg_offline() kills the new one. The blkcg
association check moves from wb_get_lookup() to the creation side: once
a memcg with io enabled is removed, cgroup_get_e_css() returns an
ancestor's io css and its dying wb would not match.
With commit 168a8c13159c ("writeback: size foreign flushes by target
wb dirty pages"), the flush is then sized from the wb's own dirty
pages and writes out what the replacement dirtied.
Test, on top of that commit: a cgroup appends to 1000 files at
250MiB/s and is removed, then a second cgroup keeps appending to the
same files under a 4G parent limit. Worst lag of the second writer
behind schedule over 120s, in three runs:
without this patch 85.3s 46.9s 52.1s
with this patch 1.0s 1.4s 1.0s
Suggested-by: Tejun Heo <tj@kernel.org>
Assisted-by: Claude:claude-opus-5-5 checkpatch sparse
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>
---
include/linux/backing-dev-defs.h | 13 +++++-
include/linux/backing-dev.h | 2 +-
mm/backing-dev.c | 92 +++++++++++++++++++++++++++++++---------
3 files changed, 84 insertions(+), 23 deletions(-)
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 4f1084937315..035d25f5b81b 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -196,7 +196,7 @@ struct backing_dev_info {
struct bdi_writeback wb; /* the root writeback info for this bdi */
struct list_head wb_list; /* list of all wbs */
#ifdef CONFIG_CGROUP_WRITEBACK
- struct radix_tree_root cgwb_tree; /* radix tree of active cgroup wbs */
+ struct radix_tree_root cgwb_tree; /* radix tree of cgroup wbs, incl. killed */
struct mutex cgwb_release_mutex; /* protect shutdown of wb structs */
struct rw_semaphore wb_switch_rwsem; /* no cgwb switch while syncing */
#endif
@@ -229,6 +229,17 @@ static inline bool wb_tryget(struct bdi_writeback *wb)
return true;
}
+/**
+ * wb_tryget_live - try to increment a wb's refcount if it hasn't been killed
+ * @wb: bdi_writeback to get
+ */
+static inline bool wb_tryget_live(struct bdi_writeback *wb)
+{
+ if (wb != &wb->bdi->wb)
+ return percpu_ref_tryget_live(&wb->refcnt);
+ return true;
+}
+
/**
* wb_get - increment a wb's refcount
* @wb: bdi_writeback to get
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index c2284466e7aa..f7ef5895625a 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -222,7 +222,7 @@ wb_get_create_current(struct backing_dev_info *bdi, gfp_t gfp)
rcu_read_lock();
wb = wb_find_current(bdi);
- if (wb && unlikely(!wb_tryget(wb)))
+ if (wb && unlikely(!wb_tryget_live(wb)))
wb = NULL;
rcu_read_unlock();
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index cecbcf9060a6..a370dd94b6a4 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -614,6 +614,12 @@ static void cgwb_release_workfn(struct work_struct *work)
release_work);
struct backing_dev_info *bdi = wb->bdi;
+ scoped_guard(spinlock_irq, &cgwb_lock) {
+ /* a newer wb may have taken the slot, see cgwb_create() */
+ radix_tree_delete_item(&bdi->cgwb_tree, wb->memcg_css->id, wb);
+ list_del(&wb->offline_node);
+ }
+
mutex_lock(&wb->bdi->cgwb_release_mutex);
wb_shutdown(wb);
@@ -627,10 +633,6 @@ static void cgwb_release_workfn(struct work_struct *work)
fprop_local_destroy_percpu(&wb->memcg_completions);
- spin_lock_irq(&cgwb_lock);
- list_del(&wb->offline_node);
- spin_unlock_irq(&cgwb_lock);
-
wb_exit(wb);
bdi_put(bdi);
WARN_ON_ONCE(!list_empty(&wb->b_attached));
@@ -645,11 +647,16 @@ static void cgwb_release(struct percpu_ref *refcnt)
queue_work(cgwb_release_wq, &wb->release_work);
}
+/*
+ * A killed wb stays in bdi->cgwb_tree until it is released or replaced in
+ * cgwb_create(), so that foreign flushes can still find it through
+ * wb_get_lookup(). Inodes attached to it can keep collecting dirty pages
+ * from other memcgs until they are written back and switched away.
+ */
static void cgwb_kill(struct bdi_writeback *wb)
{
lockdep_assert_held(&cgwb_lock);
- WARN_ON(!radix_tree_delete(&wb->bdi->cgwb_tree, wb->memcg_css->id));
list_del(&wb->memcg_node);
list_del(&wb->blkcg_node);
list_add(&wb->offline_node, &offline_cgwbs);
@@ -669,7 +676,8 @@ static int cgwb_create(struct backing_dev_info *bdi,
struct mem_cgroup *memcg;
struct cgroup_subsys_state *blkcg_css;
struct list_head *memcg_cgwb_list, *blkcg_cgwb_list;
- struct bdi_writeback *wb;
+ struct bdi_writeback *wb, *old_wb;
+ void __rcu **slot;
unsigned long flags;
int ret = 0;
@@ -681,6 +689,8 @@ static int cgwb_create(struct backing_dev_info *bdi,
/* look up again under lock and discard on blkcg mismatch */
spin_lock_irqsave(&cgwb_lock, flags);
wb = radix_tree_lookup(&bdi->cgwb_tree, memcg_css->id);
+ if (wb && wb_dying(wb))
+ wb = NULL;
if (wb && wb->blkcg_css != blkcg_css) {
cgwb_kill(wb);
wb = NULL;
@@ -726,9 +736,24 @@ static int cgwb_create(struct backing_dev_info *bdi,
ret = -ENODEV;
spin_lock_irqsave(&cgwb_lock, flags);
if (test_bit(WB_registered, &bdi->wb.state) &&
- blkcg_cgwb_list->next && memcg_cgwb_list->next) {
- /* we might have raced another instance of this function */
- ret = radix_tree_insert(&bdi->cgwb_tree, memcg_css->id, wb);
+ blkcg_cgwb_list->next && memcg_cgwb_list->next &&
+ !css_is_dying(memcg_css)) {
+ /*
+ * We might have raced another instance of this function. A
+ * dying wb keeps its slot until released; take it over.
+ */
+ slot = radix_tree_lookup_slot(&bdi->cgwb_tree, memcg_css->id);
+ if (!slot) {
+ ret = radix_tree_insert(&bdi->cgwb_tree, memcg_css->id, wb);
+ } else {
+ old_wb = radix_tree_deref_slot_protected(slot, &cgwb_lock);
+ if (wb_dying(old_wb)) {
+ radix_tree_replace_slot(&bdi->cgwb_tree, slot, wb);
+ ret = 0;
+ } else {
+ ret = -EEXIST;
+ }
+ }
if (!ret) {
list_add_tail_rcu(&wb->bdi_node, &bdi->wb_list);
list_add(&wb->memcg_node, memcg_cgwb_list);
@@ -768,10 +793,29 @@ static int cgwb_create(struct backing_dev_info *bdi,
* Try to get the wb for @memcg_css on @bdi. The returned wb has its
* refcount incremented.
*
- * This function uses css_get() on @memcg_css and thus expects its refcnt
- * to be positive on invocation. IOW, rcu_read_lock() protection on
- * @memcg_css isn't enough. try_get it before calling this function.
- *
+ * The wb may have been killed and its blkcg association may be stale. This
+ * is what foreign flushes want: they target the wb that owns the dirty
+ * inodes, which can be a killed one. Use wb_get_create() to get a wb to
+ * attach inodes to.
+ */
+struct bdi_writeback *wb_get_lookup(struct backing_dev_info *bdi,
+ struct cgroup_subsys_state *memcg_css)
+{
+ struct bdi_writeback *wb;
+
+ if (!memcg_css->parent)
+ return &bdi->wb;
+
+ rcu_read_lock();
+ wb = radix_tree_lookup(&bdi->cgwb_tree, memcg_css->id);
+ if (wb && !wb_tryget(wb))
+ wb = NULL;
+ rcu_read_unlock();
+
+ return wb;
+}
+
+/*
* A wb is keyed by its associated memcg. As blkcg implicitly enables
* memcg on the default hierarchy, memcg association is guaranteed to be
* more specific (equal or descendant to the associated blkcg) and thus can
@@ -782,9 +826,13 @@ static int cgwb_create(struct backing_dev_info *bdi,
* both the memcg and blkcg associated with it and verifies the blkcg on
* each lookup. On mismatch, the existing wb is discarded and a new one is
* created.
+ *
+ * This function uses css_get() on @memcg_css and thus expects its refcnt
+ * to be positive on invocation. IOW, rcu_read_lock() protection on
+ * @memcg_css isn't enough. try_get it before calling this function.
*/
-struct bdi_writeback *wb_get_lookup(struct backing_dev_info *bdi,
- struct cgroup_subsys_state *memcg_css)
+static struct bdi_writeback *cgwb_get_live(struct backing_dev_info *bdi,
+ struct cgroup_subsys_state *memcg_css)
{
struct bdi_writeback *wb;
@@ -798,7 +846,7 @@ struct bdi_writeback *wb_get_lookup(struct backing_dev_info *bdi,
/* see whether the blkcg association has changed */
blkcg_css = cgroup_get_e_css(memcg_css->cgroup, &io_cgrp_subsys);
- if (unlikely(wb->blkcg_css != blkcg_css || !wb_tryget(wb)))
+ if (unlikely(wb->blkcg_css != blkcg_css || !wb_tryget_live(wb)))
wb = NULL;
css_put(blkcg_css);
}
@@ -813,8 +861,8 @@ struct bdi_writeback *wb_get_lookup(struct backing_dev_info *bdi,
* @memcg_css: cgroup_subsys_state of the target memcg (must have positive ref)
* @gfp: allocation mask to use
*
- * Try to get the wb for @memcg_css on @bdi. If it doesn't exist, try to
- * create one. See wb_get_lookup() for more details.
+ * Try to get the live wb for @memcg_css on @bdi. If it doesn't exist, try
+ * to create one. See cgwb_get_live() for more details.
*/
struct bdi_writeback *wb_get_create(struct backing_dev_info *bdi,
struct cgroup_subsys_state *memcg_css,
@@ -825,7 +873,7 @@ struct bdi_writeback *wb_get_create(struct backing_dev_info *bdi,
might_alloc(gfp);
do {
- wb = wb_get_lookup(bdi, memcg_css);
+ wb = cgwb_get_live(bdi, memcg_css);
} while (!wb && !cgwb_create(bdi, memcg_css, gfp));
return wb;
@@ -858,8 +906,10 @@ static void cgwb_bdi_unregister(struct backing_dev_info *bdi)
WARN_ON(test_bit(WB_registered, &bdi->wb.state));
spin_lock_irq(&cgwb_lock);
- radix_tree_for_each_slot(slot, &bdi->cgwb_tree, &iter, 0)
- cgwb_kill(*slot);
+ radix_tree_for_each_slot(slot, &bdi->cgwb_tree, &iter, 0) {
+ if (!wb_dying(*slot))
+ cgwb_kill(*slot);
+ }
spin_unlock_irq(&cgwb_lock);
mutex_lock(&bdi->cgwb_release_mutex);
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v4 2/2] writeback: switch a replaced cgwb's inodes to its successor
2026-09-30 19:35 [PATCH v4 0/2] writeback: let foreign flushes reach dying cgwbs Liz Fong-Jones
2026-09-30 19:35 ` [PATCH v4 1/2] " Liz Fong-Jones
@ 2026-09-30 19:35 ` Liz Fong-Jones
2026-10-01 18:43 ` Tejun Heo
1 sibling, 1 reply; 5+ messages in thread
From: Liz Fong-Jones @ 2026-09-30 19:35 UTC (permalink / raw)
To: Christian Brauner, Jan Kara, Tejun Heo
Cc: Alexander Viro, Jens Axboe, Andrew Morton, Johannes Weiner,
Roman Gushchin, Shakeel Butt, Xin Yin, linux-fsdevel, linux-mm,
cgroups, linux-kernel, ian, Liz Fong-Jones
When a live memcg's blkcg association changes, cgwb_create() replaces
its killed wb in bdi->cgwb_tree. Foreign flushes for the memcg then
find the new wb, but the inodes, dirty or not, are still attached to the
old one, and they only move over as they are written back (see
wbc_attach_and_unlock_inode()). Until then, other memcgs appending to
those inodes stall in balance_dirty_pages().
After the takeover, queue a work item that switches all of the old wb's
inodes to the wb foreign flushes now find, like cleanup_offline_cgwb()
does for b_attached and b_dirty_time, but including b_dirty, b_io and
b_more_io. The switch carries the dirty and writeback page counts, so
nothing needs to be written back first. cgwb_create() can run with
interrupts disabled (folio_account_dirtied() -> inode_attach_wb()), so
it only queues the work, after dropping cgwb_lock.
Test: a live cgroup appends to 5000 files on XFS at 250MiB/s, io is
disabled on its parent so its wb is killed, it creates a file (the
takeover), and a sibling keeps appending to its files under a 4G parent
limit. Worst lag of the sibling behind schedule over 30s, in three runs:
without this patch 16.6s 28.7s 1.6s
with this patch 1.3s 1.4s 1.0s
Switching all 5001 inodes took 40-60ms per takeover, with up to 139MiB
of the old cgroup's pages dirty.
Suggested-by: Tejun Heo <tj@kernel.org>
Assisted-by: Claude:claude-opus-5-5 checkpatch sparse
Assisted-by: Claude:claude-fable-5-1
Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>
---
fs/fs-writeback.c | 53 ++++++++++++++++++++++++++++++++++++++++
include/linux/backing-dev-defs.h | 1 +
include/linux/writeback.h | 1 +
mm/backing-dev.c | 27 +++++++++++++++++++-
4 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index d26b2cf05283..1d26a137e8e4 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -803,6 +803,59 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
return restart;
}
+/**
+ * switch_replaced_cgwb - switch a replaced wb's inodes to its successor
+ * @wb: target wb, replaced in bdi->cgwb_tree by another wb of its memcg
+ *
+ * Switch all inodes attached to @wb, dirty or not, to the wb that foreign
+ * flushes now find for @wb's memcg. The switch carries the dirty and
+ * writeback page counts, so nothing needs to be written back first. Returns
+ * %true if not all inodes were switched and the function has to be restarted.
+ */
+bool switch_replaced_cgwb(struct bdi_writeback *wb)
+{
+ struct inode_switch_wbs_context *isw;
+ struct bdi_writeback *new_wb;
+ bool restart;
+ int nr = 0;
+
+ new_wb = wb_get_lookup(wb->bdi, wb->memcg_css);
+ if (!new_wb)
+ return false;
+ if (new_wb == wb) {
+ wb_put(new_wb);
+ return false;
+ }
+
+ isw = kzalloc_flex(*isw, inodes, WB_MAX_INODES_PER_ISW);
+ if (!isw) {
+ wb_put(new_wb);
+ return false;
+ }
+
+ atomic_inc(&isw_nr_in_flight);
+
+ spin_lock(&wb->list_lock);
+ restart = isw_prepare_wbs_switch(new_wb, isw, &wb->b_attached, &nr) ||
+ isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty, &nr) ||
+ isw_prepare_wbs_switch(new_wb, isw, &wb->b_io, &nr) ||
+ isw_prepare_wbs_switch(new_wb, isw, &wb->b_more_io, &nr) ||
+ isw_prepare_wbs_switch(new_wb, isw, &wb->b_dirty_time, &nr);
+ spin_unlock(&wb->list_lock);
+
+ if (nr == 0) {
+ atomic_dec(&isw_nr_in_flight);
+ wb_put(new_wb);
+ kfree(isw);
+ return restart;
+ }
+
+ trace_inode_switch_wbs_queue(wb, new_wb, nr);
+ wb_queue_isw(new_wb, isw);
+
+ return restart;
+}
+
/**
* wbc_attach_and_unlock_inode - associate wbc with target inode and unlock it
* @wbc: writeback_control of interest
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 035d25f5b81b..0d9a113f6fd6 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -160,6 +160,7 @@ struct bdi_writeback {
* to this wb */
struct llist_head switch_wbs_ctxs; /* queued contexts for
* writeback switching */
+ struct work_struct replaced_work; /* see switch_replaced_cgwb() */
union {
struct work_struct release_work;
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index b749a9a5a5ee..88e4b5049698 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -209,6 +209,7 @@ int cgroup_writeback_by_id(u64 bdi_id, int memcg_id,
enum wb_reason reason, struct wb_completion *done);
void cgroup_writeback_umount(struct super_block *sb);
bool cleanup_offline_cgwb(struct bdi_writeback *wb);
+bool switch_replaced_cgwb(struct bdi_writeback *wb);
/**
* inode_attach_wb - associate an inode with its wb
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index a370dd94b6a4..1acda0233e69 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -637,9 +637,20 @@ static void cgwb_release_workfn(struct work_struct *work)
bdi_put(bdi);
WARN_ON_ONCE(!list_empty(&wb->b_attached));
WARN_ON_ONCE(work_pending(&wb->switch_work));
+ WARN_ON_ONCE(work_pending(&wb->replaced_work));
call_rcu(&wb->rcu, cgwb_free_rcu);
}
+static void cgwb_replaced_workfn(struct work_struct *work)
+{
+ struct bdi_writeback *wb = container_of(work, struct bdi_writeback,
+ replaced_work);
+
+ while (switch_replaced_cgwb(wb))
+ cond_resched();
+ wb_put(wb);
+}
+
static void cgwb_release(struct percpu_ref *refcnt)
{
struct bdi_writeback *wb = container_of(refcnt, struct bdi_writeback,
@@ -676,7 +687,7 @@ static int cgwb_create(struct backing_dev_info *bdi,
struct mem_cgroup *memcg;
struct cgroup_subsys_state *blkcg_css;
struct list_head *memcg_cgwb_list, *blkcg_cgwb_list;
- struct bdi_writeback *wb, *old_wb;
+ struct bdi_writeback *wb, *old_wb, *replaced_wb = NULL;
void __rcu **slot;
unsigned long flags;
int ret = 0;
@@ -724,6 +735,7 @@ static int cgwb_create(struct backing_dev_info *bdi,
INIT_WORK(&wb->switch_work, inode_switch_wbs_work_fn);
init_llist_head(&wb->switch_wbs_ctxs);
INIT_WORK(&wb->release_work, cgwb_release_workfn);
+ INIT_WORK(&wb->replaced_work, cgwb_replaced_workfn);
set_bit(WB_registered, &wb->state);
bdi_get(bdi);
@@ -749,6 +761,8 @@ static int cgwb_create(struct backing_dev_info *bdi,
old_wb = radix_tree_deref_slot_protected(slot, &cgwb_lock);
if (wb_dying(old_wb)) {
radix_tree_replace_slot(&bdi->cgwb_tree, slot, wb);
+ if (wb_tryget(old_wb))
+ replaced_wb = old_wb;
ret = 0;
} else {
ret = -EEXIST;
@@ -764,6 +778,17 @@ static int cgwb_create(struct backing_dev_info *bdi,
}
}
spin_unlock_irqrestore(&cgwb_lock, flags);
+
+ /*
+ * The replaced wb is out of foreign flushes' reach but may still have
+ * inodes attached, dirty or not. Switch them over to @wb. We may be
+ * running with interrupts disabled, so use a work item. A replaced wb
+ * never returns to the tree, so its work can't already be pending.
+ */
+ if (replaced_wb &&
+ !queue_work(system_dfl_wq, &replaced_wb->replaced_work))
+ wb_put(replaced_wb);
+
if (ret) {
if (ret == -EEXIST)
ret = 0;
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread