From patchwork Thu Feb 1 15:34:47 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13541364 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2644215D5BA; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; cv=none; b=heVmmP/6C9SBqCb80Pg/Z1w+PiGGEXU1lL4b98+XT17nYK3FsrFAWnfUuz56Wm+A+8qQUpXbWlkGmXzn2ii87MzFjlaQ9+bWqu3U/1qcneBziwSORDpUWJuqUnf7Vjw8q2eHTvdzro8QQobYGeznIA2BHqiBPPS22KlOWG23UOY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; c=relaxed/simple; bh=NPnmMMbyV1baHC4KsmvnpUoEetauizgbbY4RJQJGnHo=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=BQtIFSYrl41JnnQOktQ5i+WhHxsxYXj9f3WNqg0tje7Ki6kBi+KeyS3bsN1inE4W0hBkBU8n2JNkmqQnSZuOu5SU0tBXPGMIdVOxum4v14vEZtMjGoU0nSDMPfnyaJZ44tBaSAxDeNlUn6wgSC6m/0aJkXZUSAOwQPkwm1mVFPE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id F3886C433A6; Thu, 1 Feb 2024 16:15:59 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1rVZjc-00000005Tik-47ok; Thu, 01 Feb 2024 11:16:16 -0500 Message-ID: <20240201161616.843551963@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 01 Feb 2024 10:34:47 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Christian Brauner , Al Viro , Ajay Kaher Subject: [PATCH 1/6] eventfs: Warn if an eventfs_inode is freed without is_freed being set References: <20240201153446.138990674@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" There should never be a case where an evenfs_inode is being freed without is_freed being set. Add a WARN_ON_ONCE() if it ever happens. That would mean there was one too many put_ei()s. Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 515fdace1eea..ca7daee7c811 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -73,6 +73,9 @@ enum { static void release_ei(struct kref *ref) { struct eventfs_inode *ei = container_of(ref, struct eventfs_inode, kref); + + WARN_ON_ONCE(!ei->is_freed); + kfree(ei->entry_attrs); kfree_const(ei->name); kfree_rcu(ei, rcu); @@ -84,6 +87,14 @@ static inline void put_ei(struct eventfs_inode *ei) kref_put(&ei->kref, release_ei); } +static inline void free_ei(struct eventfs_inode *ei) +{ + if (ei) { + ei->is_freed = 1; + put_ei(ei); + } +} + static inline struct eventfs_inode *get_ei(struct eventfs_inode *ei) { if (ei) @@ -679,7 +690,7 @@ struct eventfs_inode *eventfs_create_dir(const char *name, struct eventfs_inode /* Was the parent freed? */ if (list_empty(&ei->list)) { - put_ei(ei); + free_ei(ei); ei = NULL; } return ei; @@ -770,7 +781,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry return ei; fail: - put_ei(ei); + free_ei(ei); tracefs_failed_creating(dentry); return ERR_PTR(-ENOMEM); } @@ -801,9 +812,8 @@ static void eventfs_remove_rec(struct eventfs_inode *ei, int level) list_for_each_entry(ei_child, &ei->children, list) eventfs_remove_rec(ei_child, level + 1); - ei->is_freed = 1; list_del(&ei->list); - put_ei(ei); + free_ei(ei); } /** From patchwork Thu Feb 1 15:34:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13541366 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 96C7316088E; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; cv=none; b=SSXuekQKbGrDclHqfNoIbgg1x9Y421strppmFWGp+v5gmJ691cRU4aX+r8YrjuDFNoL709Ca9ZgywE5WVcDgEw+9Pz1+TR+eVNMOdXEqadRlw9OtuNvCT+27V4A5LBbo3WJ5f/9PNAbCh8uuz/Rvk3SR18hT8cf/DecSSH3fEdM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; c=relaxed/simple; bh=UgB47jemCzZI8cfp+JEFI8vC2gts63luDIkaSyClb+c=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=DXMu9nSOLUN9n5Wo+M2r5VeoMQZ5v3U5Dz+lnjpffIZ9EWmqAN01zTaGb/b1Qb4POx4Pzbz3lRPCp2v/l2YvXt6qiU1YCVuq1jTVg4tf95P45Tg2DS9+CX+vYN3IybNINWquD1mNIFBbY9mM5fw8RD+ltMiETl0/umGLHb6+dbM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 113EFC43394; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1rVZjd-00000005TjE-0cEI; Thu, 01 Feb 2024 11:16:17 -0500 Message-ID: <20240201161617.002321438@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 01 Feb 2024 10:34:48 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Christian Brauner , Al Viro , Ajay Kaher Subject: [PATCH 2/6] eventfs: Restructure eventfs_inode structure to be more condensed References: <20240201153446.138990674@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" Some of the eventfs_inode structure has holes in it. Rework the structure to be a bit more condensed, and also remove the no longer used llist field. Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/internal.h | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index 1886f1826cd8..beb3dcd0e434 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -32,40 +32,37 @@ struct eventfs_attr { /* * struct eventfs_inode - hold the properties of the eventfs directories. * @list: link list into the parent directory + * @rcu: Union with @list for freeing + * @children: link list into the child eventfs_inode * @entries: the array of entries representing the files in the directory * @name: the name of the directory to create - * @children: link list into the child eventfs_inode * @events_dir: the dentry of the events directory * @entry_attrs: Saved mode and ownership of the @d_children - * @attr: Saved mode and ownership of eventfs_inode itself * @data: The private data to pass to the callbacks + * @attr: Saved mode and ownership of eventfs_inode itself * @is_freed: Flag set if the eventfs is on its way to be freed * Note if is_freed is set, then dentry is corrupted. + * @is_events: Flag set for only the top level "events" directory * @nr_entries: The number of items in @entries + * @ino: The saved inode number */ struct eventfs_inode { - struct kref kref; - struct list_head list; + union { + struct list_head list; + struct rcu_head rcu; + }; + struct list_head children; const struct eventfs_entry *entries; const char *name; - struct list_head children; struct dentry *events_dir; struct eventfs_attr *entry_attrs; - struct eventfs_attr attr; void *data; + struct eventfs_attr attr; + struct kref kref; unsigned int is_freed:1; unsigned int is_events:1; unsigned int nr_entries:30; unsigned int ino; - /* - * Union - used for deletion - * @llist: for calling dput() if needed after RCU - * @rcu: eventfs_inode to delete in RCU - */ - union { - struct llist_node llist; - struct rcu_head rcu; - }; }; static inline struct tracefs_inode *get_tracefs(const struct inode *inode) From patchwork Thu Feb 1 15:34:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13541368 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A6F29160895; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; cv=none; b=S0JyU8S9o6z6tPLawayfje7KS9T8ZDoEekuyrq1mlON/7bmTUdT59N7Xvda+IkbcMFaq0H3UwbbzWRwZm1pQxRRrP6GShoDLEQnUqMYeqdf9k/Uc6qx+ZVBlKsN8rpcRIoQmDftff2xOZhkBE5v1IaQz935c2Ed1VZtyWQ4mm0I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; c=relaxed/simple; bh=lNtIhhFDOlXg3opQxPWZoYHwa0F4HyqlbB3ETO/rgP4=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=aBEJMUw4SW44XD7MVs2b6xj4GJUq4mL+NiGGs6jimyT1mJBEXwTLCNmwj2jm+SbbqX4wuBXbY4mWAiF2jENI2l5ekdKYenxSck7bwCTNQpwGqZcQEYtupiwKF5FcuGWWbt51y4LvXNTqpWkg7Xtz8IGBHzC9J34/kq5M3HvnhHs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4B97EC43390; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1rVZjd-00000005Tji-1JJ6; Thu, 01 Feb 2024 11:16:17 -0500 Message-ID: <20240201161617.166973329@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 01 Feb 2024 10:34:49 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Christian Brauner , Al Viro , Ajay Kaher , stable@vger.kernel.org, Al Viro Subject: [PATCH 3/6] eventfs: Remove fsnotify*() functions from lookup() References: <20240201153446.138990674@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" The dentries and inodes are created when referenced in the lookup code. There's no reason to call fsnotify_*() functions when they are created by a reference. It doesn't make any sense. Link: https://lore.kernel.org/linux-trace-kernel/20240201002719.GS2087318@ZenIV/ Cc: stable@vger.kernel.org Fixes: a376007917776 ("eventfs: Implement functions to create files and dirs when accessed"); Suggested-by: Al Viro Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index ca7daee7c811..9e031e5a2713 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -366,7 +366,6 @@ static struct dentry *lookup_file(struct eventfs_inode *parent_ei, dentry->d_fsdata = get_ei(parent_ei); d_add(dentry, inode); - fsnotify_create(dentry->d_parent->d_inode, dentry); return NULL; }; @@ -408,7 +407,6 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry, inc_nlink(inode); d_add(dentry, inode); inc_nlink(dentry->d_parent->d_inode); - fsnotify_mkdir(dentry->d_parent->d_inode, dentry); return NULL; } From patchwork Thu Feb 1 15:34:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13541367 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A5865160894; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; cv=none; b=GDVjlxYkZtc+HS4w0BAJNhXQa3XTlLft+OApsQLoAfYs2pX4OPGY2DhFWGK696fE8ix0OnLujqS3W3KnAUEgAmQLUPT7k7Ko3wQStPXBFBkqTk0swp2Pto6B9gy+e3PCNdpyg+s9SatOv3DWv6Kqt1dQlKKhU8nfZANOvvxpbxs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; c=relaxed/simple; bh=aYQmR4grRcJrLfjdmABWmdW24Zibr5d0HrWyIDxBXt8=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=L+4y6w0Y2Ydtbi02X7/i10xoUKaeoj3ZxuXjzsvUnS8d1Bndiek7CWxdsZY3TGeLZOaPndXuh3HbGePzk0GwqSpS8DXx/CLWDbQaGjPE5ZVMsjjibROye0fC7h11ZCDn+r2YfUbEpVnm93sSGeos+B874kqFimQlXLwujkstqUU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 56772C43141; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1rVZjd-00000005TkC-20gX; Thu, 01 Feb 2024 11:16:17 -0500 Message-ID: <20240201161617.339968298@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 01 Feb 2024 10:34:50 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Christian Brauner , Al Viro , Ajay Kaher , stable@vger.kernel.org, Al Viro Subject: [PATCH 4/6] eventfs: Keep all directory links at 1 References: <20240201153446.138990674@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" The directory link count in eventfs was somewhat bogus. It was only being updated when a directory child was being looked up and not on creation. One solution would be to update in get_attr() the link count by iterating the ei->children list and then adding 2. But that could slow down simple stat() calls, especially if it's done on all directories in eventfs. Another solution would be to add a parent pointer to the eventfs_inode and keep track of the number of sub directories it has on creation. But this adds overhead for something not really worthwhile. The solution decided upon is to keep all directory links in eventfs as 1. This tells user space not to rely on the hard links of directories. Which in this case it shouldn't. Link: https://lore.kernel.org/linux-trace-kernel/20240201002719.GS2087318@ZenIV/ Cc: stable@vger.kernel.org Fixes: c1504e510238 ("eventfs: Implement eventfs dir creation functions") Suggested-by: Al Viro Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 9e031e5a2713..110e8a272189 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -404,9 +404,7 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry, dentry->d_fsdata = get_ei(ei); - inc_nlink(inode); d_add(dentry, inode); - inc_nlink(dentry->d_parent->d_inode); return NULL; } @@ -769,9 +767,17 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry dentry->d_fsdata = get_ei(ei); - /* directory inodes start off with i_nlink == 2 (for "." entry) */ - inc_nlink(inode); + /* + * Keep all eventfs directories with i_nlink == 1. + * Due to the dynamic nature of the dentry creations and not + * wanting to add a pointer to the parent eventfs_inode in the + * eventfs_inode structure, keeping the i_nlink in sync with the + * number of directories would cause too much complexity for + * something not worth much. Keeping directory links at 1 + * tells userspace not to trust the link number. + */ d_instantiate(dentry, inode); + /* The dentry of the "events" parent does keep track though */ inc_nlink(dentry->d_parent->d_inode); fsnotify_mkdir(dentry->d_parent->d_inode, dentry); tracefs_end_creating(dentry); From patchwork Thu Feb 1 15:34:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13541369 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D885D161B59; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; cv=none; b=Mno0Fl39F0v3qA8PQ9EBSz1pJw9vulVwqRLlYPBEfEmGeVOBIzX42mIraN4Vh9MrLmcnFLo3WJetwMiKRTsLVTgO6rXJsLm/mPB+bXW5YQns5Z/b4GDp4bAIwkaJ4mHxKP2xqswZnQfDuR/sRiS+FM6NAEp7c2at0tlv7c96yCA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; c=relaxed/simple; bh=3+ZSFsB8n+3f4GqZyRsq0vWj2Y9oAbKeKdikbNmaVQ4=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=FHYgZ6NObH+udnYmPxfXvRnGMvRHvdJ/LZOlNWuXCQkzCJ3e2MMVtAqGGC7cuC7XLAmfJOdlbAYBJ22m8b4w3oiOEqrkS0aI8q72omPmI8Hqq4UdwGtRFuhoS4W15anb80Cv2DAfHfvm5wowTZTkSgm9sw355qkdneWopfzyobE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 97DE6C43330; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1rVZjd-00000005Tkg-2gF8; Thu, 01 Feb 2024 11:16:17 -0500 Message-ID: <20240201161617.499712009@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 01 Feb 2024 10:34:51 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Christian Brauner , Al Viro , Ajay Kaher , Al Viro Subject: [PATCH 5/6] eventfs: Add WARN_ON_ONCE() to checks in eventfs_root_lookup() References: <20240201153446.138990674@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" There's a couple of if statements in eventfs_root_lookup() that should never be true. Instead of removing them, add WARN_ON_ONCE() around them. One is a tracefs_inode not being for eventfs. The other is a child being freed but still on the parent's children list. When a child is freed, it is removed from the list under the same mutex that is held during the iteration. Link: https://lore.kernel.org/linux-trace-kernel/20240201002719.GS2087318@ZenIV/ Reported-by: Al Viro Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 110e8a272189..1a831ba1042b 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -483,7 +483,7 @@ static struct dentry *eventfs_root_lookup(struct inode *dir, struct dentry *result = NULL; ti = get_tracefs(dir); - if (!(ti->flags & TRACEFS_EVENT_INODE)) + if (WARN_ON_ONCE!(ti->flags & TRACEFS_EVENT_INODE))) return ERR_PTR(-EIO); mutex_lock(&eventfs_mutex); @@ -495,7 +495,8 @@ static struct dentry *eventfs_root_lookup(struct inode *dir, list_for_each_entry(ei_child, &ei->children, list) { if (strcmp(ei_child->name, name) != 0) continue; - if (ei_child->is_freed) + /* A child is freed and removed from the list at the same time */ + if (WARN_ON_ONCE(ei_child->is_freed)) goto out; result = lookup_dir_entry(dentry, ei, ei_child); goto out; From patchwork Thu Feb 1 15:34:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steven Rostedt X-Patchwork-Id: 13541370 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DA164161B5A; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; cv=none; b=VB8ysJzuZqpiRNu5wDwSEVM69UmeC57KQZuwsfTvqcGO8jeNiaCdVzUXPwsP4RC8Ruy2c5HMC/jRCxQiX2Ty6EtKR21qjyAY5yYfGkYa4ZL4zoGL9/eXtw6wlPy1nN7hj4BPzorSr4g4qOEAF8Mwbf7ulnf4ykNWpwFB2iyLn2c= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706804160; c=relaxed/simple; bh=TwXMBgsYj3cjkDRxSyydgTaD5Knftq7Y2tB5Ttzmt/o=; h=Message-ID:Date:From:To:Cc:Subject:References:MIME-Version: Content-Type; b=aMcvm0WCz0C/QiDVpqjkRugXULfZ6vYDjO8AM+gB36+R+xEOlOeCxMCCMdgaHER5K5Q/gDcoY+/r7cngwLRZHzB1OQ0TBw5oYAqDWpg1wMg4UflApcPXxjDWyYUiT3E6KOB3RwevXJDiVZt4EhYFg/rT/43KHN4vbEIBcXs2+gU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id A0DDFC4166C; Thu, 1 Feb 2024 16:16:00 +0000 (UTC) Received: from rostedt by gandalf with local (Exim 4.97) (envelope-from ) id 1rVZjd-00000005TlA-3MFD; Thu, 01 Feb 2024 11:16:17 -0500 Message-ID: <20240201161617.658992558@goodmis.org> User-Agent: quilt/0.67 Date: Thu, 01 Feb 2024 10:34:52 -0500 From: Steven Rostedt To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Cc: Linus Torvalds , Masami Hiramatsu , Mark Rutland , Mathieu Desnoyers , Christian Brauner , Al Viro , Ajay Kaher Subject: [PATCH 6/6] eventfs: Create eventfs_root_inode to store dentry References: <20240201153446.138990674@goodmis.org> Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "Steven Rostedt (Google)" Only the root "events" directory stores a dentry. There's no reason to hold a dentry pointer for every eventfs_inode as it is never set except for the root "events" eventfs_inode. Create a eventfs_root_inode structure that holds the events_dir dentry. The "events" eventfs_inode *is* special, let it have its own descriptor. Signed-off-by: Steven Rostedt (Google) --- fs/tracefs/event_inode.c | 65 +++++++++++++++++++++++++++++++++------- fs/tracefs/internal.h | 2 -- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 1a831ba1042b..c50d089c9a7f 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -35,6 +35,17 @@ static DEFINE_MUTEX(eventfs_mutex); /* Choose something "unique" ;-) */ #define EVENTFS_FILE_INODE_INO 0x12c4e37 +struct eventfs_root_inode { + struct eventfs_inode ei; + struct dentry *events_dir; +}; + +static struct eventfs_root_inode *get_root_inode(struct eventfs_inode *ei) +{ + WARN_ON_ONCE(!ei->is_events); + return container_of(ei, struct eventfs_root_inode, ei); +} + /* Just try to make something consistent and unique */ static int eventfs_dir_ino(struct eventfs_inode *ei) { @@ -73,12 +84,18 @@ enum { static void release_ei(struct kref *ref) { struct eventfs_inode *ei = container_of(ref, struct eventfs_inode, kref); + struct eventfs_root_inode *rei; WARN_ON_ONCE(!ei->is_freed); kfree(ei->entry_attrs); kfree_const(ei->name); - kfree_rcu(ei, rcu); + if (ei->is_events) { + rei = get_root_inode(ei); + kfree_rcu(rei, ei.rcu); + } else { + kfree_rcu(ei, rcu); + } } static inline void put_ei(struct eventfs_inode *ei) @@ -408,19 +425,43 @@ static struct dentry *lookup_dir_entry(struct dentry *dentry, return NULL; } +static inline struct eventfs_inode *init_ei(struct eventfs_inode *ei, const char *name) +{ + ei->name = kstrdup_const(name, GFP_KERNEL); + if (!ei->name) + return NULL; + kref_init(&ei->kref); + return ei; +} + static inline struct eventfs_inode *alloc_ei(const char *name) { struct eventfs_inode *ei = kzalloc(sizeof(*ei), GFP_KERNEL); + struct eventfs_inode *result; if (!ei) return NULL; - ei->name = kstrdup_const(name, GFP_KERNEL); - if (!ei->name) { + result = init_ei(ei, name); + if (!result) kfree(ei); + + return result; +} + +static inline struct eventfs_inode *alloc_root_ei(const char *name) +{ + struct eventfs_root_inode *rei = kzalloc(sizeof(*rei), GFP_KERNEL); + struct eventfs_inode *ei; + + if (!rei) return NULL; - } - kref_init(&ei->kref); + + rei->ei.is_events = 1; + ei = init_ei(&rei->ei, name); + if (!ei) + kfree(rei); + return ei; } @@ -710,6 +751,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry int size, void *data) { struct dentry *dentry = tracefs_start_creating(name, parent); + struct eventfs_root_inode *rei; struct eventfs_inode *ei; struct tracefs_inode *ti; struct inode *inode; @@ -722,7 +764,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry if (IS_ERR(dentry)) return ERR_CAST(dentry); - ei = alloc_ei(name); + ei = alloc_root_ei(name); if (!ei) goto fail; @@ -731,10 +773,11 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry goto fail; // Note: we have a ref to the dentry from tracefs_start_creating() - ei->events_dir = dentry; + rei = get_root_inode(ei); + rei->events_dir = dentry; + ei->entries = entries; ei->nr_entries = size; - ei->is_events = 1; ei->data = data; /* Save the ownership of this directory */ @@ -845,13 +888,15 @@ void eventfs_remove_dir(struct eventfs_inode *ei) */ void eventfs_remove_events_dir(struct eventfs_inode *ei) { + struct eventfs_root_inode *rei; struct dentry *dentry; - dentry = ei->events_dir; + rei = get_root_inode(ei); + dentry = rei->events_dir; if (!dentry) return; - ei->events_dir = NULL; + rei->events_dir = NULL; eventfs_remove_dir(ei); /* diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h index beb3dcd0e434..15c26f9aaad4 100644 --- a/fs/tracefs/internal.h +++ b/fs/tracefs/internal.h @@ -36,7 +36,6 @@ struct eventfs_attr { * @children: link list into the child eventfs_inode * @entries: the array of entries representing the files in the directory * @name: the name of the directory to create - * @events_dir: the dentry of the events directory * @entry_attrs: Saved mode and ownership of the @d_children * @data: The private data to pass to the callbacks * @attr: Saved mode and ownership of eventfs_inode itself @@ -54,7 +53,6 @@ struct eventfs_inode { struct list_head children; const struct eventfs_entry *entries; const char *name; - struct dentry *events_dir; struct eventfs_attr *entry_attrs; void *data; struct eventfs_attr attr;