[PATCH] writeback: let foreign flushes reach dying cgwbs

[PATCH] writeback: let foreign flushes reach dying cgwbs

[Liz Fong-Jones]

cgwb_kill() takes an offline memcg's wbs out of bdi->cgwb_tree, but
inodes that are still being dirtied stay attached to them. When a
replacement container keeps appending to those files, its foreign
flushes fail with -ENOENT in wb_get_lookup(), and it sleeps in
balance_dirty_pages() for seconds to minutes.

Fall back to searching bdi->wb_list, where killed wbs stay until they
are released. 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.0s   1.1s

Fixes: d62241c7a406 ("writeback, memcg: Implement cgroup_writeback_by_id()")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5 checkpatch sparse
Signed-off-by: Liz Fong-Jones <lizf@honeycomb.io>
---
At Honeycomb, a container that reads from Kafka and writes columnar
files to a host volume stalls for 30-60s after each deploy replaces it
(v6.18). This is likely the cause: the stall looks the same as in the
reproducer (little CPU use, lag growing linearly and then recovering),
though we haven't traced it in production yet.

Based on vfs-7.4.misc, next to Xin Yin's 168a8c13159c, which fixes the
sizing but not the lookup of a removed cgroup's wb. Each helps without
the other; this one also applies cleanly to mainline. On mainline
(fd179f8a05be), without 168a8c13159c, the test from the changelog gave
52.8s, 73.3s and 5.2s worst lag before this patch and 1.0s, 1.0s and
1.0s after.

Trigger: a cgroup dirties files and is removed while they are still
dirty; a sibling keeps appending to the same files under a parent memory
limit. Symptom: cgroup_writeback_by_id() returns -ENOENT and the sibling
stalls in balance_dirty_pages(). Minimal recipe below; the harness that
produced the numbers (paced writer, lag per second, MODE=alive control)
is at https://gist.github.com/lizthegrey/2209d831930588f63076bdc0ac7b78e2.

On ext4, syncing before the removal doesn't reliably help: writeback can
leave inodes I_DIRTY_SYNC after syncfs, and cleanup_offline_cgwbs_workfn()
then skips the dying wb entirely (2 of 3 runs at 5000 files; a second
syncfs avoided it in 5 of 5). On XFS one syncfs left the wb clean in 5
of 5 runs.

Harness settings for all numbers: MEM_MAX=max POD_MAX=4G RATE_MBPS=250,
plus NFILES=5000 SYNC_OLD=1 (or 2) for the syncfs runs and
NFILES=500000 OLD_SECS=120 SYNC_OLD=1 POD_HOP=10 for the 500k-file runs.

Also reproduced unpatched on bare-metal arm64 with Ubuntu's 7.0 kernel
(ext4 on a loop device): 12.9s worst lag with the old cgroup removed,
2.5s with it kept.

Tested: arm64 KVM guests (virtme-ng), ext4 and XFS on virtio. No lockdep,
PROVE_RCU, DEBUG_ATOMIC_SLEEP or DEBUG_PAGEALLOC reports. W=1, sparse
and checkpatch --strict clean.
No new warnings building arm64 defconfig with and without
CGROUP_WRITEBACK, allnoconfig, arm multi_v7_defconfig with
CGROUP_WRITEBACK, and the touched files (W=1) under arm64 allmodconfig
and x86_64 defconfig with CGROUP_WRITEBACK.

Not tested: x86, production, linux-next.

Workaround until this lands: lowering vm.dirty_expire_centisecs from
3000 to 500 cut the worst lag on unpatched mainline to 4.8s, 4.6s and
14.3s, but did not remove the stall.

Stable: please take 168a8c13159c too. Without it the flush is sized from
the dead memcg's own dirty pages, so this only helps while it has some.
f6988c90671e (already in mainline and tagged for stable) also matters:
on XFS with 500k synced inodes and the pod cgroup removed 10s later, the
handover was slow enough without it that the replacement fell 21-22s
behind (vfs-7.4.misc), against 1.0-1.2s with it (mainline).
Before 5.14 there is no offline_cgwbs cleanup, but the bdi->wb_list walk
should still apply.

Not covered: after a blkcg association change, wb_get_lookup() finds the
new clean wb, so the fallback doesn't run.

Developed with Claude Opus 5.5, which wrote the code, the reproducers
and first drafts of this text, and ran the builds and VM tests. I drove
the investigation from the production symptoms, designed the
experiments and controls (repeated runs, parent-commit baselines,
testing this patch on its own), and reviewed the analysis, code and
results.

Minimal recipe:
	#!/bin/bash
	# As root, cgroup v2, in a directory on ext4/xfs/btrfs ($DIR).
	cg=/sys/fs/cgroup/wbmini
	mkdir $cg && echo +memory > $cg/cgroup.subtree_control
	echo 4G > $cg/memory.max
	mkdir $cg/old $cg/new
	append() {  # append $1 blocks of $2 to each of 1000 files
		for i in $(seq 1000); do
			dd if=/dev/zero of=$DIR/f$i bs=$2 count=$1 \
			   oflag=append conv=notrunc status=none
		done
	}
	(echo $BASHPID > $cg/old/cgroup.procs
	 append 8 1M            # old owner writes the files...
	 append 1 64k)          # ...and exits with all of them dirty
	rmdir $cg/old           # its cgroup goes away while they are dirty
	bpftrace -e 'kretprobe:cgroup_writeback_by_id { @ret[(int32)retval] = count(); }' &
	sleep 3
	time (echo $BASHPID > $cg/new/cgroup.procs
	      append 4 1M)      # replacement keeps appending to the same files
	kill -INT $!; wait
	rmdir $cg/new $cg
	# Unpatched: many @ret[-2] (-ENOENT) and the append stalls.
	# Patched: @ret[0] only.
---
 fs/fs-writeback.c           |  2 ++
 include/linux/backing-dev.h |  2 ++
 mm/backing-dev.c            | 31 +++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index d26b2cf05283..5fb70c16488d 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1189,6 +1189,8 @@ int cgroup_writeback_by_id(u64 bdi_id, int memcg_id,
 	 * there's nothing to flush, don't create one.
 	 */
 	wb = wb_get_lookup(bdi, memcg_css);
+	if (!wb)
+		wb = wb_get_lookup_dying(bdi, memcg_css);
 	if (!wb) {
 		ret = -ENOENT;
 		goto out_css_put;
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index c2284466e7aa..2bbc74b3b180 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -151,6 +151,8 @@ static inline void bdi_wb_stat_mod(struct inode *inode, enum wb_stat_item item,
 
 struct bdi_writeback *wb_get_lookup(struct backing_dev_info *bdi,
 				    struct cgroup_subsys_state *memcg_css);
+struct bdi_writeback *wb_get_lookup_dying(struct backing_dev_info *bdi,
+					  struct cgroup_subsys_state *memcg_css);
 struct bdi_writeback *wb_get_create(struct backing_dev_info *bdi,
 				    struct cgroup_subsys_state *memcg_css,
 				    gfp_t gfp);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index cecbcf9060a6..1437f80ed7e3 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -807,6 +807,37 @@ struct bdi_writeback *wb_get_lookup(struct backing_dev_info *bdi,
 	return wb;
 }
 
+/**
+ * wb_get_lookup_dying - get a killed wb for a given memcg
+ * @bdi: target bdi
+ * @memcg_css: cgroup_subsys_state of the target memcg (must have positive ref)
+ *
+ * wb_get_lookup() only finds wbs that are still in @bdi->cgwb_tree.  A
+ * killed wb leaves the tree but stays on @bdi->wb_list until it is
+ * released, and inodes attached to it can keep collecting dirty pages from
+ * other memcgs until they are written back and switched away, so foreign
+ * flushes still need to reach it.  Returns a wb of @memcg_css on @bdi which
+ * has dirty IO, with a reference held, or %NULL.  If there are several,
+ * the first one found is returned; this is best effort.
+ */
+struct bdi_writeback *wb_get_lookup_dying(struct backing_dev_info *bdi,
+					  struct cgroup_subsys_state *memcg_css)
+{
+	struct bdi_writeback *wb;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(wb, &bdi->wb_list, bdi_node) {
+		if (wb->memcg_css == memcg_css && wb_has_dirty_io(wb) &&
+		    wb_tryget(wb)) {
+			rcu_read_unlock();
+			return wb;
+		}
+	}
+	rcu_read_unlock();
+
+	return NULL;
+}
+
 /**
  * wb_get_create - get wb for a given memcg, create if necessary
  * @bdi: target bdi

---
base-commit: 168a8c13159c6e3f0f08da6f8fa2f633a91ba9fd
change-id: 20260926-wb-dying-cgwb-flush-06e260e2abad

Best regards,
--  
Liz Fong-Jones <lizf@honeycomb.io>

Re: [PATCH] writeback: let foreign flushes reach dying cgwbs

[Tejun Heo]

Hello, Liz.

On Sun, Sep 27, 2026 at 05:04:39AM +0000, Liz Fong-Jones wrote:
> Fall back to searching bdi->wb_list, where killed wbs stay until they
> are released. 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.

Instead of walking bdi->wb_list, can't we update the lifetime rule so that
a wb stays on bdi->cgwb_tree until it's actually released? That is, remove
it from the tree in cgwb_release_workfn() instead of cgwb_kill() and have
the creation paths skip dying wbs. cgroup_writeback_by_id() would then
find the dying wb through the regular lookup.

Note that the blkcg association check in wb_get_lookup() would have to
move to the creation side. If io is enabled on the removed cgroup,
cgroup_get_e_css() returns an ancestor's io css and a dying wb would never
match.

> Fixes: d62241c7a406 ("writeback, memcg: Implement cgroup_writeback_by_id()")
> Cc: stable@vger.kernel.org

I don't think this qualifies as a fix. Can you drop the Fixes: and stable
tags?

Thanks.

-- 
tejun

Re: [PATCH] writeback: let foreign flushes reach dying cgwbs

[Liz Fong-Jones]

Hi Tejun,

On Mon, Sep 28, 2026 at 09:19:37AM -1000, Tejun Heo wrote:
> Instead of walking bdi->wb_list, can't we update the lifetime rule so that
> a wb stays on bdi->cgwb_tree until it's actually released? That is, remove
> it from the tree in cgwb_release_workfn() instead of cgwb_kill() and have
> the creation paths skip dying wbs. cgroup_writeback_by_id() would then
> find the dying wb through the regular lookup.

Yes, that's cleaner, thanks for the suggestion. Implemented in v2:
https://lore.kernel.org/all/20260928-wb-dying-cgwb-flush-v2-1-56b54cda74f2@honeycomb.io/

One edge case: a live memcg whose blkcg association changes needs a new
wb in the same slot while the killed one is still there, so
cgwb_create() takes over a dying wb's slot and cgwb_release_workfn()
uses radix_tree_delete_item() to only remove a wb that still owns it.
cgwb_bdi_unregister() skips dying wbs so they aren't killed twice.

> Note that the blkcg association check in wb_get_lookup() would have to
> move to the creation side.

Done, along with a wb_tryget_live() for the creation paths, including
wb_get_create_current().

> I don't think this qualifies as a fix. Can you drop the Fixes: and stable
> tags?

Dropped in v2. I'd tagged it because foreign flushes can't reach the
owning wb in exactly the case where its memcg has gone away, which
looked like a bug in the original implementation. But we avoided it in
userspace by syncing after the old container's last write, so agreed, it
doesn't need to go to stable.

Thanks,
Liz