This is a note to let you know that I've just added the patch titled eventfs: Use simple_recursive_removal() to clean up dentries to the 6.6-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: eventfs-use-simple_recursive_removal-to-clean-up-dentries.patch and it can be found in the queue-6.6 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From SRS0=eEWY=JP=rostedt.homelinux.com=rostedt@xxxxxxxxxx Tue Feb 6 13:11:19 2024 From: Steven Rostedt <rostedt@xxxxxxxxxxx> Date: Tue, 06 Feb 2024 07:09:28 -0500 Subject: eventfs: Use simple_recursive_removal() to clean up dentries To: linux-kernel@xxxxxxxxxxxxxxx, stable@xxxxxxxxxxxxxxx Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>, Sasha Levin <sashal@xxxxxxxxxx>, Masami Hiramatsu <mhiramat@xxxxxxxxxx>, Mark Rutland <mark.rutland@xxxxxxx>, Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Al Viro <viro@xxxxxxxxxxxxxxxxxx> Message-ID: <20240206120950.121281039@xxxxxxxxxxxxxxxxxxxxx> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx> commit 407c6726ca71b33330d2d6345d9ea7ebc02575e9 upstream. Looking at how dentry is removed via the tracefs system, I found that eventfs does not do everything that it did under tracefs. The tracefs removal of a dentry calls simple_recursive_removal() that does a lot more than a simple d_invalidate(). As it should be a requirement that any eventfs_inode that has a dentry, so does its parent. When removing a eventfs_inode, if it has a dentry, a call to simple_recursive_removal() on that dentry should clean up all the dentries underneath it. Add WARN_ON_ONCE() to check for the parent having a dentry if any children do. Link: https://lore.kernel.org/all/20231101022553.GE1957730@ZenIV/ Link: https://lkml.kernel.org/r/20231101172650.552471568@xxxxxxxxxxx Cc: stable@xxxxxxxxxxxxxxx Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx> Cc: Mark Rutland <mark.rutland@xxxxxxx> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Fixes: 5bdcd5f5331a2 ("eventfs: Implement removal of meta data from eventfs") Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- fs/tracefs/event_inode.c | 77 ++++++++++++++++++++++++++--------------------- fs/tracefs/internal.h | 2 - 2 files changed, 44 insertions(+), 35 deletions(-) --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -967,30 +967,29 @@ static void unhook_dentry(struct dentry { if (!dentry) return; - - /* Keep the dentry from being freed yet (see eventfs_workfn()) */ + /* + * Need to add a reference to the dentry that is expected by + * simple_recursive_removal(), which will include a dput(). + */ dget(dentry); - dentry->d_fsdata = NULL; - d_invalidate(dentry); - mutex_lock(&eventfs_mutex); - /* dentry should now have at least a single reference */ - WARN_ONCE((int)d_count(dentry) < 1, - "dentry %px (%s) less than one reference (%d) after invalidate\n", - dentry, dentry->d_name.name, d_count(dentry)); - mutex_unlock(&eventfs_mutex); + /* + * Also add a reference for the dput() in eventfs_workfn(). + * That is required as that dput() will free the ei after + * the SRCU grace period is over. + */ + dget(dentry); } /** * eventfs_remove_rec - remove eventfs dir or file from list * @ei: eventfs_inode to be removed. - * @head: the list head to place the deleted @ei and children * @level: prevent recursion from going more than 3 levels deep. * * This function recursively removes eventfs_inodes which * contains info of files and/or directories. */ -static void eventfs_remove_rec(struct eventfs_inode *ei, struct list_head *head, int level) +static void eventfs_remove_rec(struct eventfs_inode *ei, int level) { struct eventfs_inode *ei_child; @@ -1009,13 +1008,26 @@ static void eventfs_remove_rec(struct ev /* search for nested folders or files */ list_for_each_entry_srcu(ei_child, &ei->children, list, lockdep_is_held(&eventfs_mutex)) { - eventfs_remove_rec(ei_child, head, level + 1); + /* Children only have dentry if parent does */ + WARN_ON_ONCE(ei_child->dentry && !ei->dentry); + eventfs_remove_rec(ei_child, level + 1); } + ei->is_freed = 1; + for (int i = 0; i < ei->nr_entries; i++) { + if (ei->d_children[i]) { + /* Children only have dentry if parent does */ + WARN_ON_ONCE(!ei->dentry); + unhook_dentry(ei->d_children[i]); + } + } + + unhook_dentry(ei->dentry); + list_del_rcu(&ei->list); - list_add_tail(&ei->del_list, head); + call_srcu(&eventfs_srcu, &ei->rcu, free_rcu_ei); } /** @@ -1026,30 +1038,22 @@ static void eventfs_remove_rec(struct ev */ void eventfs_remove_dir(struct eventfs_inode *ei) { - struct eventfs_inode *tmp; - LIST_HEAD(ei_del_list); + struct dentry *dentry; if (!ei) return; - /* - * Move the deleted eventfs_inodes onto the ei_del_list - * which will also set the is_freed value. Note, this has to be - * done under the eventfs_mutex, but the deletions of - * the dentries must be done outside the eventfs_mutex. - * Hence moving them to this temporary list. - */ mutex_lock(&eventfs_mutex); - eventfs_remove_rec(ei, &ei_del_list, 0); + dentry = ei->dentry; + eventfs_remove_rec(ei, 0); mutex_unlock(&eventfs_mutex); - list_for_each_entry_safe(ei, tmp, &ei_del_list, del_list) { - for (int i = 0; i < ei->nr_entries; i++) - unhook_dentry(ei->d_children[i]); - unhook_dentry(ei->dentry); - list_del(&ei->del_list); - call_srcu(&eventfs_srcu, &ei->rcu, free_rcu_ei); - } + /* + * If any of the ei children has a dentry, then the ei itself + * must have a dentry. + */ + if (dentry) + simple_recursive_removal(dentry, NULL); } /** @@ -1060,10 +1064,17 @@ void eventfs_remove_dir(struct eventfs_i */ void eventfs_remove_events_dir(struct eventfs_inode *ei) { - struct dentry *dentry = ei->dentry; + struct dentry *dentry; + dentry = ei->dentry; eventfs_remove_dir(ei); - /* Matches the dget() from eventfs_create_events_dir() */ + /* + * Matches the dget() done by tracefs_start_creating() + * in eventfs_create_events_dir() when it the dentry was + * created. In other words, it's a normal dentry that + * sticks around while the other ei->dentry are created + * and destroyed dynamically. + */ dput(dentry); } --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -55,12 +55,10 @@ struct eventfs_inode { /* * Union - used for deletion * @llist: for calling dput() if needed after RCU - * @del_list: list of eventfs_inode to delete * @rcu: eventfs_inode to delete in RCU */ union { struct llist_node llist; - struct list_head del_list; struct rcu_head rcu; }; unsigned int is_freed:1; Patches currently in stable-queue which might be from rostedt@xxxxxxxxxx are queue-6.6/eventfs-keep-all-directory-links-at-1.patch queue-6.6/eventfs-make-sure-that-parent-d_inode-is-locked-in-creating-files-dirs.patch queue-6.6/eventfs-save-directory-inodes-in-the-eventfs_inode-structure.patch queue-6.6/revert-eventfs-save-ownership-and-mode.patch queue-6.6/tracefs-zero-out-the-tracefs_inode-when-allocating-it.patch queue-6.6/eventfs-read-ei-entries-before-ei-children-in-eventfs_iterate.patch queue-6.6/eventfs-do-not-invalidate-dentry-in-create_file-dir_dentry.patch queue-6.6/eventfs-fix-file-and-directory-uid-and-gid-ownership.patch queue-6.6/eventfs-remove-lookup-parameter-from-create_dir-file_dentry.patch queue-6.6/eventfs-use-gfp_nofs-for-allocation-when-eventfs_mutex-is-held.patch queue-6.6/eventfs-remove-fsnotify-functions-from-lookup.patch queue-6.6/eventfs-use-err_cast-in-eventfs_create_events_dir.patch queue-6.6/revert-eventfs-use-simple_recursive_removal-to-clean-up-dentries.patch queue-6.6/eventfs-use-simple_recursive_removal-to-clean-up-dentries.patch queue-6.6/eventfs-stop-using-dcache_readdir-for-getdents.patch queue-6.6/eventfs-have-event-files-and-directories-default-to-parent-uid-and-gid.patch queue-6.6/eventfs-use-eventfs_remove_events_dir.patch queue-6.6/eventfs-delete-eventfs_inode-when-the-last-dentry-is-freed.patch queue-6.6/tracefs-avoid-using-the-ei-dentry-pointer-unnecessarily.patch queue-6.6/tracefs-remove-stale-update_gid-code.patch queue-6.6/eventfs-initialize-the-tracefs-inode-properly.patch queue-6.6/eventfs-remove-special-processing-of-dput-of-events-directory.patch queue-6.6/eventfs-save-ownership-and-mode.patch queue-6.6/tracefs-check-for-dentry-d_inode-exists-in-set_gid.patch queue-6.6/eventfs-do-ctx-pos-update-for-all-iterations-in-eventfs_iterate.patch queue-6.6/tracefs-dentry-lookup-crapectomy.patch queue-6.6/eventfs-move-taking-of-inode_lock-into-dcache_dir_open_wrapper.patch queue-6.6/eventfs-have-a-free_ei-that-just-frees-the-eventfs_inode.patch queue-6.6/revert-eventfs-remove-is_freed-union-with-rcu-head.patch queue-6.6/eventfs-have-the-inodes-all-for-files-and-directories-all-be-the-same.patch queue-6.6/eventfs-use-kcalloc-instead-of-kzalloc.patch queue-6.6/eventfs-test-for-ei-is_freed-when-accessing-ei-dentry.patch queue-6.6/eventfs-fix-bitwise-fields-for-is_events.patch queue-6.6/eventfs-fix-warn_on-in-create_file_dentry.patch queue-6.6/eventfs-fix-events-beyond-name_max-blocking-tasks.patch queue-6.6/eventfs-shortcut-eventfs_iterate-by-skipping-entries-already-read.patch queue-6.6/revert-eventfs-do-not-allow-null-parent-to-eventfs_start_creating.patch queue-6.6/eventfs-do-not-allow-null-parent-to-eventfs_start_creating.patch queue-6.6/eventfs-do-not-create-dentries-nor-inodes-in-iterate_shared.patch queue-6.6/eventfs-have-eventfs_iterate-stop-immediately-if-ei-is_freed-is-set.patch queue-6.6/eventfs-fix-typo-in-eventfs_inode-union-comment.patch queue-6.6/eventfs-remove-expectation-that-ei-is_freed-means-ei-dentry-null.patch queue-6.6/eventfs-restructure-eventfs_inode-structure-to-be-more-condensed.patch queue-6.6/eventfs-warn-if-an-eventfs_inode-is-freed-without-is_freed-being-set.patch queue-6.6/eventfs-get-rid-of-dentry-pointers-without-refcounts.patch queue-6.6/eventfs-remove-unused-d_parent-pointer-field.patch queue-6.6/eventfs-hold-eventfs_mutex-when-calling-callback-functions.patch queue-6.6/eventfs-remove-is_freed-union-with-rcu-head.patch queue-6.6/tracefs-eventfs-modify-mismatched-function-name.patch queue-6.6/eventfs-fix-kerneldoc-of-eventfs_remove_rec.patch queue-6.6/tracefs-eventfs-use-root-and-instance-inodes-as-default-ownership.patch queue-6.6/revert-eventfs-check-for-null-ef-in-eventfs_set_attr.patch queue-6.6/eventfs-fix-failure-path-in-eventfs_create_events_dir.patch queue-6.6/eventfs-clean-up-dentry-ops-and-add-revalidate-function.patch