[PATCH] dcache: keep shrink_dcache_for_umount() making progress on busy roots

[Karl Mehltretter]

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 is still busy once do_one_tree() is done with it,
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.

The root does not need a leaked reference of its own for that.  Every
child pins its parent (d_alloc() takes a reference on it) and
umount_check() deliberately reports a busy descendant instead of
complaining about its ancestors, so a single leaked dentry reference
anywhere below a secondary root is enough.  For filesystems that build
->s_root with d_obtain_root() - nfs, ceph, nilfs2 snapshot mounts -
that is the entire tree.

Before e9895609cb7f, ___d_drop() special-cased IS_ROOT dentries and
removed them from ->s_roots regardless of their refcount, so the
d_drop() in do_one_tree() detached the root from the superblock no
matter what.  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.

Restore that by detaching the root in do_one_tree() itself, next to
the d_drop() that used to do it.  That covers both callers - the
->s_roots loop and ->s_root, which for the filesystems above is a
secondary root as well.  In the normal case dentry_unlist() finds
->d_sib already unhashed when eviction occurs.

A permanently leaked reference remains leaked after unmount, as it did
before e9895609cb7f; if the extra reference is merely delayed, its
final dput() may run after teardown has advanced.  Leaving the root on
->s_roots is not an alternative: the superblock would then be freed
with a live dentry still linked into it, and that dentry's
dentry_unlist() would take ->s_roots_lock on freed memory.

Christian Brauner <brauner@xxxxxxxxxx> says:
Moved the ->s_roots removal from the shrink_dcache_for_umount() loop
into do_one_tree(), so a busy ->s_root obtained from d_obtain_root() is
detached on the first pass instead of being reported a second time when
the loop picks it off ->s_roots.  Extended the commit message with the
pinned-ancestor case.

Fixes: e9895609cb7f ("wind ->s_roots via ->d_sib instead of ->d_hash")
Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
Link: https://patch.msgid.link/20260729005933.15858-1-kmehltretter@xxxxxxxxx
Signed-off-by: Christian Brauner (Amutable) <brauner@xxxxxxxxxx>
---
 fs/dcache.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3e9af9de7074..aded2564b589 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1794,7 +1794,12 @@ static void do_one_tree(struct dentry *dentry)
 {
 	shrink_dcache_tree(dentry, true);
 	d_walk(dentry, dentry, umount_check);
-	d_drop(dentry);
+	spin_lock(&dentry->d_lock);
+	__d_drop(dentry);
+	// a busy root survives the dput() below; don't leave it on ->s_roots
+	if (unlikely(!hlist_unhashed(&dentry->d_sib)))
+		unlink_secondary_root(dentry);
+	spin_unlock(&dentry->d_lock);
 	dput(dentry);
 }
 
-- 
2.53.0