Hi Steve, I have some comments on this.
On Mon, 10 Aug 2026 16:07:08 -0400 Steven Rostedt <[email protected]> wrote: > @@ -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; [...] > @@ -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) <[email protected]>
