Re: [PATCH v3] tracefs: Add read-only eventfs filesystem at /sys/kernel/events
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2026-08-11 01:28:23
Also in:
lkml
Hi Steve, I have some comments on this. On Mon, 10 Aug 2026 16:07:08 -0400 Steven Rostedt [off-list ref] wrote:
quoted hunk ↗ jump to hunk
@@ -551,20 +643,40 @@ static struct dentry *eventfs_root_lookup(struct inode *dir, if (strcmp(name, entry->name) != 0) continue; + if (ro && !entry->read_only) + return NULL; + data = ei->data; if (entry->callback(name, &mode, &data, &fops) <= 0) return NULL; + if (ro) + mode |= 0444;
Don't we need to clear writable bits? e.g. mode = (mode & ~0222) | 0444; [...]
quoted hunk ↗ jump to hunk
@@ -812,6 +937,52 @@ struct eventfs_inode *eventfs_create_events_dir(const char *name, struct dentry return ERR_PTR(-ENOMEM); } +/** + * eventfs_create_events_dir_ro - create a read-only events directory + * @name: The name of the top level directory to create. + * @entries: A list of entries that represent the files under this directory + * @size: The number of @entries + * @data: The default data to pass to the files (an entry may override it).
This document need to be updated too. It should be "eventfs_create_events_ro_copy" and only takes @name and @ei.
+ *
+ * This function configures the eventfs filesystem root as a read-only
+ * trace event directory using the existing eventfs_inode lazy-lookup
+ * infrastructure.
+ *
+ * See eventfs_create_dir() for use of @entries.
+ */
+int eventfs_create_events_ro_copy(const char *name, struct eventfs_inode *ei)
+{
+ static struct dentry *dentry;
+ struct tracefs_inode *ti;
+ struct inode *inode;
+
+ /* Can only be called once. */
+ if (dentry)
+ return -EBUSY;
+
+ /* Reference acquired but never freed */
+ dentry = eventfs_ro_get_root();
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ inode = d_inode(dentry);
+
+ INIT_LIST_HEAD(&ei->children);
+ INIT_LIST_HEAD(&ei->list);Nit: This seems redundant because those lists are initialized in event_create_events_dir() already, and here we initialize again. Currently, there is no chance to add anything on these lists. (But if we add something on these lists, re-initializing will make the items orphaned silently.)
+ + ti = get_tracefs(inode); + ti->flags |= TRACEFS_EVENT_INODE; + ti->private = ei; + + inode->i_op = &eventfs_ro_dir_inode_operations; + inode->i_fop = &eventfs_ro_file_operations; + + /* This is never freed */ + dentry->d_fsdata = get_ei(ei); + + return 0; +}
Thanks, -- Masami Hiramatsu (Google) [off-list ref]