mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Karl Mehltretter <kmehltretter@gmail.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Karl Mehltretter <kmehltretter@gmail.com>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [RFC PATCH] dcache: keep shrink_dcache_for_umount() making progress on busy roots
Date: Wed, 29 Jul 2026 02:59:33 +0200	[thread overview]
Message-ID: <20260729005933.15858-1-kmehltretter@gmail.com> (raw)

Commit e9895609cb7f ("wind ->s_roots via ->d_sib instead of ->d_hash")
moved secondary roots from ->d_hash to ->d_sib.  Secondary roots are
now d_unhashed(), so __d_drop() returns without removing them from
->s_roots.  Consequently, d_drop() in do_one_tree() no longer
guarantees progress through the list.

If a secondary root has an unexpected extra reference, do_one_tree()
reports it, but its final dput() cannot evict it.  The root remains
->s_roots.first and the loop selects it forever, holding ->s_umount for
write and repeatedly reporting the same dentry.

Before e9895609cb7f, ___d_drop() special-cased IS_ROOT dentries and
removed them from ->s_roots regardless of their refcount.  Commit
9c8c10e262e0 ("more graceful recovery in umount_collect()") deliberately
made busy dentries nonfatal: report them and finish the unmount rather
than BUG() while holding ->s_umount.

Detaching busy secondary roots from ->s_roots restores the previous
behavior.  A permanently leaked reference remains leaked after unmount;
if the extra reference is only delayed, its final dput() may run after
teardown has advanced and hit poisoned or freed filesystem state.  The
current code avoids that late cleanup only by looping indefinitely under
->s_umount for a real leak.

Restore that behavior by removing each live secondary root from
->s_roots after taking the temporary reference.  In the normal case,
dentry_unlist() finds ->d_sib already unhashed when eviction occurs.

Fixes: e9895609cb7f ("wind ->s_roots via ->d_sib instead of ->d_hash")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---

Notes:
    The reproducer mounts ramfs, allocates a fresh directory inode on its
    superblock, calls d_obtain_root(), and deliberately retains the returned
    dentry reference before unmounting.  The fresh inode ensures that no
    existing alias is returned.

                            d_unhashed()   reports   umount returns
      v7.1                             0         1              yes
      7.2-rc4+ unpatched               1     13013               no
      7.2-rc4+ patched                 1         1              yes

    All 13013 reports in the unpatched run identified the same dentry,
    confirming that the loop kept selecting ->s_roots.first.  With the patch,
    diagnostics match v7.1: one busy-dentry warning, one busy-inode warning,
    and no list-debug corruption.

    This injects a pre-existing refcount bug; no in-tree filesystem was found
    to create one by itself.

    Detaching the root guarantees progress for a permanent leak.  If the extra
    reference is merely delayed, however, its eventual dput() can run after
    teardown has advanced.  Leaving the root attached avoids that late cleanup
    but retries indefinitely for a permanent leak.

 fs/dcache.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3e9af9de7074..2aff15ee07fb 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1832,6 +1832,9 @@ void shrink_dcache_for_umount(struct super_block *sb)
 				wait_for_completion(&wait.completion);
 		} else {
 			dget_dlock(dentry);
+			// A busy root survives do_one_tree(); unlink it so the
+			// ->s_roots loop keeps making progress.
+			unlink_secondary_root(dentry);
 			spin_unlock(&dentry->d_lock);
 			do_one_tree(dentry);
 		}
-- 
2.53.0

             reply	other threads:[~2026-07-29  0:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29  0:59 Karl Mehltretter [this message]
2026-07-29 10:01 ` Christian Brauner
2026-07-31  0:08   ` Karl Mehltretter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729005933.15858-1-kmehltretter@gmail.com \
    --to=kmehltretter@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®