On Tue, 30 Jan 2024 21:25:30 -0800 Linus Torvalds <torva...@linux-foundation.org> wrote:
> > 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
