From nobody Wed Dec 31 10:50:17 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id ACF9FC4332F for ; Sun, 5 Nov 2023 16:01:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229505AbjKEQB4 (ORCPT ); Sun, 5 Nov 2023 11:01:56 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46906 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229537AbjKEQBn (ORCPT ); Sun, 5 Nov 2023 11:01:43 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DFF85FF; Sun, 5 Nov 2023 08:01:38 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8F2DBC43391; Sun, 5 Nov 2023 16:01:38 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97-RC3) (envelope-from ) id 1qzfZE-00000000Cig-0WIE; Sun, 05 Nov 2023 11:01:40 -0500 Message-ID: <20231105160139.983291500@goodmis.org> User-Agent: quilt/0.67 Date: Sun, 05 Nov 2023 10:56:35 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, Cc: Masami Hiramatsu , Mark Rutland , Andrew Morton , Al Viro Subject: [v6.6][PATCH 5/5] eventfs: Use simple_recursive_removal() to clean up dentries References: <20231105155630.925114107@goodmis.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: "Steven Rostedt (Google)" 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@goodmis.org Cc: stable@vger.kernel.org Cc: Masami Hiramatsu Cc: Mark Rutland Cc: Andrew Morton Cc: Al Viro Fixes: 5bdcd5f5331a2 ("eventfs: Implement removal of meta data from eventfs= ") Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 71 +++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 38 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 7aa92b8ebc51..5fcfb634fec2 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -54,12 +54,10 @@ struct eventfs_file { /* * Union - used for deletion * @llist: for calling dput() if needed after RCU - * @del_list: list of eventfs_file to delete * @rcu: eventfs_file to delete in RCU */ union { struct llist_node llist; - struct list_head del_list; struct rcu_head rcu; }; void *data; @@ -276,7 +274,6 @@ static void free_ef(struct eventfs_file *ef) */ void eventfs_set_ef_status_free(struct tracefs_inode *ti, struct dentry *d= entry) { - struct tracefs_inode *ti_parent; struct eventfs_inode *ei; struct eventfs_file *ef; =20 @@ -297,10 +294,6 @@ void eventfs_set_ef_status_free(struct tracefs_inode *= ti, struct dentry *dentry) =20 mutex_lock(&eventfs_mutex); =20 - ti_parent =3D get_tracefs(dentry->d_parent->d_inode); - if (!ti_parent || !(ti_parent->flags & TRACEFS_EVENT_INODE)) - goto out; - ef =3D dentry->d_fsdata; if (!ef) goto out; @@ -873,30 +866,29 @@ static void unhook_dentry(struct dentry *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); =20 - dentry->d_fsdata =3D 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); } =20 /** * eventfs_remove_rec - remove eventfs dir or file from list * @ef: eventfs_file to be removed. - * @head: to create list of eventfs_file to be deleted * @level: to check recursion depth * * The helper function eventfs_remove_rec() is used to clean up and free t= he * associated data from eventfs for both of the added functions. */ -static void eventfs_remove_rec(struct eventfs_file *ef, struct list_head *= head, int level) +static void eventfs_remove_rec(struct eventfs_file *ef, int level) { struct eventfs_file *ef_child; =20 @@ -916,14 +908,16 @@ static void eventfs_remove_rec(struct eventfs_file *e= f, struct list_head *head, /* search for nested folders or files */ list_for_each_entry_srcu(ef_child, &ef->ei->e_top_files, list, lockdep_is_held(&eventfs_mutex)) { - eventfs_remove_rec(ef_child, head, level + 1); + eventfs_remove_rec(ef_child, level + 1); } } =20 ef->is_freed =3D 1; =20 + unhook_dentry(ef->dentry); + list_del_rcu(&ef->list); - list_add_tail(&ef->del_list, head); + call_srcu(&eventfs_srcu, &ef->rcu, free_rcu_ef); } =20 /** @@ -934,28 +928,22 @@ static void eventfs_remove_rec(struct eventfs_file *e= f, struct list_head *head, */ void eventfs_remove(struct eventfs_file *ef) { - struct eventfs_file *tmp; - LIST_HEAD(ef_del_list); + struct dentry *dentry; =20 if (!ef) return; =20 - /* - * 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(ef, &ef_del_list, 0); + dentry =3D ef->dentry; + eventfs_remove_rec(ef, 0); mutex_unlock(&eventfs_mutex); =20 - list_for_each_entry_safe(ef, tmp, &ef_del_list, del_list) { - unhook_dentry(ef->dentry); - list_del(&ef->del_list); - call_srcu(&eventfs_srcu, &ef->rcu, free_rcu_ef); - } + /* + * If any of the ei children has a dentry, then the ei itself + * must have a dentry. + */ + if (dentry) + simple_recursive_removal(dentry, NULL); } =20 /** @@ -966,6 +954,8 @@ void eventfs_remove(struct eventfs_file *ef) */ void eventfs_remove_events_dir(struct dentry *dentry) { + struct eventfs_file *ef_child; + struct eventfs_inode *ei; struct tracefs_inode *ti; =20 if (!dentry || !dentry->d_inode) @@ -975,6 +965,11 @@ void eventfs_remove_events_dir(struct dentry *dentry) if (!ti || !(ti->flags & TRACEFS_EVENT_INODE)) return; =20 - d_invalidate(dentry); - dput(dentry); + mutex_lock(&eventfs_mutex); + ei =3D ti->private; + list_for_each_entry_srcu(ef_child, &ei->e_top_files, list, + lockdep_is_held(&eventfs_mutex)) { + eventfs_remove_rec(ef_child, 0); + } + mutex_unlock(&eventfs_mutex); } --=20 2.42.0