Re: [PATCH v10 05/30] tracing: Introduce trace remotes

2026-01-29 Thread Vincent Donnefort
[...]

> > +static ssize_t buffer_size_kb_write(struct file *filp, const char __user 
> > *ubuf, size_t cnt,
> > +   loff_t *ppos)
> > +{
> > +   struct trace_remote *remote = filp->private_data;
> > +   unsigned long val;
> > +   int ret;
> > +
> > +   ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /* KiB to Bytes */
> > +   if (!val || check_shl_overflow(val, 10, &val))
> > +   return -EINVAL;
> > +
> > +   guard(mutex)(&remote->lock);
> > +
> > +   remote->trace_buffer_size = val;
> 
> Should this be allowed to change when it is already loaded?

It would only be effective on the next unload/load. 

But now thinking more about it, this is probably not clear to a user and
returning -EBUSY when loaded might be a better solution?

> 
> -- Steve
> 
> > +
> > +   return cnt;
> > +}
> > +
> > +static int buffer_size_kb_show(struct seq_file *s, void *unused)
> > +{
> > +   struct trace_remote *remote = s->private;
> > +
> > +   seq_printf(s, "%lu (%s)\n", remote->trace_buffer_size >> 10,
> > +  trace_remote_loaded(remote) ? "loaded" : "unloaded");
> > +
> > +   return 0;
> > +}
> > +DEFINE_SHOW_STORE_ATTRIBUTE(buffer_size_kb);
> > +
> 



Re: [PATCH v10 05/30] tracing: Introduce trace remotes

2026-01-28 Thread Steven Rostedt
On Mon, 26 Jan 2026 10:43:54 +
Vincent Donnefort  wrote:

> + if (!trace_create_file("tracing_on", TRACEFS_MODE_WRITE, remote_d, 
> remote,
> +&tracing_on_fops) ||
> + !trace_create_file("buffer_size_kb", TRACEFS_MODE_WRITE, remote_d, 
> remote,
> +&buffer_size_kb_fops) ||
> + !trace_create_file("trace_pipe", TRACEFS_MODE_READ, remote_d, 
> remote,
> +&trace_pipe_fops))
> + goto err;
> +

Oh, and don't do this in an if statement. Make them each individually as:

ret = trace_create_file(...);
if (!ret)
goto err;

Linus hates complex if statements and I can see him having a conniption if
he were to see this.

-- Steve



Re: [PATCH v10 05/30] tracing: Introduce trace remotes

2026-01-28 Thread Steven Rostedt
On Mon, 26 Jan 2026 10:43:54 +
Vincent Donnefort  wrote:

> diff --git a/include/linux/trace_remote.h b/include/linux/trace_remote.h
> new file mode 100644
> index ..feb3433c2128
> --- /dev/null
> +++ b/include/linux/trace_remote.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_TRACE_REMOTE_H
> +#define _LINUX_TRACE_REMOTE_H
> +
> +#include 
> +
> +/**
> + * struct trace_remote_callbacks - Callbacks used by Tracefs to control the 
> remote
> + *
> + * @load_trace_buffer:  Called before Tracefs accesses the trace buffer for 
> the first
> + *   time. Must return a &trace_buffer_desc
> + *   (most likely filled with trace_remote_alloc_buffer())
> + * @unload_trace_buffer:
> + *   Called once Tracefs has no use for the trace buffer
> + *   (most likely call trace_remote_free_buffer())
> + * @enable_tracing:  Called on Tracefs tracing_on. It is expected from the
> + *   remote to allow writing.
> + * @swap_reader_page:Called when Tracefs consumes a new page from a
> + *   ring-buffer. It is expected from the remote to isolate a
> + *   new reader-page from the @cpu ring-buffer.
> + */
> +struct trace_remote_callbacks {
> + struct trace_buffer_desc *(*load_trace_buffer)(unsigned long size, void 
> *priv);
> + void(*unload_trace_buffer)(struct trace_buffer_desc *desc, void 
> *priv);
> + int (*enable_tracing)(bool enable, void *priv);
> + int (*swap_reader_page)(unsigned int cpu, void *priv);
> +};
> +
> +/**
> + * trace_remote_register() - Register a Tracefs remote
> + *
> + * A trace remote is an entity, outside of the kernel (most likely firmware 
> or
> + * hypervisor) capable of writing events into a Tracefs compatible 
> ring-buffer.
> + * The kernel would then act as a reader.
> + *
> + * The registered remote will be found under the Tracefs directory
> + * remotes/.
> + *
> + * @name:Name of the remote, used for the Tracefs remotes/ directory.
> + * @cbs: Set of callbacks used to control the remote.
> + * @priv:Private data, passed to each callback from @cbs.
> + * @events:  Array of events. &remote_event.name and &remote_event.id must be
> + *   filled by the caller.
> + * @nr_events:   Number of events in the @events array.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int trace_remote_register(const char *name, struct trace_remote_callbacks 
> *cbs, void *priv);

Nit, but kerneldoc goes before the functions and not before their prototypes.


> +
> +/**
> + * trace_remote_alloc_buffer() - Dynamically allocate a trace buffer
> + *
> + * Helper to dynamically allocate a set of pages (enough to cover 
> @buffer_size)
> + * for each CPU from @cpumask and fill @desc. Most likely called from
> + * &trace_remote_callbacks.load_trace_buffer.
> + *
> + * @desc:Uninitialized trace_buffer_desc
> + * @desc_size:   Size of the trace_buffer_desc. Must be at least 
> equal to
> + *   trace_buffer_desc_size()
> + * @buffer_size: Size in bytes of each per-CPU ring-buffer
> + * @cpumask: CPUs to allocate a ring-buffer for
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t 
> desc_size, size_t buffer_size,
> +   const struct cpumask *cpumask);
> +
> +/**
> + * trace_remote_free_buffer() - Free trace buffer allocated with
> + *   trace_remote_alloc_buffer()
> + *
> + * Most likely called from &trace_remote_callbacks.unload_trace_buffer.
> + *
> + * @desc:Descriptor of the per-CPU ring-buffers, originally filled by
> + *   trace_remote_alloc_buffer()
> + */
> +void trace_remote_free_buffer(struct trace_buffer_desc *desc);
> +
> +#endif
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index bfa2ec46e075..135edf143073 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -1278,4 +1278,7 @@ config HIST_TRIGGERS_DEBUG
>  
>  source "kernel/trace/rv/Kconfig"
>  
> +config TRACE_REMOTE
> + bool
> +
>  endif # FTRACE
> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> index fc5dcc888e13..6e75d710fc25 100644
> --- a/kernel/trace/Makefile
> +++ b/kernel/trace/Makefile
> @@ -127,4 +127,5 @@ obj-$(CONFIG_FPROBE_EVENTS) += trace_fprobe.o
>  obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
>  obj-$(CONFIG_RV) += rv/
>  
> +obj-$(CONFIG_TRACE_REMOTE) += trace_remote.o
>  libftrace-y := ftrace.o
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8bd4ec08fb36..1fcbdea992d2 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -9356,7 +9356,7 @@ static struct dentry *tracing_dentry_percpu(struct 
> trace_array *tr, int cpu)
>   return tr->percpu_dir;
>  }
>  
> -static struct dentry *
> +struct dentry *

[PATCH v10 05/30] tracing: Introduce trace remotes

2026-01-26 Thread Vincent Donnefort
A trace remote relies on ring-buffer remotes to read and control
compatible tracing buffers, written by entity such as firmware or
hypervisor.

Add a Tracefs directory remotes/ that contains all instances of trace
remotes. Each instance follows the same hierarchy as any other to ease
the support by existing user-space tools.

This currently does not provide any event support, which will come
later.

Signed-off-by: Vincent Donnefort 

diff --git a/include/linux/trace_remote.h b/include/linux/trace_remote.h
new file mode 100644
index ..feb3433c2128
--- /dev/null
+++ b/include/linux/trace_remote.h
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_TRACE_REMOTE_H
+#define _LINUX_TRACE_REMOTE_H
+
+#include 
+
+/**
+ * struct trace_remote_callbacks - Callbacks used by Tracefs to control the 
remote
+ *
+ * @load_trace_buffer:  Called before Tracefs accesses the trace buffer for 
the first
+ * time. Must return a &trace_buffer_desc
+ * (most likely filled with trace_remote_alloc_buffer())
+ * @unload_trace_buffer:
+ * Called once Tracefs has no use for the trace buffer
+ * (most likely call trace_remote_free_buffer())
+ * @enable_tracing:Called on Tracefs tracing_on. It is expected from the
+ * remote to allow writing.
+ * @swap_reader_page:  Called when Tracefs consumes a new page from a
+ * ring-buffer. It is expected from the remote to isolate a
+ * new reader-page from the @cpu ring-buffer.
+ */
+struct trace_remote_callbacks {
+   struct trace_buffer_desc *(*load_trace_buffer)(unsigned long size, void 
*priv);
+   void(*unload_trace_buffer)(struct trace_buffer_desc *desc, void 
*priv);
+   int (*enable_tracing)(bool enable, void *priv);
+   int (*swap_reader_page)(unsigned int cpu, void *priv);
+};
+
+/**
+ * trace_remote_register() - Register a Tracefs remote
+ *
+ * A trace remote is an entity, outside of the kernel (most likely firmware or
+ * hypervisor) capable of writing events into a Tracefs compatible ring-buffer.
+ * The kernel would then act as a reader.
+ *
+ * The registered remote will be found under the Tracefs directory
+ * remotes/.
+ *
+ * @name:  Name of the remote, used for the Tracefs remotes/ directory.
+ * @cbs:   Set of callbacks used to control the remote.
+ * @priv:  Private data, passed to each callback from @cbs.
+ * @events:Array of events. &remote_event.name and &remote_event.id must be
+ * filled by the caller.
+ * @nr_events: Number of events in the @events array.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int trace_remote_register(const char *name, struct trace_remote_callbacks 
*cbs, void *priv);
+
+/**
+ * trace_remote_alloc_buffer() - Dynamically allocate a trace buffer
+ *
+ * Helper to dynamically allocate a set of pages (enough to cover @buffer_size)
+ * for each CPU from @cpumask and fill @desc. Most likely called from
+ * &trace_remote_callbacks.load_trace_buffer.
+ *
+ * @desc:  Uninitialized trace_buffer_desc
+ * @desc_size: Size of the trace_buffer_desc. Must be at least equal to
+ * trace_buffer_desc_size()
+ * @buffer_size:   Size in bytes of each per-CPU ring-buffer
+ * @cpumask:   CPUs to allocate a ring-buffer for
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int trace_remote_alloc_buffer(struct trace_buffer_desc *desc, size_t 
desc_size, size_t buffer_size,
+ const struct cpumask *cpumask);
+
+/**
+ * trace_remote_free_buffer() - Free trace buffer allocated with
+ * trace_remote_alloc_buffer()
+ *
+ * Most likely called from &trace_remote_callbacks.unload_trace_buffer.
+ *
+ * @desc:  Descriptor of the per-CPU ring-buffers, originally filled by
+ * trace_remote_alloc_buffer()
+ */
+void trace_remote_free_buffer(struct trace_buffer_desc *desc);
+
+#endif
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index bfa2ec46e075..135edf143073 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -1278,4 +1278,7 @@ config HIST_TRIGGERS_DEBUG
 
 source "kernel/trace/rv/Kconfig"
 
+config TRACE_REMOTE
+   bool
+
 endif # FTRACE
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index fc5dcc888e13..6e75d710fc25 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -127,4 +127,5 @@ obj-$(CONFIG_FPROBE_EVENTS) += trace_fprobe.o
 obj-$(CONFIG_TRACEPOINT_BENCHMARK) += trace_benchmark.o
 obj-$(CONFIG_RV) += rv/
 
+obj-$(CONFIG_TRACE_REMOTE) += trace_remote.o
 libftrace-y := ftrace.o
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8bd4ec08fb36..1fcbdea992d2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9356,7 +9356,7 @@ static struct dentry *tracing_dentry_percpu(struct 
trace_array *tr, int cpu)