mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/3]  writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs()
@ 2026-05-21  9:50 Baokun Li
  2026-05-21  9:50 ` [PATCH v4 1/3] " Baokun Li
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Baokun Li @ 2026-05-21  9:50 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: viro, brauner, jack, tj, linux-kernel

Hi all,

Changes since v3:
 * Collect RVB from Jan Kara. (Thanks for your review!)
 * Patch 3: Remove stale comments. (Reported by Sashiko)

Changes since v2:
 * Collect RVB from Jan Kara. (Thanks for your review!)
 * Patch 3: switch to wake_up_var() / wait_var_event() to drain
   s_isw_nr_in_flight. (Suggested by Christian Brauner and Sashiko)
 * Polish comments and changelogs.

Changes since v1:
 * Use a simple RCU-based fix (patch 1) that is easy to backport to
   older kernels; the per-sb refcount optimization is split out as a
   separate performance patch (patch 3). (Suggested by Jan Kara)

v1: https://patch.msgid.link/20260513094829.867648-1-libaokun@linux.alibaba.com
v2: https://patch.msgid.link/20260517142147.3354909-1-libaokun@linux.alibaba.com
v3: https://patch.msgid.link/20260518135349.1187628-1-libaokun@linux.alibaba.com

======

When a container exits, a race between cgroup_writeback_umount() and
inode_switch_wbs() / cleanup_offline_cgwb() can trigger
"VFS: Busy inodes after unmount" followed by a use-after-free on
percpu counters.

There is a window between inode_prepare_wbs_switch() returning true
(having passed the SB_ACTIVE check and grabbed the inode) and the
subsequent wb_queue_isw() call.  If cgroup_writeback_umount() observes
the global isw_nr_in_flight counter as non-zero but flush_workqueue()
finds nothing queued, it returns early -- leaving a held inode
reference that blocks evict_inodes() and a later iput() that hits
freed percpu counters.

Patch 1 closes the race by extending the RCU read-side critical
section to cover the window from inode_prepare_wbs_switch() through
wb_queue_isw(), and adding synchronize_rcu() in the umount path so
that all in-flight switchers complete queueing before
flush_workqueue() runs.  rcu_barrier() is intentionally retained so
the same hunk applies cleanly to stable trees that still queue
switches via queue_rcu_work().

Patch 2 removes the now-dead rcu_barrier() that was left over from
the queue_rcu_work() era (replaced by plain queue_work() in commit
e1b849cfa6b6 "writeback: Avoid contention on wb->list_lock when
switching inodes").  This is mainline-only.

Patch 3 replaces the global synchronize_rcu()/flush_workqueue() pair
with a per-sb counter (s_isw_nr_in_flight) plus three small helpers
(cgroup_writeback_pin / cgroup_writeback_unpin /
cgroup_writeback_drain), eliminating the global serialization
penalty.  This also reverts the RCU extension from patch 1 since the
per-sb counter makes it unnecessary.

Performance
-----------

Measured on a 16 vCPU QEMU guest, all kernels share the same .config.
Background load: 4 ext4 superblocks each running

  while :; do
      mkdir /sys/fs/cgroup/<tag>-tmp$N
      ( echo $BASHPID > <tag>-tmp$N/cgroup.procs
        dd if=/dev/zero of=$mp/burner bs=4k count=256 conv=notrunc \
       oflag=sync)
      rmdir /sys/fs/cgroup/<tag>-tmp$N
  done

This drives both inode_switch_wbs() (different cgroups writing the
same inode) and cleanup_offline_cgwb() (dying memcgs), keeping the
global isw_nr_in_flight non-zero throughout the run.  Latencies are
wall-clock around umount(8) on a separate target sb; only the target
sb's umount is measured.

Four kernels are compared at each step of the series:

  base       pre-fix mainline
  +race      base + patch 1 (race fix, keeps rcu_barrier)
  +rmbarrier +race + patch 2 (drop rcu_barrier)
  +persb     +rmbarrier + patch 3 (per-sb counter)

Target sb runs its own cgwb churn:

                p50      p95      p99      max
  base         99.7 ms 112.9 ms 112.9 ms 127.2 ms
  +race       110.2 ms 153.8 ms 153.8 ms 160.4 ms
  +rmbarrier   67.6 ms  88.3 ms  88.3 ms  96.8 ms
  +persb        7.9 ms  10.0 ms  10.0 ms  10.1 ms

Idle target umount under cross-sb cgwb-switch pressure:

                p50      p95      p99      max
  base         92.0 ms 123.5 ms 136.5 ms 141.3 ms
  +race       118.8 ms 154.6 ms 164.7 ms 165.3 ms
  +rmbarrier   62.7 ms  95.4 ms 108.1 ms 108.6 ms
  +persb        5.3 ms   6.9 ms   7.4 ms   7.4 ms

8 concurrent umounts of idle sbs under the same pressure:

                p50      p95      p99      max
  base        137.5 ms 166.9 ms 166.9 ms 171.3 ms
  +race       162.2 ms 183.9 ms 183.9 ms 217.0 ms
  +rmbarrier   61.3 ms  99.5 ms  99.5 ms 113.7 ms
  +persb        8.1 ms   9.1 ms   9.1 ms   9.5 ms

A no-pressure baseline run (no background load) measures ~5 ms p50
across all four kernels, validating that the methodology has no
systematic bias.

In-kernel cgroup_writeback_umount() cumulative cost across the same
run (bpftrace, ~340 calls covering all four scenarios):

                                cgroup_writeback_umount() time
  base                          21240 ms total  (~62 ms / call)
  +race      (rcu_barrier+sync) 24966 ms total  (~73 ms / call)
  +rmbarrier (synchronize_rcu)  12371 ms total  (~36 ms / call)
  +persb     (per-sb counter)    1.37 ms total  ( ~4 us / call)

Under +persb the wait_var_event() condition is true on entry
whenever the target sb has nothing in flight, so synchronize_rcu()
and flush_workqueue() are never called on this path.

Notes:

  - Patch 1 adds ~10-27 ms p50 over base by introducing
    synchronize_rcu().  This is the cost of closing the race
    correctly and is paid by stable backports as well.
  - Patch 2 ("drop rcu_barrier()") was expected to be a pure cleanup
    on mainline, but actually removes a real wait: rcu_barrier()
    drains call_rcu() callbacks from *all* subsystems, and the
    cgroup teardown path keeps that pipeline busy under this
    workload.  Removing it cuts ~43-101 ms p50 on top of patch 1.
  - Patch 3 (per-sb counter) replaces the global wait entirely; the
    target sb no longer waits for activity on unrelated sbs,
    recovering near-baseline latency in all three scenarios.

Comments and questions are, as always, welcome.

Thanks,
Baokun


Baokun Li (3):
  writeback: fix race between cgroup_writeback_umount() and
    inode_switch_wbs()
  writeback: drop now-unnecessary rcu_barrier() in
    cgroup_writeback_umount()
  writeback: use a per-sb counter to drain inode wb switches at umount

 fs/fs-writeback.c              | 71 +++++++++++++++++++++-------------
 include/linux/fs/super_types.h |  8 ++++
 2 files changed, 53 insertions(+), 26 deletions(-)

-- 
2.43.7


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs()
  2026-05-21  9:50 [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Baokun Li
@ 2026-05-21  9:50 ` Baokun Li
  2026-05-21 17:18   ` Tejun Heo
  2026-05-21  9:50 ` [PATCH v4 2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount() Baokun Li
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Baokun Li @ 2026-05-21  9:50 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: viro, brauner, jack, tj, linux-kernel, stable

When a container exits, the following BUG_ON() is occasionally triggered:

==================================================================
 VFS: Busy inodes after unmount of sdb (ext4)
 ------------[ cut here ]------------
 kernel BUG at fs/super.c:695!
 CPU: 3 PID: 6 Comm: containerd-shim Tainted: G OE K 6.6 #1
 pstate: 63400009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
 pc : generic_shutdown_super+0xf0/0x100
 lr : generic_shutdown_super+0xf0/0x100
 Call trace:
  generic_shutdown_super+0xf0/0x100
  kill_block_super+0x20/0x48
  ext4_kill_sb+0x28/0x60
  deactivate_locked_super+0x54/0x130
  deactivate_super+0x84/0xa0
  cleanup_mnt+0xa4/0x140
  __cleanup_mnt+0x18/0x28
  task_work_run+0x78/0xe0
  do_notify_resume+0x204/0x240
==================================================================

The root cause is a race between cgroup_writeback_umount() and
inode_switch_wbs()/cleanup_offline_cgwb(). There is a window between
inode_prepare_wbs_switch() returning true and the subsequent
wb_queue_isw() call. Following is the process that triggers the issue:

      CPU A (umount)           |          CPU B (writeback)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                 inode_switch_wbs/cleanup_offline_cgwb
                                  atomic_inc(&isw_nr_in_flight)
                                  inode_prepare_wbs_switch
                                   -> passes SB_ACTIVE check
                                   __iget(inode)
 generic_shutdown_super
  sb->s_flags &= ~SB_ACTIVE
  cgroup_writeback_umount(sb)
   smp_mb()
   atomic_read(&isw_nr_in_flight)
   rcu_barrier()
    -> no pending RCU callbacks
   flush_workqueue(isw_wq)
    -> nothing queued, returns
  evict_inodes(sb)
   -> Inode skipped as isw still holds a ref.
  sop->put_super(sb)
   /* destroys percpu counters */
  -> VFS: Busy inodes after unmount!
                                  wb_queue_isw()
                                   queue_work(isw_wq, ...)
                                  /* later in work function */
                                  inode_switch_wbs_work_fn
                                   process_inode_switch_wbs
                                    iput() -> evict
                                     percpu_counter_dec() // UAF!

Fix this by extending the RCU read-side critical section in
inode_switch_wbs() and cleanup_offline_cgwb() to cover from
inode_prepare_wbs_switch() through wb_queue_isw().  Since there is
no sleep in this window, rcu_read_lock() can be used.  Then add a
synchronize_rcu() in cgroup_writeback_umount() before the existing
rcu_barrier(), so that all in-flight switchers that have passed the
SB_ACTIVE check have completed queue_work() before flush_workqueue()
is called.

The existing rcu_barrier() is intentionally retained so this fix can
be backported unchanged to stable kernels (5.10.y, 6.6.y, ...) that
still queue switches via queue_rcu_work(). It is a no-op on current
mainline (since commit e1b849cfa6b6 ("writeback: Avoid contention on
wb->list_lock when switching inodes")) and is removed in a follow-up
patch.

Fixes: a1a0e23e4903 ("writeback: flush inode cgroup wb switches instead of pinning super_block")
Cc: stable@vger.kernel.org
Suggested-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/all/mxnjq2l6guusfchvauxr3v7c4bwjasybxlleqbbh4efloeqspz@iqylk76ohufz
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 fs/fs-writeback.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index a65694cbfe68..6766de9f9d75 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -660,12 +660,19 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
 
 	atomic_inc(&isw_nr_in_flight);
 
-	/* find and pin the new wb */
+	/*
+	 * Paired with synchronize_rcu() in cgroup_writeback_umount():
+	 * holding rcu_read_lock across inode_prepare_wbs_switch()
+	 * (covering the SB_ACTIVE check and the inode grab) and
+	 * wb_queue_isw() ensures synchronize_rcu() cannot return until
+	 * the work is queued, so the subsequent flush_workqueue() will
+	 * wait for the switch.
+	 */
 	rcu_read_lock();
+	/* find and pin the new wb */
 	memcg_css = css_from_id(new_wb_id, &memory_cgrp_subsys);
 	if (memcg_css && !css_tryget(memcg_css))
 		memcg_css = NULL;
-	rcu_read_unlock();
 	if (!memcg_css)
 		goto out_free;
 
@@ -681,9 +688,11 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
 
 	trace_inode_switch_wbs_queue(inode->i_wb, new_wb, 1);
 	wb_queue_isw(new_wb, isw);
+	rcu_read_unlock();
 	return;
 
 out_free:
+	rcu_read_unlock();
 	atomic_dec(&isw_nr_in_flight);
 	if (new_wb)
 		wb_put(new_wb);
@@ -741,6 +750,14 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 		new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */
 
 	nr = 0;
+	/*
+	 * Paired with synchronize_rcu() in cgroup_writeback_umount().
+	 * Holding rcu_read_lock across the SB_ACTIVE check, the inode grab
+	 * and wb_queue_isw() ensures synchronize_rcu() cannot return until
+	 * the work is queued, so the subsequent flush_workqueue() will wait
+	 * for the switch.
+	 */
+	rcu_read_lock();
 	spin_lock(&wb->list_lock);
 	/*
 	 * In addition to the inodes that have completed writeback, also switch
@@ -758,6 +775,7 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 
 	/* no attached inodes? bail out */
 	if (nr == 0) {
+		rcu_read_unlock();
 		atomic_dec(&isw_nr_in_flight);
 		wb_put(new_wb);
 		kfree(isw);
@@ -766,6 +784,7 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 
 	trace_inode_switch_wbs_queue(wb, new_wb, nr);
 	wb_queue_isw(new_wb, isw);
+	rcu_read_unlock();
 
 	return restart;
 }
@@ -1221,6 +1240,14 @@ void cgroup_writeback_umount(struct super_block *sb)
 	smp_mb();
 
 	if (atomic_read(&isw_nr_in_flight)) {
+		/*
+		 * Paired with rcu_read_lock() in inode_switch_wbs() and
+		 * cleanup_offline_cgwb().  synchronize_rcu() waits for any
+		 * in-flight switcher that already passed the SB_ACTIVE check
+		 * to finish queueing its work, so flush_workqueue() below
+		 * will then drain it.
+		 */
+		synchronize_rcu();
 		/*
 		 * Use rcu_barrier() to wait for all pending callbacks to
 		 * ensure that all in-flight wb switches are in the workqueue.
-- 
2.43.7


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount()
  2026-05-21  9:50 [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Baokun Li
  2026-05-21  9:50 ` [PATCH v4 1/3] " Baokun Li
@ 2026-05-21  9:50 ` Baokun Li
  2026-05-21 17:18   ` Tejun Heo
  2026-05-21  9:50 ` [PATCH v4 3/3] writeback: use a per-sb counter to drain inode wb switches at umount Baokun Li
  2026-05-22 10:07 ` [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Christian Brauner
  3 siblings, 1 reply; 8+ messages in thread
From: Baokun Li @ 2026-05-21  9:50 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: viro, brauner, jack, tj, linux-kernel

Commit e1b849cfa6b6 ("writeback: Avoid contention on wb->list_lock when
switching inodes") replaced the queue_rcu_work() based scheduling of
inode wb switches with a plain queue_work().  Since then no switcher
goes through call_rcu(), so rcu_barrier() in cgroup_writeback_umount()
has no callbacks of its own to wait for.  It still drains unrelated
call_rcu() callbacks from other subsystems on busy systems, which
incidentally slows umount down; drop it.

Fixes: e1b849cfa6b6 ("writeback: Avoid contention on wb->list_lock when switching inodes")
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 fs/fs-writeback.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 6766de9f9d75..325a30cc35bf 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -1248,11 +1248,6 @@ void cgroup_writeback_umount(struct super_block *sb)
 		 * will then drain it.
 		 */
 		synchronize_rcu();
-		/*
-		 * Use rcu_barrier() to wait for all pending callbacks to
-		 * ensure that all in-flight wb switches are in the workqueue.
-		 */
-		rcu_barrier();
 		flush_workqueue(isw_wq);
 	}
 }
-- 
2.43.7


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 3/3] writeback: use a per-sb counter to drain inode wb switches at umount
  2026-05-21  9:50 [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Baokun Li
  2026-05-21  9:50 ` [PATCH v4 1/3] " Baokun Li
  2026-05-21  9:50 ` [PATCH v4 2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount() Baokun Li
@ 2026-05-21  9:50 ` Baokun Li
  2026-05-21 17:40   ` Tejun Heo
  2026-05-22 10:07 ` [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Christian Brauner
  3 siblings, 1 reply; 8+ messages in thread
From: Baokun Li @ 2026-05-21  9:50 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: viro, brauner, jack, tj, linux-kernel

Tracking in-flight inode wb switches with a single global counter
(isw_nr_in_flight) plus a synchronize_rcu() based wait in
cgroup_writeback_umount() forces every umount to take a global hit
whenever any other superblock on the system has wb switches in flight,
even if the superblock being unmounted has none of its own.

Replace the global synchronize_rcu()/flush_workqueue() pair with a
per-sb counter, s_isw_nr_in_flight, plus three small helpers:

  - cgroup_writeback_pin(sb)   - increment counter
  - cgroup_writeback_unpin(sb) - decrement and wake drainer if last
  - cgroup_writeback_drain(sb) - wait for counter to reach zero

The wiring is:

  - inode_prepare_wbs_switch() pins before checking SB_ACTIVE and
    grabbing the inode; failure paths unpin before returning.  A
    lockless SB_ACTIVE check at the top of the function lets us skip
    the atomic_inc/smp_mb dance once SB_ACTIVE has been cleared (it
    is monotonic and never set back).
  - process_inode_switch_wbs() unpins after the matching iput().
  - cgroup_writeback_umount() drains the per-sb counter via
    wait_var_event().

The smp_mb() pair between inode_prepare_wbs_switch() and
cgroup_writeback_umount() keeps the SB_ACTIVE / counter ordering:
either the umounter sees a non-zero counter and waits, or the
switcher sees SB_ACTIVE cleared and aborts before grabbing the
inode.

The global isw_nr_in_flight is left in place, since it is still used
to throttle in-flight switches via WB_FRN_MAX_IN_FLIGHT.

The rcu_read_lock() extension in inode_switch_wbs() and
cleanup_offline_cgwb() that the race fix added is no longer needed
and is reverted; the synchronize_rcu() that the race fix added to
cgroup_writeback_umount() is dropped as well.

The following numbers were measured on a 16 vCPU QEMU guest with 4
background superblocks each churning "create memcg -> write 1 MiB ->
rmdir memcg" to keep the global isw_nr_in_flight non-zero.  Latencies
are wall-clock around umount(8); only the target sb's umount is
measured.

Target sb runs its own cgwb churn:

                              p50      p95      p99      max
  global synchronize_rcu()   67.6 ms  88.3 ms  88.3 ms  96.8 ms
  per-sb counter (this)       7.9 ms  10.0 ms  10.0 ms  10.1 ms

Idle target umount latency under cross-sb cgwb-switch pressure:

                              p50      p95      p99      max
  global synchronize_rcu()   62.7 ms  95.4 ms 108.1 ms 108.6 ms
  per-sb counter (this)       5.3 ms   6.9 ms   7.4 ms   7.4 ms
  no-pressure baseline        4.9 ms   5.9 ms   6.3 ms   6.7 ms

8 concurrent umounts of idle sbs under the same pressure:

                              p50      p95      max
  global synchronize_rcu()   61.3 ms  99.5 ms 113.7 ms
  per-sb counter (this)       8.1 ms   9.1 ms   9.5 ms

In-kernel cgroup_writeback_umount() time across the same run
(bpftrace, ~340 calls covering all scenarios):

  global synchronize_rcu()    12371 ms total (~36 ms / call)
  per-sb counter (this)        1.37 ms total ( ~4 us / call)

Suggested-by: Christian Brauner <brauner@kernel.org>
Link: https://lore.kernel.org/r/177910456953.488929.2169908940676707307.b4-review@b4
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
---
 fs/fs-writeback.c              | 97 ++++++++++++++++------------------
 include/linux/fs/super_types.h |  8 +++
 2 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 325a30cc35bf..900ad7818bd4 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -497,6 +497,23 @@ static bool inode_do_switch_wbs(struct inode *inode,
 	return switched;
 }
 
+static inline void cgroup_writeback_pin(struct super_block *sb)
+{
+	atomic_inc(&sb->s_isw_nr_in_flight);
+}
+
+static inline void cgroup_writeback_unpin(struct super_block *sb)
+{
+	if (atomic_dec_and_test(&sb->s_isw_nr_in_flight))
+		wake_up_var(&sb->s_isw_nr_in_flight);
+}
+
+static inline void cgroup_writeback_drain(struct super_block *sb)
+{
+	wait_var_event(&sb->s_isw_nr_in_flight,
+		       !atomic_read(&sb->s_isw_nr_in_flight));
+}
+
 static void process_inode_switch_wbs(struct bdi_writeback *new_wb,
 				     struct inode_switch_wbs_context *isw)
 {
@@ -554,8 +571,12 @@ static void process_inode_switch_wbs(struct bdi_writeback *new_wb,
 		wb_put_many(old_wb, nr_switched);
 	}
 
-	for (inodep = isw->inodes; *inodep; inodep++)
+	for (inodep = isw->inodes; *inodep; inodep++) {
+		struct super_block *sb = (*inodep)->i_sb;
+
 		iput(*inodep);
+		cgroup_writeback_unpin(sb);
+	}
 	wb_put(new_wb);
 	kfree(isw);
 	atomic_dec(&isw_nr_in_flight);
@@ -598,16 +619,19 @@ void inode_switch_wbs_work_fn(struct work_struct *work)
 static bool inode_prepare_wbs_switch(struct inode *inode,
 				     struct bdi_writeback *new_wb)
 {
+	/* Avoid the atomic_inc/smp_mb dance once SB_ACTIVE is gone. */
+	if (!(inode->i_sb->s_flags & SB_ACTIVE))
+		return false;
+
 	/*
-	 * Paired with smp_mb() in cgroup_writeback_umount().
-	 * isw_nr_in_flight must be increased before checking SB_ACTIVE and
-	 * grabbing an inode, otherwise isw_nr_in_flight can be observed as 0
-	 * in cgroup_writeback_umount() and the isw_wq will be not flushed.
+	 * Pairs with smp_mb() in cgroup_writeback_umount(): the umounter either
+	 * sees a non-zero counter and waits, or we see SB_ACTIVE clear below.
 	 */
+	cgroup_writeback_pin(inode->i_sb);
 	smp_mb();
 
 	if (IS_DAX(inode))
-		return false;
+		goto out_unpin;
 
 	/* while holding I_WB_SWITCH, no one else can update the association */
 	spin_lock(&inode->i_lock);
@@ -615,13 +639,17 @@ static bool inode_prepare_wbs_switch(struct inode *inode,
 	    inode_state_read(inode) & (I_WB_SWITCH | I_FREEING | I_WILL_FREE) ||
 	    inode_to_wb(inode) == new_wb) {
 		spin_unlock(&inode->i_lock);
-		return false;
+		goto out_unpin;
 	}
 	inode_state_set(inode, I_WB_SWITCH);
 	__iget(inode);
 	spin_unlock(&inode->i_lock);
 
 	return true;
+
+out_unpin:
+	cgroup_writeback_unpin(inode->i_sb);
+	return false;
 }
 
 static void wb_queue_isw(struct bdi_writeback *wb,
@@ -660,19 +688,12 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
 
 	atomic_inc(&isw_nr_in_flight);
 
-	/*
-	 * Paired with synchronize_rcu() in cgroup_writeback_umount():
-	 * holding rcu_read_lock across inode_prepare_wbs_switch()
-	 * (covering the SB_ACTIVE check and the inode grab) and
-	 * wb_queue_isw() ensures synchronize_rcu() cannot return until
-	 * the work is queued, so the subsequent flush_workqueue() will
-	 * wait for the switch.
-	 */
-	rcu_read_lock();
 	/* find and pin the new wb */
+	rcu_read_lock();
 	memcg_css = css_from_id(new_wb_id, &memory_cgrp_subsys);
 	if (memcg_css && !css_tryget(memcg_css))
 		memcg_css = NULL;
+	rcu_read_unlock();
 	if (!memcg_css)
 		goto out_free;
 
@@ -688,11 +709,9 @@ static void inode_switch_wbs(struct inode *inode, int new_wb_id)
 
 	trace_inode_switch_wbs_queue(inode->i_wb, new_wb, 1);
 	wb_queue_isw(new_wb, isw);
-	rcu_read_unlock();
 	return;
 
 out_free:
-	rcu_read_unlock();
 	atomic_dec(&isw_nr_in_flight);
 	if (new_wb)
 		wb_put(new_wb);
@@ -750,14 +769,6 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 		new_wb = &wb->bdi->wb; /* wb_get() is noop for bdi's wb */
 
 	nr = 0;
-	/*
-	 * Paired with synchronize_rcu() in cgroup_writeback_umount().
-	 * Holding rcu_read_lock across the SB_ACTIVE check, the inode grab
-	 * and wb_queue_isw() ensures synchronize_rcu() cannot return until
-	 * the work is queued, so the subsequent flush_workqueue() will wait
-	 * for the switch.
-	 */
-	rcu_read_lock();
 	spin_lock(&wb->list_lock);
 	/*
 	 * In addition to the inodes that have completed writeback, also switch
@@ -775,7 +786,6 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 
 	/* no attached inodes? bail out */
 	if (nr == 0) {
-		rcu_read_unlock();
 		atomic_dec(&isw_nr_in_flight);
 		wb_put(new_wb);
 		kfree(isw);
@@ -784,7 +794,6 @@ bool cleanup_offline_cgwb(struct bdi_writeback *wb)
 
 	trace_inode_switch_wbs_queue(wb, new_wb, nr);
 	wb_queue_isw(new_wb, isw);
-	rcu_read_unlock();
 
 	return restart;
 }
@@ -1217,39 +1226,27 @@ int cgroup_writeback_by_id(u64 bdi_id, int memcg_id,
 }
 
 /**
- * cgroup_writeback_umount - flush inode wb switches for umount
+ * cgroup_writeback_umount - wait for in-flight inode wb switches on @sb
  * @sb: target super_block
  *
- * This function is called when a super_block is about to be destroyed and
- * flushes in-flight inode wb switches.  An inode wb switch goes through
- * RCU and then workqueue, so the two need to be flushed in order to ensure
- * that all previously scheduled switches are finished.  As wb switches are
- * rare occurrences and synchronize_rcu() can take a while, perform
- * flushing iff wb switches are in flight.
+ * Wait until every inode wb switch that already passed the SB_ACTIVE
+ * check on this superblock has been completed by the worker.  Since
+ * SB_ACTIVE is cleared before this is called, no new switches can start
+ * for @sb, so s_isw_nr_in_flight will monotonically drop to zero.
  */
 void cgroup_writeback_umount(struct super_block *sb)
 {
-
 	if (!(sb->s_bdi->capabilities & BDI_CAP_WRITEBACK))
 		return;
 
 	/*
-	 * SB_ACTIVE should be reliably cleared before checking
-	 * isw_nr_in_flight, see generic_shutdown_super().
+	 * Pairs with smp_mb() in inode_prepare_wbs_switch(): we either observe
+	 * a non-zero counter and wait, or the switcher sees SB_ACTIVE clear
+	 * (cleared by generic_shutdown_super()) and bails before grabbing the
+	 * inode.
 	 */
 	smp_mb();
-
-	if (atomic_read(&isw_nr_in_flight)) {
-		/*
-		 * Paired with rcu_read_lock() in inode_switch_wbs() and
-		 * cleanup_offline_cgwb().  synchronize_rcu() waits for any
-		 * in-flight switcher that already passed the SB_ACTIVE check
-		 * to finish queueing its work, so flush_workqueue() below
-		 * will then drain it.
-		 */
-		synchronize_rcu();
-		flush_workqueue(isw_wq);
-	}
+	cgroup_writeback_drain(sb);
 }
 
 static int __init cgroup_writeback_init(void)
diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
index 383050e7fdf5..1ab4e2265129 100644
--- a/include/linux/fs/super_types.h
+++ b/include/linux/fs/super_types.h
@@ -274,6 +274,14 @@ struct super_block {
 
 	/* number of fserrors that are being sent to fsnotify/filesystems */
 	refcount_t				s_pending_errors;
+
+#ifdef CONFIG_CGROUP_WRITEBACK
+	/*
+	 * Number of in-flight inode wb switches for this sb.  Drained by
+	 * cgroup_writeback_umount() before tear-down.
+	 */
+	atomic_t				s_isw_nr_in_flight;
+#endif
 } __randomize_layout;
 
 /*
-- 
2.43.7


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs()
  2026-05-21  9:50 ` [PATCH v4 1/3] " Baokun Li
@ 2026-05-21 17:18   ` Tejun Heo
  0 siblings, 0 replies; 8+ messages in thread
From: Tejun Heo @ 2026-05-21 17:18 UTC (permalink / raw)
  To: Baokun Li; +Cc: linux-fsdevel, viro, brauner, jack, linux-kernel, stable

On Thu, May 21, 2026 at 05:50:14PM +0800, Baokun Li wrote:
> When a container exits, the following BUG_ON() is occasionally triggered:
> 
> ==================================================================
>  VFS: Busy inodes after unmount of sdb (ext4)
>  ------------[ cut here ]------------
>  kernel BUG at fs/super.c:695!
>  CPU: 3 PID: 6 Comm: containerd-shim Tainted: G OE K 6.6 #1
>  pstate: 63400009 (nZCv daif +PAN -UAO +TCO +DIT -SSBS BTYPE=--)
>  pc : generic_shutdown_super+0xf0/0x100
>  lr : generic_shutdown_super+0xf0/0x100
>  Call trace:
>   generic_shutdown_super+0xf0/0x100
>   kill_block_super+0x20/0x48
>   ext4_kill_sb+0x28/0x60
>   deactivate_locked_super+0x54/0x130
>   deactivate_super+0x84/0xa0
>   cleanup_mnt+0xa4/0x140
>   __cleanup_mnt+0x18/0x28
>   task_work_run+0x78/0xe0
>   do_notify_resume+0x204/0x240
> ==================================================================
> 
> The root cause is a race between cgroup_writeback_umount() and
> inode_switch_wbs()/cleanup_offline_cgwb(). There is a window between
> inode_prepare_wbs_switch() returning true and the subsequent
> wb_queue_isw() call. Following is the process that triggers the issue:
> 
>       CPU A (umount)           |          CPU B (writeback)
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                  inode_switch_wbs/cleanup_offline_cgwb
>                                   atomic_inc(&isw_nr_in_flight)
>                                   inode_prepare_wbs_switch
>                                    -> passes SB_ACTIVE check
>                                    __iget(inode)
>  generic_shutdown_super
>   sb->s_flags &= ~SB_ACTIVE
>   cgroup_writeback_umount(sb)
>    smp_mb()
>    atomic_read(&isw_nr_in_flight)
>    rcu_barrier()
>     -> no pending RCU callbacks
>    flush_workqueue(isw_wq)
>     -> nothing queued, returns
>   evict_inodes(sb)
>    -> Inode skipped as isw still holds a ref.
>   sop->put_super(sb)
>    /* destroys percpu counters */
>   -> VFS: Busy inodes after unmount!
>                                   wb_queue_isw()
>                                    queue_work(isw_wq, ...)
>                                   /* later in work function */
>                                   inode_switch_wbs_work_fn
>                                    process_inode_switch_wbs
>                                     iput() -> evict
>                                      percpu_counter_dec() // UAF!
> 
> Fix this by extending the RCU read-side critical section in
> inode_switch_wbs() and cleanup_offline_cgwb() to cover from
> inode_prepare_wbs_switch() through wb_queue_isw().  Since there is
> no sleep in this window, rcu_read_lock() can be used.  Then add a
> synchronize_rcu() in cgroup_writeback_umount() before the existing
> rcu_barrier(), so that all in-flight switchers that have passed the
> SB_ACTIVE check have completed queue_work() before flush_workqueue()
> is called.
> 
> The existing rcu_barrier() is intentionally retained so this fix can
> be backported unchanged to stable kernels (5.10.y, 6.6.y, ...) that
> still queue switches via queue_rcu_work(). It is a no-op on current
> mainline (since commit e1b849cfa6b6 ("writeback: Avoid contention on
> wb->list_lock when switching inodes")) and is removed in a follow-up
> patch.
> 
> Fixes: a1a0e23e4903 ("writeback: flush inode cgroup wb switches instead of pinning super_block")
> Cc: stable@vger.kernel.org
> Suggested-by: Jan Kara <jack@suse.cz>
> Link: https://lore.kernel.org/all/mxnjq2l6guusfchvauxr3v7c4bwjasybxlleqbbh4efloeqspz@iqylk76ohufz
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount()
  2026-05-21  9:50 ` [PATCH v4 2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount() Baokun Li
@ 2026-05-21 17:18   ` Tejun Heo
  0 siblings, 0 replies; 8+ messages in thread
From: Tejun Heo @ 2026-05-21 17:18 UTC (permalink / raw)
  To: Baokun Li; +Cc: linux-fsdevel, viro, brauner, jack, linux-kernel

On Thu, May 21, 2026 at 05:50:15PM +0800, Baokun Li wrote:
> Commit e1b849cfa6b6 ("writeback: Avoid contention on wb->list_lock when
> switching inodes") replaced the queue_rcu_work() based scheduling of
> inode wb switches with a plain queue_work().  Since then no switcher
> goes through call_rcu(), so rcu_barrier() in cgroup_writeback_umount()
> has no callbacks of its own to wait for.  It still drains unrelated
> call_rcu() callbacks from other subsystems on busy systems, which
> incidentally slows umount down; drop it.
> 
> Fixes: e1b849cfa6b6 ("writeback: Avoid contention on wb->list_lock when switching inodes")
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 3/3] writeback: use a per-sb counter to drain inode wb switches at umount
  2026-05-21  9:50 ` [PATCH v4 3/3] writeback: use a per-sb counter to drain inode wb switches at umount Baokun Li
@ 2026-05-21 17:40   ` Tejun Heo
  0 siblings, 0 replies; 8+ messages in thread
From: Tejun Heo @ 2026-05-21 17:40 UTC (permalink / raw)
  To: Baokun Li; +Cc: linux-fsdevel, viro, brauner, jack, linux-kernel

On Thu, May 21, 2026 at 05:50:16PM +0800, Baokun Li wrote:
...
> Suggested-by: Christian Brauner <brauner@kernel.org>
> Link: https://lore.kernel.org/r/177910456953.488929.2169908940676707307.b4-review@b4
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs()
  2026-05-21  9:50 [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Baokun Li
                   ` (2 preceding siblings ...)
  2026-05-21  9:50 ` [PATCH v4 3/3] writeback: use a per-sb counter to drain inode wb switches at umount Baokun Li
@ 2026-05-22 10:07 ` Christian Brauner
  3 siblings, 0 replies; 8+ messages in thread
From: Christian Brauner @ 2026-05-22 10:07 UTC (permalink / raw)
  To: linux-fsdevel, Baokun Li; +Cc: Christian Brauner, viro, jack, tj, linux-kernel

On Thu, 21 May 2026 17:50:13 +0800, Baokun Li wrote:
> Changes since v3:
>  * Collect RVB from Jan Kara. (Thanks for your review!)
>  * Patch 3: Remove stale comments. (Reported by Sashiko)
> 
> Changes since v2:
>  * Collect RVB from Jan Kara. (Thanks for your review!)
>  * Patch 3: switch to wake_up_var() / wait_var_event() to drain
>    s_isw_nr_in_flight. (Suggested by Christian Brauner and Sashiko)
>  * Polish comments and changelogs.
> 
> [...]

Applied to the vfs-7.2.writeback branch of the vfs/vfs.git tree.
Patches in the vfs-7.2.writeback branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.2.writeback

[1/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs()
      https://git.kernel.org/vfs/vfs/c/cba38ec4cbd3
[2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount()
      https://git.kernel.org/vfs/vfs/c/e90a6d668e26
[3/3] writeback: use a per-sb counter to drain inode wb switches at umount
      https://git.kernel.org/vfs/vfs/c/31c1d19ead2c

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-22 10:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-21  9:50 [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Baokun Li
2026-05-21  9:50 ` [PATCH v4 1/3] " Baokun Li
2026-05-21 17:18   ` Tejun Heo
2026-05-21  9:50 ` [PATCH v4 2/3] writeback: drop now-unnecessary rcu_barrier() in cgroup_writeback_umount() Baokun Li
2026-05-21 17:18   ` Tejun Heo
2026-05-21  9:50 ` [PATCH v4 3/3] writeback: use a per-sb counter to drain inode wb switches at umount Baokun Li
2026-05-21 17:40   ` Tejun Heo
2026-05-22 10:07 ` [PATCH v4 0/3] writeback: fix race between cgroup_writeback_umount() and inode_switch_wbs() Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®