[RFC 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 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@xxxxxxxxx>
---

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