Re: [PATCH] dcache: rename d_genocide()

[Al Viro <viro@zeniv.linux.org.uk>]

On Sat, Feb 10, 2024 at 12:06:43PM +0200, Amir Goldstein wrote:

> I am not usually for PC culture and I know that you are on team
> "freedom of speech" ;-), but IMO this one stood out for its high ratio
> of bad taste vs. usefulness.
> 
> The patch is based on your revert of "get rid of DCACHE_GENOCIDE".
> I was hoping that you could queue my patch along with the revert.
> 
> BTW, why was d_genocide() only dropping refcounts on the s_root tree
> and not on the s_roots trees like shrink_dcache_for_umount()?
> Is it because dentries on s_roots are not supposed to be hashed?

Because secondary roots make no sense for "everything's in dcache"
kind of filesystems, mostly.

FWIW, I don't believe that cosmetic renaming is the right thing to
do here.  The real issue here is that those fs-pinned dentries are
hard to distinguish.  The rule is "if dentry on such filesystem is
positive and hashed, that contributes 1 to its refcount".

Unfortunately, that doesn't come with sane locking rules.
If nothing else, I would rather have an explicit flag set along
with bumping ->d_count on creation side and cleared along with
dropping refcount on removal, both under ->d_lock.

Another piece of ugliness is the remaining places that try to
open-code simple_recursive_removal(); they get it wrong more
often than not.  Connected to the previous, since that's where
those games with simple_unlink()/simple_rmdir() and associated
fun with refcounts tend to happen.

I'm trying to untangle that mess - on top of that revert,
obviously.  Interposing your patch in there is doable, of course,
but it's not particularly useful, IMO, especially since the
whole d_genocide() thing is quite likely to disappear, turning
kill_litter_super() into an alias for kill_anon_super().