From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Andrew Morton <akpm@linux-foundation.org>,
stable@vger.kernel.org, Christian Brauner <brauner@kernel.org>,
Al Viro <viro@ZenIV.linux.org.uk>,
Ajay Kaher <ajay.kaher@broadcom.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: [for-linus][PATCH 04/13] tracefs: Avoid using the ei->dentry pointer unnecessarily
Date: Thu, 01 Feb 2024 21:30:03 -0500 [thread overview]
Message-ID: <20240202023022.554271330@goodmis.org> (raw)
In-Reply-To: 20240202022959.515961549@goodmis.org
From: Linus Torvalds <torvalds@linux-foundation.org>
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>
---
fs/tracefs/event_inode.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 824b1811e342..e9819d719d2a 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -156,33 +156,30 @@ static int eventfs_set_attr(struct mnt_idmap *idmap, struct dentry *dentry,
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_find_events(struct dentry *dentry)
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_find_events(struct dentry *dentry)
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;
}
--
2.43.0
next prev parent reply other threads:[~2024-02-02 2:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-02 2:29 [for-linus][PATCH 00/13] tracing/eventfs: Updates for v6.8 Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 01/13] ring-buffer: Clean ring_buffer_poll_wait() error return Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 02/13] tracefs: Zero out the tracefs_inode when allocating it Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 03/13] eventfs: Initialize the tracefs inode properly Steven Rostedt
2024-02-02 2:30 ` Steven Rostedt [this message]
2024-02-02 2:30 ` [for-linus][PATCH 05/13] tracefs: dentry lookup crapectomy Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 06/13] eventfs: Remove unused d_parent pointer field Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 07/13] eventfs: Clean up dentry ops and add revalidate function Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 08/13] eventfs: Get rid of dentry pointers without refcounts Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 09/13] tracing/timerlat: Move hrtimer_init to timerlat_fd open() Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 10/13] eventfs: Warn if an eventfs_inode is freed without is_freed being set Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 11/13] eventfs: Restructure eventfs_inode structure to be more condensed Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 12/13] eventfs: Remove fsnotify*() functions from lookup() Steven Rostedt
2024-02-02 2:30 ` [for-linus][PATCH 13/13] eventfs: Keep all directory links at 1 Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240202023022.554271330@goodmis.org \
--to=rostedt@goodmis.org \
--cc=ajay.kaher@broadcom.com \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox