[PATCH] ceph: fix leaked inode reference on writeback abort at umount

[PATCH] ceph: fix leaked inode reference on writeback abort at umount

[matthew]

From: Matthew Brown <matthew@bargrove.com>

When ceph_submit_write() fails to take an osd_stopping_blocker -- which
only happens once the filesystem has begun unmounting
(mdsc->stopping >= CEPH_MDSC_STOPPING_FLUSHING) -- it redirties and
unlocks the folios it had already pulled out of
ceph_process_folio_batch() via redirty_page_for_writepage(), instead of
submitting them for writeback.

Those folios have already been through folio_clear_dirty_for_io(), so
PG_dirty is clear, but they are still "claimed" by the original
ceph_dirty_folio() call: ci->i_wrbuffer_ref is still counting them,
folio->private still pins a ceph_snap_context, and on the 0 -> 1
i_wrbuffer_ref transition ceph_dirty_folio() took an ihold() on the
inode. None of that has been released, because the folio was never
submitted, so writepages_finish() -- the only place that would release
it -- never runs.

redirty_page_for_writepage() calls folio_mark_dirty(), which invokes
->dirty_folio() again as soon as it observes PG_dirty clear. That
re-enters ceph_dirty_folio() on a folio that is still claimed.
ceph_dirty_folio() cannot distinguish "still claimed, never submitted"
from "genuinely clean again": it increments i_wrbuffer_ref again and
re-attaches a second snap_context reference, but does not take a second
ihold(), because ihold() only fires on the 0 -> 1 transition and
i_wrbuffer_ref is already non-zero.

The result is two i_wrbuffer_ref increments backed by only one ihold().
When the folio is eventually written back successfully on a later,
unrelated writeback pass, writepages_finish() drops exactly one wrbuffer
ref for it, so i_wrbuffer_ref never returns to 0 and the iput() that
should balance the original ihold() never runs. The inode's i_count
stays elevated, evict_inodes() cannot free it, and
generic_shutdown_super() trips:

  VFS: Busy inodes after unmount of ceph (ceph)
  kernel BUG at fs/super.c:650!

(a WARNING rather than a BUG without CONFIG_BUG_ON_DATA_CORRUPTION; on
an affected production node with that config set it is a fatal panic and
a full node reboot).

Fix it by releasing the existing wrbuffer/snap_context claim before
redirtying, mirroring what ceph_invalidate_folio() and
write_folio_nounlock() already do on their own abort paths. This
restores the invariant that ceph_dirty_folio() only ever runs against a
folio that is not already claimed, so it takes exactly one ihold() per
0 -> 1 transition and one matching wrbuffer ref per claimed folio.

Only the locked_pages loop is changed here. The preceding fbatch loop is
deliberately left alone: those entries can include folios that are
legitimately still under a different, in-flight write, where releasing
the claim would be a premature release. The two structurally similar
redirty-after-clear-dirty-for-io sites in ceph_process_folio_batch()
appear to share the same defect class and are left for a follow-up to
keep this fix minimal.

Fixes: fd7449d937e7 ("ceph: fix generic/421 test failure")
Link: https://tracker.ceph.com/issues/79149
Signed-off-by: Matthew Brown <matthew@bargrove.com>
---
Reproduced and verified on a 6.18.40 kernel: forcing the
ceph_submit_write() osd_stopping_blocker-failure branch and unmounting a
CephFS kernel-client mount with dirty pages reliably produces the
"VFS: Busy inodes after unmount of ceph" / fs/super.c:650 splat on the
unpatched kernel, and produces no such splat with this patch applied
(identical kernel, identical trigger). The original failure was captured
in production off-box via the serial console (pstore could not persist
the panic).

 fs/ceph/addr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index ecf33b6..97d9ef1 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -1426,6 +1426,16 @@ void ceph_shift_unused_folios_left(struct folio_batch *fbatch)
 	fbatch->nr = n;
 }
 
+static void ceph_undo_wrbuffer_claim(struct inode *inode, struct page *page)
+{
+	struct ceph_snap_context *snapc = detach_page_private(page);
+
+	if (!snapc)
+		return;
+	ceph_put_wrbuffer_cap_refs(ceph_inode(inode), 1, snapc);
+	ceph_put_snap_context(snapc);
+}
+
 static
 int ceph_submit_write(struct address_space *mapping,
 			struct writeback_control *wbc,
@@ -1489,6 +1499,7 @@ int ceph_submit_write(struct address_space *mapping,
 			if (!page)
 				continue;
 
+			ceph_undo_wrbuffer_claim(inode, page);
 			redirty_page_for_writepage(wbc, page);
 			unlock_page(page);
 		}
-- 
2.44.0

Re: ceph: fix leaked inode reference on writeback abort at umount

[Matt Brown]

Thanks both.

Alex, thank you for the correction -- you're right.  As you describe,
an aborted (never-submitted) write orphans its wrbuffer claim:
writepages_finish() is the only release path and never runs, so
i_wrbuffer_ref never returns to 0, the ihold() is never dropped, and
the inode can't be evicted.  v2 will rewrite the commit message around
that, and switch the helper to struct folio * / folio_detach_private().

Xiubo -- on the ownership distinction: folio_clear_dirty_for_io() is
the ownership-transfer point.  A folio reaches locked_pages only after
this call clears its dirty flag, and the successful
move_dirty_folio_in_page_array() NULLs it out of the fbatch, so at the
abort the two arrays are disjoint.  The locked_pages entries are
exactly the folios this call dirty-cleared, so it owns their claim and
must release it.  Everything still in the fbatch was never dirty-cleared
by this call -- under writeback by another request, already cleared by
another writer (folio_clear_dirty_for_io() == false), or not yet
reached -- so its claim belongs elsewhere and the fbatch loop only
redirties and unlocks.  I'll fold that into the v2 message.

The code is otherwise unchanged.

For what it's worth, I've confirmed the fix on one of our production
machines: busy-inode reboots on our CephFS clients dropped sharply
after deploying it.  While evaluating it we did catch one residual
fs/super.c:650 panic on the patched kernel, but via a different path --
preceded by "ceph: ... umount timed out", i.e. the umount drain wait
timing out with inodes still pinned, not the writeback-abort path this
patch addresses.  I've reported that separately as
https://tracker.ceph.com/issues/79436.

v2 to follow.

Best regards,

Matthew




On Wed, 12 Aug 2026 at 14:42, Alex Markuze <amarkuze@redhat.com> wrote:
>
> Hi matthew,
>
> NACK for now. Thanks for the patch, but I have some concerns
> that need a v2:
>
>   1. [major] commit message: Commit message describes a nonexistent redirty mechanism
>      The commit message claims `redirty_page_for_writepage()` calls
>      `folio_mark_dirty()`, which re-enters `ceph_dirty_folio()` and
>      double-increments `i_wrbuffer_ref`. This doesn't happen. The actual
>      chain is `redirty_page_for_writepage()` →
>      `folio_redirty_for_writepage()` → `filemap_dirty_folio()`, which
>      sets PG_dirty directly — it never dispatches through
>      `->dirty_folio`, so `ceph_dirty_folio()` is never re-entered. The
>      real bug is simpler: the wrbuffer claim (i_wrbuffer_ref,
>      snap_context, ihold) is leaked because `writepages_finish()` — the
>      only release path — never runs for writes that were never
>      submitted. During umount the osd_stopping_blocker keeps failing, so
>      the claim stays leaked and the inode can't be evicted.
>      Suggested fix: Drop the double-increment theory. Rewrite around the
>      actual failure: unsubmitted writes orphan their wrbuffer claim, and
>      releasing it before redirtying lets `i_wrbuffer_ref` reach 0 and
>      unblocks inode eviction.
>   2. [nit] fs/ceph/addr.c:1429: Helper uses page API instead of folio API
>      `ceph_undo_wrbuffer_claim()` takes `struct page *` and uses
>      `detach_page_private()`, while `ceph_invalidate_folio()` uses
>      `folio_detach_private()`. Functionally identical but folio API is
>      preferred for new code.
>      Suggested fix: Worth converting if respinning anyway, but not a
>      blocker.
>
> Code fix is correct but commit message wrongly claims redirty re-enters ceph_dirty_folio(); needs respin with accurate description of the leaked wrbuffer claim.
>
>
> --
> Alex Markuze
>

Re: [PATCH] ceph: fix leaked inode reference on writeback abort at umount

[Xiubo Li]

> Only the locked_pages loop is changed here. The preceding fbatch loop is > deliberately left alone...

The refcounting analysis and the fix look correct to me. In
particular, the claim needs to be dropped before
redirty_page_for_writepage(), otherwise ceph_dirty_folio() can observe
an already-claimed folio and create an extra wrbuffer reference
without taking another inode reference.

One thing I'd like to clarify is why the locked_pages entries are
guaranteed to own the claim being undone, while the entries handled
through fbatch may still belong to another in-flight write. This seems
to be the key distinction for why ceph_process_folio_batch() is
intentionally left unchanged. Assuming that ownership distinction is
guaranteed by the locking/state transitions, the fix looks good to me.

Thanks
Xiubo Li