| From SRS0=eEWY=JP=rostedt.homelinux.com=rostedt@kernel.org Tue Feb 6 12:35:32 2024 |
| From: Steven Rostedt <rostedt@goodmis.org> |
| Date: Tue, 06 Feb 2024 06:32:13 -0500 |
| Subject: tracefs: Avoid using the ei->dentry pointer unnecessarily |
| To: linux-kernel@vger.kernel.org, stable@vger.kernel.org |
| Cc: Linus Torvalds <torvalds@linux-foundation.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Sasha Levin <sashal@kernel.org>, Masami Hiramatsu <mhiramat@kernel.org>, Mark Rutland <mark.rutland@arm.com>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Christian Brauner <brauner@kernel.org>, Al Viro <viro@ZenIV.linux.org.uk>, Ajay Kaher <ajay.kaher@broadcom.com> |
| Message-ID: <20240206113400.525587454@rostedt.homelinux.com> |
| |
| From: Linus Torvalds <torvalds@linux-foundation.org> |
| |
| commit 99c001cb617df409dac275a059d6c3f187a2da7a upstream. |
| |
| The eventfs_find_events() code tries to walk up the tree to find the |
| event directory that a dentry belongs to, in order to then find the |
| eventfs inode that is associated with that event directory. |
| |
| However, it uses an odd combination of walking the dentry parent, |
| looking up the eventfs inode associated with that, and then looking up |
| the dentry from there. Repeat. |
| |
| But the code shouldn't have back-pointers to dentries in the first |
| place, and it should just walk the dentry parenthood chain directly. |
| |
| Similarly, 'set_top_events_ownership()' looks up the dentry from the |
| eventfs inode, but the only reason it wants a dentry is to look up the |
| superblock in order to look up the root dentry. |
| |
| But it already has the real filesystem inode, which has that same |
| superblock pointer. So just pass in the superblock pointer using the |
| information that's already there, instead of looking up extraneous data |
| that is irrelevant. |
| |
| Link: https://lore.kernel.org/linux-trace-kernel/202401291043.e62e89dc-oliver.sang@intel.com/ |
| Link: https://lore.kernel.org/linux-trace-kernel/20240131185512.638645365@goodmis.org |
| |
| Cc: stable@vger.kernel.org |
| Cc: Masami Hiramatsu <mhiramat@kernel.org> |
| Cc: Mark Rutland <mark.rutland@arm.com> |
| Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> |
| Cc: Christian Brauner <brauner@kernel.org> |
| Cc: Al Viro <viro@ZenIV.linux.org.uk> |
| Cc: Ajay Kaher <ajay.kaher@broadcom.com> |
| Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| Fixes: c1504e510238 ("eventfs: Implement eventfs dir creation functions") |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| fs/tracefs/event_inode.c | 26 ++++++++++++-------------- |
| 1 file changed, 12 insertions(+), 14 deletions(-) |
| |
| --- a/fs/tracefs/event_inode.c |
| +++ b/fs/tracefs/event_inode.c |
| @@ -156,33 +156,30 @@ static int eventfs_set_attr(struct mnt_i |
| return ret; |
| } |
| |
| -static void update_top_events_attr(struct eventfs_inode *ei, struct dentry *dentry) |
| +static void update_top_events_attr(struct eventfs_inode *ei, struct super_block *sb) |
| { |
| - struct inode *inode; |
| + struct inode *root; |
| |
| /* Only update if the "events" was on the top level */ |
| if (!ei || !(ei->attr.mode & EVENTFS_TOPLEVEL)) |
| return; |
| |
| /* Get the tracefs root inode. */ |
| - inode = d_inode(dentry->d_sb->s_root); |
| - ei->attr.uid = inode->i_uid; |
| - ei->attr.gid = inode->i_gid; |
| + root = d_inode(sb->s_root); |
| + ei->attr.uid = root->i_uid; |
| + ei->attr.gid = root->i_gid; |
| } |
| |
| static void set_top_events_ownership(struct inode *inode) |
| { |
| struct tracefs_inode *ti = get_tracefs(inode); |
| struct eventfs_inode *ei = ti->private; |
| - struct dentry *dentry; |
| |
| /* The top events directory doesn't get automatically updated */ |
| if (!ei || !ei->is_events || !(ei->attr.mode & EVENTFS_TOPLEVEL)) |
| return; |
| |
| - dentry = ei->dentry; |
| - |
| - update_top_events_attr(ei, dentry); |
| + update_top_events_attr(ei, inode->i_sb); |
| |
| if (!(ei->attr.mode & EVENTFS_SAVE_UID)) |
| inode->i_uid = ei->attr.uid; |
| @@ -235,8 +232,10 @@ static struct eventfs_inode *eventfs_fin |
| |
| mutex_lock(&eventfs_mutex); |
| do { |
| - /* The parent always has an ei, except for events itself */ |
| - ei = dentry->d_parent->d_fsdata; |
| + // The parent is stable because we do not do renames |
| + dentry = dentry->d_parent; |
| + // ... and directories always have d_fsdata |
| + ei = dentry->d_fsdata; |
| |
| /* |
| * If the ei is being freed, the ownership of the children |
| @@ -246,12 +245,11 @@ static struct eventfs_inode *eventfs_fin |
| ei = NULL; |
| break; |
| } |
| - |
| - dentry = ei->dentry; |
| + // Walk upwards until you find the events inode |
| } while (!ei->is_events); |
| mutex_unlock(&eventfs_mutex); |
| |
| - update_top_events_attr(ei, dentry); |
| + update_top_events_attr(ei, dentry->d_sb); |
| |
| return ei; |
| } |