From 78db93943210df61c8446aae35af6836e2cf04aa Mon Sep 17 00:00:00 2001 From: Karl Mehltretter Date: Wed, 29 Jul 2026 02:59:33 +0200 Subject: [PATCH] dcache: keep shrink_dcache_for_umount() making progress on busy roots 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 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 Link: https://patch.msgid.link/20260729005933.15858-1-kmehltretter@gmail.com Signed-off-by: Christian Brauner (Amutable) --- fs/dcache.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/dcache.c b/fs/dcache.c index 2aee85f3fbaa..1b1a81f10da6 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 so 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); }