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@xxxxxxxxxx> 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
>