Re: [PATCHES][RFC] the meat of tree-in-dcache series

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

On Sat, Sep 20, 2025 at 09:26:27AM -0700, Linus Torvalds wrote:

> Anyway, apart from that I only had one reaction: I think
> d_make_discardable() should have a
> 
>         WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT))
> 
> because without that I think it can mistakenly be used as some kind of
> "dput that always takes the dentry lock", which seems bad.
> 
> Or was that intentional for some reason?

In the end - sure, we want that.  But in the meanwhile that would require
separate variants of simple_unlink()/simple_recursive_removal()/etc.
for those filesystems that are already marking persistent ones, with
the only difference being that warning.

A lot more noise that way.

So I'd prefer to put a warning in the source for the time being.  In principle,
by the end of series as posted we are down to very few filesystems that use
simple_unlink() and friends without having marked them persistent in the
first place, so it would be possible to put a "make d_make_discardable() warn
if it's not marked persistent, add variants of simple_unlink()/simple_rmdir()/
simple_recursive_removal()/locked_recursive_removal() that would *NOT* warn
and switch the handful of unconverted users to calling those", but...

By the end of series as posted that's down to nfsctl, rpc_pipe, securityfs,
configfs and apparmorfs.  The first 3 - because they used to have subseries
of their own in separate branches, with corresponding conversion commits
sitting on top of merges (#work.nfsctl is the last of those branches).
No real obstacle to moving them into the queue, I just wanted to post it
for review before we get to -rc7.

The remaining two (configfs and apparmor) are special enough to warrant private
copies of simple_{unlink,rmdir}().  So I'd rather have that in patch adding
the warning - simple_recursive_remove() wouldn't need a separate copy that
way at all.

configfs has a separate series untangling it, but that's a separate story -
that work goes back to 2018; I want to resurrect it, but I'm not mixing it
into this pile.

appramor is... special.  They've got locking of their own, completely broken and
interspersed with regular directory locks.  John Johansen, if I understood him
correctly, has some plans re fixing that, and I'm happy not to have analysis
of their locking on my plate.  _Maybe_ it will end up close enough to the usual
tree-in-dcache to switch to that stuff, but at the moment I'd rather open-code
simple_{unlink,rmdir} in aafs_remove() and leave it at that.