Re: [PATCH v6 07/24] tracing: Add events/ root files to trace remotes

Steven Rostedt
2025-09-09T21:52:06Z

On Thu, 21 Aug 2025 09:13:55 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:

> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index 822cdd76e334..5065b162c8ef 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c
> @@ -710,10 +710,118 @@ static int remote_event_callback(const char *name, umode_t *mode, void **data,
> return 0;
> }
>
> +static ssize_t remote_events_dir_enable_write(struct file *filp, const char __user *ubuf,
> + size_t count, loff_t *ppos)
> +{
> + struct trace_remote *remote = file_inode(filp)->i_private;
> + u8 enable;
> + int i, ret;
> +
> + ret = kstrtou8_from_user(ubuf, count, 10, &enable);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&remote->lock);
> +
> + for (i = 0; i < remote->nr_events; i++) {
> + struct remote_event *evt = &remote->events[i];
> +
> + trace_remote_enable_event(remote, evt, enable);
> + }
> +
> + return count;
> +}
> +
> +static const struct file_operations remote_events_dir_enable_fops = {
> + .write = remote_events_dir_enable_write,

Hmm, why not a read as well? Or is it too difficult to figure out what is
enabled and what isn't.

For the main trace buffer, it's:

0 - all disabled
1 - all enabled
X - some disabled some enabled

> +};
> +
> +static ssize_t
> +remote_events_dir_header_page_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
> +{
> + struct trace_seq *s;
> + int ret;
> +
> + s = kmalloc(sizeof(*s), GFP_KERNEL);
> + if (!s)
> + return -ENOMEM;
> +
> + trace_seq_init(s);
> +
> + ring_buffer_print_page_header(NULL, s);
> + ret = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, trace_seq_used(s));
> + kfree(s);
> +
> + return ret;
> +}
> +
> +static const struct file_operations remote_events_dir_header_page_fops = {
> + .read = remote_events_dir_header_page_read,
> +};
> +
> +static ssize_t
> +remote_events_dir_header_event_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
> +{
> + struct trace_seq *s;
> + int ret;
> +
> + s = kmalloc(sizeof(*s), GFP_KERNEL);
> + if (!s)
> + return -ENOMEM;
> +
> + trace_seq_init(s);
> +
> + ring_buffer_print_entry_header(s);
> + ret = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, trace_seq_used(s));
> + kfree(s);
> +
> + return ret;
> +}
> +
> +static const struct file_operations remote_events_dir_header_event_fops = {
> + .read = remote_events_dir_header_event_read,
> +};
> +
> +static int remote_events_dir_callback(const char *name, umode_t *mode, void **data,
> + const struct file_operations **fops)
> +{
> + if (!strcmp(name, "enable")) {
> + *mode = 0200;

As 0200 is a bit strange.

-- Steve

> + *fops = &remote_events_dir_enable_fops;
> + return 1;
> + }
> +
> + if (!strcmp(name, "header_page")) {
> + *mode = TRACEFS_MODE_READ;
> + *fops = &remote_events_dir_header_page_fops;
> + return 1;
> + }
> +
> + if (!strcmp(name, "header_event")) {
> + *mode = TRACEFS_MODE_READ;
> + *fops = &remote_events_dir_header_event_fops;
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> static int trace_remote_init_eventfs(const char *remote_name, struct trace_remote *remote,
> struct remote_event *evt)
> {
> struct eventfs_inode *eventfs = remote->eventfs;
> + static struct eventfs_entry dir_entries[] = {
> + {
> + .name = "enable",
> + .callback = remote_events_dir_callback,
> + }, {
> + .name = "header_page",
> + .callback = remote_events_dir_callback,
> + }, {
> + .name = "header_event",
> + .callback = remote_events_dir_callback,
> + }
> + };
> static struct eventfs_entry entries[] = {
> {
> .name = "enable",
> @@ -729,7 +837,8 @@ static int trace_remote_init_eventfs(const char *remote_name, struct trace_remot
> bool eventfs_create = false;
>
> if (!eventfs) {
> - eventfs = eventfs_create_events_dir("events", remote->dentry, NULL, 0, NULL);
> + eventfs = eventfs_create_events_dir("events", remote->dentry, dir_entries,
> + ARRAY_SIZE(dir_entries), remote);
> if (IS_ERR(eventfs))
> return PTR_ERR(eventfs);
>