Re: [PATCH v3] tracefs: Add read-only eventfs filesystem at /sys/kernel/events

On Tue, 11 Aug 2026 10:28:18 +0900
Masami Hiramatsu (Google) [off-list ref] wrote:

Hi Steve,

I have some comments on this.

On Mon, 10 Aug 2026 16:07:08 -0400
Steven Rostedt [off-list ref] wrote:

quoted

@@ -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;

Sure.

[...]

quoted

@@ -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.

Bah, I thought I fixed that. I may have but lost the changes in a rebase.

quoted

+ *
+ * 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.)

Nice catch. I'll fix it.

I also found some other issues with the superblock setup. This isn't
going to go into the next merge window as it's too late.

Thanks for looking at it,

-- Steve

quoted

+
+	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,