Re: [PATCH 5/6] eventfs: get rid of dentry pointers without refcounts

On Tue, 30 Jan 2024 21:25:30 -0800
Linus Torvalds [off-list ref] wrote:

quoted

Does this work:

        d_invalidate(dentry);  

It does, but it's basically irrelevant with the d_revalidate approach.

Basically, once you have d_revalidate(), the unhashing happens there,
and it's just extra work and pointless to do it elsewhere.

So if you look at the "clean up dentry ops and add revalidate
function" patch, you'll see that it just does

-       simple_recursive_removal(dentry, NULL);

and the thing is just history.

With even the last patch included, without the d_invalidate() I get errors
with simply doing:

 # cd /sys/kernel/tracing
 # mkdir instances/foo
 # ls instances/foo/events
 # rmdir instances/foo

As the rmdir calls tracefs_remove() that calls simple_recursive_removal()
that then walks into the "events" directory. Without that d_invalidate, it
walks beyond just the top directory and then splats on the dentries that
are cached.

So really, that final patch is the one that fixes the whole eventfs
mess for good (knock wood). But you can't do it first, because it
basically depends on all the refcount fixes.

I'm running my full suite with the final patch included, plus some of the
updates I mentioned in replies to other patches, as well as including this
"d_invalidate()" as it doesn't pass without it.

It might be possible to re-organize the patches so that the refcount
changes go first, then the d_revalidate(), and then the rest. But I
suspect they all really end up depending on each other some way,
because the basic issue was that the whole "keep unrefcounted dentry
pointers around" was just wrong.  So it doesn't end up right until
it's _all_ fixed, because every step of the way exposes some problem.

At least that was my experience. Fix one thing, and it exposes the
hack that another thing depended on.

This is actually something that Al is a master at. You sometimes see
him send one big complicated patch where he talks about all the
problems in some area and it's one huge "fix up everything patch" that
looks very scary.

And then a week later he sends a series of 19 patches that all make
sense and all look "obvious" and all make small progress.

And magically they end up matching that big cleanup patch in the end.
And you just *know* that it didn't start out as that beautiful logical
series, because you saw the big messy patch first...

I'll take a look at breaking the patches up further, as I now have a much
better understanding of dentries then I did before this discussion.

-- Steve