Re: [PATCH v3 20/21] __dentry_kill(): new locking scheme

[Max Kellermann]

On Mon, Jul 7, 2025 at 9:31 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> > But why did you add code that keeps looping if a dead/killed dentry
> > was found, even though there is no code to do anything with such a
> > dentry?
>
> Huh?  That dentry contributes a soon-to-be-gone reference to parent;
> it's still there in the tree, but it's already in process of being
> evicted.  The parent will remain busy the end of __dentry_kill().
>
> It is *not* dead; if you want slightly distrubing metaphors, it is already
> beyond resuscitation (that's what the negative refcount indicates), but
> it has not finished dying yet.  DCACHE_DENTRY_KILLED in flags ==
> "it's dead", and those can't be found in the tree/hash/list of aliases/etc.
> Negative refcount on something found in the tree == "it's busy dying at
> the moment" and parent is kept busy until that's over.
>
> And we *want* those to be findable in the tree - think e.g. of umount.
> We really don't want to progress to destroying fs-private data structures
> before all dentries are disconnected from inodes, etc.

Sorry Al, I don't get it.
I understand that objects that are still referenced must not be freed,
and of course a dentry that has started the process of dying by
__dentry_kill() needs to remain in the tree, and that its parent must
not be freed either. Of course!

But none of this explains why you added this "d_lockref.count<0"
check, which I doubt is correct because it causes a busy loop, burning
CPU cycles without doing anything.

Maybe it's just me, maybe I'm missing something - then sorry for
bothering you with this. But I believe that there must be another way
to implement this, without burning CPU cycles. A busy wait is (almost)
never a good idea. Please help me understand. (Or maybe maybe I do
have a point and this should be optimized. I'm confused.)

Max