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

[Christian Brauner]

> 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")
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>

Wouldn't it make more sense to move this into do_one_tree()?