On Tue, 30 Jan 2024 11:03:54 -0800 Linus Torvalds <torva...@linux-foundation.org> wrote:
> +/* > + * eventfs_inode reference count management. > + * > + * NOTE! We count only references from dentries, in the > + * form 'dentry->d_fsdata'. There are also references from > + * directory inodes ('ti->private'), but the dentry reference > + * count is always a superset of the inode reference count. > + */ > +static void release_ei(struct kref *ref) > +{ > + struct eventfs_inode *ei = container_of(ref, struct eventfs_inode, > kref); > + kfree(ei->entry_attrs); I did modify this to add back the kfree_const(ei->name); I would also like to add a: WARN_ON_ONCE(!ei->is_freed); The kref_put() logic should add the protection that this is guaranteed to be true as the logic in the removal does: ei->is_freed = 1; [..] kref_put(ei); I would think that the last "put" would always have the "is_freed" set. The WARN_ON is to catch things doing too many put_ei(). > + kfree(ei); > +} > + > @@ -951,5 +857,6 @@ void eventfs_remove_events_dir(struct eventfs_inode *ei) > * sticks around while the other ei->dentry are created > * and destroyed dynamically. > */ > + simple_recursive_removal(dentry, NULL); Actually, this doesn't work. It expects all the dentries in the list to have positive ref counts, as it does dput() on them. If there's any cached, it will bug with a dput() on a dentry of zero. The only dentries that should have non zero ref counts are ones that are currently being accessed and wont go away until finished. I think all we need to do is invalidate the top dentry. Is that true? Does this work: d_invalidate(dentry); to make the directory no longer accessible. And all dentries within it should be zero, and hopefully go away on memory reclaim. The last patch removes it, but if you want to test the deletion, you can do this: # cd /sys/kernel/tracing # mkdir instances/foo # ls instances/foo/events # rmdir instances/foo But also note that this patch fails with the "ghost" files with the kprobe test again. When I apply patch 6, that goes away. I'm guessing that's because this needs the d_revalidate() call. To not break bisection, I think we need to merge this and patch 6 together. Also patch 6 removes the simple_recursive_removal() which without at least the d_invalidate() causes a dput splat. That's because the rmdir calls tracefs_remove() which calls simple_recursive_removal() that will walk to the events directory and I'm assuming if it hasn't been invalidated, it walks into it causing the same issue as a simple_recursive_removal() would have here. Try the above mkdir and rmdir to see. -- Steve > dput(dentry); > }