public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] tracing: Remove backup instance after read all
@ 2026-01-07 14:45 Masami Hiramatsu (Google)
  2026-01-07 14:45 ` [PATCH 1/2] tracing: Make the backup instance readonly Masami Hiramatsu (Google)
  2026-01-07 14:46 ` [PATCH 2/2] tracing: Add autoremove feature to the backup instance Masami Hiramatsu (Google)
  0 siblings, 2 replies; 8+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-01-07 14:45 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel

Hi,

This series of patches improves backup instances of the persistent
ring buffer. Since backup instances are a kind of snapshot of the
persistent ring buffer, it should be readonly. And if it is readonly
there is no reason to keep it after reading all data via trace_pipe
because the data has been consumed.
Thus, [1/2] makes backup instances readonly (not able to write any
events, cleanup trace, change buffer size). Also, [2/2] removes the
backup instance after consuming all data via trace_pipe.
With this improvements, even if we makes a backup instance (using
the same amount of memory of the persistent ring buffer), it will
be removed after reading the data automatically.

---

Masami Hiramatsu (Google) (2):
      tracing: Make the backup instance readonly
      tracing: Add autoremove feature to the backup instance


 kernel/trace/trace.c        |  155 ++++++++++++++++++++++++++++++++++++-------
 kernel/trace/trace.h        |   12 +++
 kernel/trace/trace_events.c |   14 +++-
 3 files changed, 153 insertions(+), 28 deletions(-)

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] tracing: Make the backup instance readonly
  2026-01-07 14:45 [PATCH 0/2] tracing: Remove backup instance after read all Masami Hiramatsu (Google)
@ 2026-01-07 14:45 ` Masami Hiramatsu (Google)
  2026-01-07 16:41   ` Steven Rostedt
  2026-01-07 14:46 ` [PATCH 2/2] tracing: Add autoremove feature to the backup instance Masami Hiramatsu (Google)
  1 sibling, 1 reply; 8+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-01-07 14:45 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Since there is no reason to reuse the backup instance, make it
readonly. Note that only backup instances are readonly, because
other trace instances will be empty unless it is writable.
Only backup instances have copy entries from the original.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace.c        |   91 ++++++++++++++++++++++++++++++++-----------
 kernel/trace/trace.h        |    6 +++
 kernel/trace/trace_events.c |   14 +++++--
 3 files changed, 84 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 38f7a7a55c23..725930f5980e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4888,6 +4888,9 @@ static int tracing_open(struct inode *inode, struct file *file)
 		int cpu = tracing_get_cpu(inode);
 		struct array_buffer *trace_buf = &tr->array_buffer;
 
+		if (trace_array_is_readonly(tr))
+			return -EPERM;
+
 #ifdef CONFIG_TRACER_MAX_TRACE
 		if (tr->current_trace->print_max)
 			trace_buf = &tr->max_buffer;
@@ -6077,6 +6080,9 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
 ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
 				  unsigned long size, int cpu_id)
 {
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	guard(mutex)(&trace_types_lock);
 
 	if (cpu_id != RING_BUFFER_ALL_CPUS) {
@@ -6298,6 +6304,10 @@ int tracing_set_tracer(struct trace_array *tr, const char *buf)
 
 	guard(mutex)(&trace_types_lock);
 
+	/* Not allowed to set new tracer on readonly instance. */
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	update_last_data(tr);
 
 	if (!tr->ring_buffer_expanded) {
@@ -6413,6 +6423,9 @@ tracing_set_trace_write(struct file *filp, const char __user *ubuf,
 	size_t ret;
 	int err;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	ret = cnt;
 
 	if (cnt > MAX_TRACER_SIZE)
@@ -6478,6 +6491,9 @@ tracing_thresh_write(struct file *filp, const char __user *ubuf,
 	struct trace_array *tr = filp->private_data;
 	int ret;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	guard(mutex)(&trace_types_lock);
 	ret = tracing_nsecs_write(&tracing_thresh, ubuf, cnt, ppos);
 	if (ret < 0)
@@ -7047,6 +7063,9 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 	unsigned long val;
 	int ret;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
 	if (ret)
 		return ret;
@@ -7595,6 +7614,9 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	unsigned long ip;
 	char *buf;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	if (tracing_disabled)
 		return -EINVAL;
 
@@ -7675,6 +7697,9 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
 	ssize_t written = -ENODEV;
 	char *buf;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	if (tracing_disabled)
 		return -EINVAL;
 
@@ -7757,6 +7782,9 @@ int tracing_set_clock(struct trace_array *tr, const char *clockstr)
 {
 	int i;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	for (i = 0; i < ARRAY_SIZE(trace_clocks); i++) {
 		if (strcmp(trace_clocks[i].name, clockstr) == 0)
 			break;
@@ -9353,12 +9381,16 @@ static void
 tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
 {
 	struct dentry *d_percpu = tracing_dentry_percpu(tr, cpu);
+	umode_t writable_mode = TRACE_MODE_WRITE;
 	struct dentry *d_cpu;
 	char cpu_dir[30]; /* 30 characters should be more than enough */
 
 	if (!d_percpu)
 		return;
 
+	if (trace_array_is_readonly(tr))
+		writable_mode = TRACE_MODE_READ;
+
 	snprintf(cpu_dir, 30, "cpu%ld", cpu);
 	d_cpu = tracefs_create_dir(cpu_dir, d_percpu);
 	if (!d_cpu) {
@@ -9371,7 +9403,7 @@ tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
 				tr, cpu, &tracing_pipe_fops);
 
 	/* per cpu trace */
-	trace_create_cpu_file("trace", TRACE_MODE_WRITE, d_cpu,
+	trace_create_cpu_file("trace", writable_mode, d_cpu,
 				tr, cpu, &tracing_fops);
 
 	trace_create_cpu_file("trace_pipe_raw", TRACE_MODE_READ, d_cpu,
@@ -9811,6 +9843,9 @@ rb_simple_write(struct file *filp, const char __user *ubuf,
 	unsigned long val;
 	int ret;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	ret = kstrtoul_from_user(ubuf, cnt, 10, &val);
 	if (ret)
 		return ret;
@@ -10597,47 +10632,54 @@ static __init void create_trace_instances(struct dentry *d_tracer)
 static void
 init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
 {
+	umode_t writable_mode = TRACE_MODE_WRITE;
+	bool readonly = trace_array_is_readonly(tr);
 	int cpu;
 
+	if (readonly)
+		writable_mode = TRACE_MODE_READ;
+
 	trace_create_file("available_tracers", TRACE_MODE_READ, d_tracer,
 			tr, &show_traces_fops);
 
-	trace_create_file("current_tracer", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("current_tracer", writable_mode, d_tracer,
 			tr, &set_tracer_fops);
 
-	trace_create_file("tracing_cpumask", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("tracing_cpumask", writable_mode, d_tracer,
 			  tr, &tracing_cpumask_fops);
 
-	trace_create_file("trace_options", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("trace_options", writable_mode, d_tracer,
 			  tr, &tracing_iter_fops);
 
-	trace_create_file("trace", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("trace", writable_mode, d_tracer,
 			  tr, &tracing_fops);
 
 	trace_create_file("trace_pipe", TRACE_MODE_READ, d_tracer,
 			  tr, &tracing_pipe_fops);
 
-	trace_create_file("buffer_size_kb", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("buffer_size_kb", writable_mode, d_tracer,
 			  tr, &tracing_entries_fops);
 
 	trace_create_file("buffer_total_size_kb", TRACE_MODE_READ, d_tracer,
 			  tr, &tracing_total_entries_fops);
 
-	trace_create_file("free_buffer", 0200, d_tracer,
-			  tr, &tracing_free_buffer_fops);
+	if (!readonly) {
+		trace_create_file("free_buffer", 0200, d_tracer,
+				tr, &tracing_free_buffer_fops);
 
-	trace_create_file("trace_marker", 0220, d_tracer,
-			  tr, &tracing_mark_fops);
+		trace_create_file("trace_marker", 0220, d_tracer,
+				tr, &tracing_mark_fops);
 
-	tr->trace_marker_file = __find_event_file(tr, "ftrace", "print");
+		tr->trace_marker_file = __find_event_file(tr, "ftrace", "print");
 
-	trace_create_file("trace_marker_raw", 0220, d_tracer,
-			  tr, &tracing_mark_raw_fops);
+		trace_create_file("trace_marker_raw", 0220, d_tracer,
+				tr, &tracing_mark_raw_fops);
+	}
 
-	trace_create_file("trace_clock", TRACE_MODE_WRITE, d_tracer, tr,
+	trace_create_file("trace_clock", writable_mode, d_tracer, tr,
 			  &trace_clock_fops);
 
-	trace_create_file("tracing_on", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("tracing_on", writable_mode, d_tracer,
 			  tr, &rb_simple_fops);
 
 	trace_create_file("timestamp_mode", TRACE_MODE_READ, d_tracer, tr,
@@ -10645,22 +10687,23 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
 
 	tr->buffer_percent = 50;
 
-	trace_create_file("buffer_percent", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("buffer_percent", writable_mode, d_tracer,
 			tr, &buffer_percent_fops);
 
-	trace_create_file("buffer_subbuf_size_kb", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("buffer_subbuf_size_kb", writable_mode, d_tracer,
 			  tr, &buffer_subbuf_size_fops);
 
-	trace_create_file("syscall_user_buf_size", TRACE_MODE_WRITE, d_tracer,
+	trace_create_file("syscall_user_buf_size", writable_mode, d_tracer,
 			 tr, &tracing_syscall_buf_fops);
 
 	create_trace_options_dir(tr);
 
 #ifdef CONFIG_TRACER_MAX_TRACE
-	trace_create_maxlat_file(tr, d_tracer);
+	if (!readonly)
+		trace_create_maxlat_file(tr, d_tracer);
 #endif
 
-	if (ftrace_create_function_files(tr, d_tracer))
+	if (!readonly && ftrace_create_function_files(tr, d_tracer))
 		MEM_FAIL(1, "Could not allocate function filter files");
 
 	if (tr->range_addr_start) {
@@ -10673,13 +10716,15 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer)
 #endif
 	}
 
-	trace_create_file("error_log", TRACE_MODE_WRITE, d_tracer,
-			  tr, &tracing_err_log_fops);
+	if (!readonly)
+		trace_create_file("error_log", TRACE_MODE_WRITE, d_tracer,
+				  tr, &tracing_err_log_fops);
 
 	for_each_tracing_cpu(cpu)
 		tracing_init_tracefs_percpu(tr, cpu);
 
-	ftrace_init_tracefs(tr, d_tracer);
+	if (!readonly)
+		ftrace_init_tracefs(tr, d_tracer);
 }
 
 #ifdef CONFIG_TRACEFS_AUTOMOUNT_DEPRECATED
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b6d42fe06115..bc0eeb2d1d07 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -483,6 +483,12 @@ extern bool trace_clock_in_ns(struct trace_array *tr);
 
 extern unsigned long trace_adjust_address(struct trace_array *tr, unsigned long addr);
 
+static inline bool trace_array_is_readonly(struct trace_array *tr)
+{
+	/* backup instance is read only. */
+	return tr->flags & TRACE_ARRAY_FL_VMALLOC;
+}
+
 /*
  * The global tracer (top) should be the first trace array added,
  * but we check the flag anyway.
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 9b07ad9eb284..f20f717f1ea9 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1379,6 +1379,9 @@ static int __ftrace_set_clr_event(struct trace_array *tr, const char *match,
 {
 	int ret;
 
+	if (trace_array_is_readonly(tr))
+		return -EPERM;
+
 	mutex_lock(&event_mutex);
 	ret = __ftrace_set_clr_event_nolock(tr, match, sub, event, set, mod);
 	mutex_unlock(&event_mutex);
@@ -4376,6 +4379,7 @@ static int events_callback(const char *name, umode_t *mode, void **data,
 static int
 create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
 {
+	umode_t writable_mode = TRACE_MODE_WRITE;
 	struct eventfs_inode *e_events;
 	struct dentry *entry;
 	int nr_entries;
@@ -4393,9 +4397,11 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
 			.callback	= events_callback,
 		},
 	};
+	if (trace_array_is_readonly(tr))
+		writable_mode = TRACE_MODE_READ;
 
-	entry = trace_create_file("set_event", TRACE_MODE_WRITE, parent,
-				  tr, &ftrace_set_event_fops);
+	entry = trace_create_file("set_event", writable_mode, parent,
+				tr, &ftrace_set_event_fops);
 	if (!entry)
 		return -ENOMEM;
 
@@ -4410,11 +4416,11 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
 
 	/* There are not as crucial, just warn if they are not created */
 
-	trace_create_file("set_event_pid", TRACE_MODE_WRITE, parent,
+	trace_create_file("set_event_pid", writable_mode, parent,
 			  tr, &ftrace_set_event_pid_fops);
 
 	trace_create_file("set_event_notrace_pid",
-			  TRACE_MODE_WRITE, parent, tr,
+			  writable_mode, parent, tr,
 			  &ftrace_set_event_notrace_pid_fops);
 
 	tr->event_dir = e_events;


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] tracing: Add autoremove feature to the backup instance
  2026-01-07 14:45 [PATCH 0/2] tracing: Remove backup instance after read all Masami Hiramatsu (Google)
  2026-01-07 14:45 ` [PATCH 1/2] tracing: Make the backup instance readonly Masami Hiramatsu (Google)
@ 2026-01-07 14:46 ` Masami Hiramatsu (Google)
  1 sibling, 0 replies; 8+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-01-07 14:46 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
	linux-trace-kernel

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Since the backup instance is readonly, after reading all data
via pipe, no data is left on the instance. Thus it can be
removed safely after closing all files.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/trace/trace.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace.h |    6 +++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 725930f5980e..dfd4385603e6 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -590,6 +590,55 @@ void trace_set_ring_buffer_expanded(struct trace_array *tr)
 	tr->ring_buffer_expanded = true;
 }
 
+static int __remove_instance(struct trace_array *tr);
+
+static void trace_array_autoremove(struct work_struct *work)
+{
+	struct trace_array *tr = container_of(work, struct trace_array, autoremove_work);
+
+	guard(mutex)(&event_mutex);
+	guard(mutex)(&trace_types_lock);
+
+	/*
+	 * This can be fail if someone gets @tr before starting this
+	 * function, but in that case, this will be kicked again when
+	 * putting it. So we don't care the result.
+	 */
+	__remove_instance(tr);
+}
+
+static struct workqueue_struct *autoremove_wq;
+
+static void trace_array_init_autoremove(struct trace_array *tr)
+{
+	INIT_WORK(&tr->autoremove_work, trace_array_autoremove);
+}
+
+static void trace_array_kick_autoremove(struct trace_array *tr)
+{
+	if (!work_pending(&tr->autoremove_work) && autoremove_wq)
+		queue_work(autoremove_wq, &tr->autoremove_work);
+}
+
+static void trace_array_cancel_autoremove(struct trace_array *tr)
+{
+	if (work_pending(&tr->autoremove_work))
+		cancel_work(&tr->autoremove_work);
+}
+
+__init static int trace_array_init_autoremove_wq(void)
+{
+	autoremove_wq = alloc_workqueue("tr_autoremove_wq",
+					WQ_UNBOUND | WQ_HIGHPRI, 0);
+	if (!autoremove_wq) {
+		pr_err("Unable to allocate tr_autoremove_wq\n");
+		return -ENOMEM;
+	}
+	return 0;
+}
+
+late_initcall_sync(trace_array_init_autoremove_wq);
+
 LIST_HEAD(ftrace_trace_arrays);
 
 int trace_array_get(struct trace_array *this_tr)
@@ -598,7 +647,7 @@ int trace_array_get(struct trace_array *this_tr)
 
 	guard(mutex)(&trace_types_lock);
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		if (tr == this_tr) {
+		if (tr == this_tr && !tr->free_on_close) {
 			tr->ref++;
 			return 0;
 		}
@@ -611,6 +660,12 @@ static void __trace_array_put(struct trace_array *this_tr)
 {
 	WARN_ON(!this_tr->ref);
 	this_tr->ref--;
+	/*
+	 * When free_on_close is set, prepare removing the array
+	 * when the last reference is released.
+	 */
+	if (this_tr->ref == 1 && this_tr->free_on_close)
+		trace_array_kick_autoremove(this_tr);
 }
 
 /**
@@ -6212,6 +6267,10 @@ static void update_last_data(struct trace_array *tr)
 	/* Only if the buffer has previous boot data clear and update it. */
 	tr->flags &= ~TRACE_ARRAY_FL_LAST_BOOT;
 
+	/* If this is a backup instance, mark it for autoremove. */
+	if (tr->flags & TRACE_ARRAY_FL_VMALLOC)
+		tr->free_on_close = true;
+
 	/* Reset the module list and reload them */
 	if (tr->scratch) {
 		struct trace_scratch *tscratch = tr->scratch;
@@ -10392,6 +10451,8 @@ trace_array_create_systems(const char *name, const char *systems,
 	if (ftrace_allocate_ftrace_ops(tr) < 0)
 		goto out_free_tr;
 
+	trace_array_init_autoremove(tr);
+
 	ftrace_init_trace_array(tr);
 
 	init_trace_flags_index(tr);
@@ -10540,6 +10601,7 @@ static int __remove_instance(struct trace_array *tr)
 	if (update_marker_trace(tr, 0))
 		synchronize_rcu();
 
+	trace_array_cancel_autoremove(tr);
 	tracing_set_nop(tr);
 	clear_ftrace_function_probes(tr);
 	event_trace_del_tracer(tr);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bc0eeb2d1d07..a8088a106d67 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -446,6 +446,12 @@ struct trace_array {
 	 * we do not waste memory on systems that are not using tracing.
 	 */
 	bool ring_buffer_expanded;
+	/*
+	 * If the ring buffer is a read only backup instance, it will be
+	 * removed after dumping all data via pipe, because no readable data.
+	 */
+	bool free_on_close;
+	struct work_struct	autoremove_work;
 };
 
 enum {


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] tracing: Make the backup instance readonly
  2026-01-07 14:45 ` [PATCH 1/2] tracing: Make the backup instance readonly Masami Hiramatsu (Google)
@ 2026-01-07 16:41   ` Steven Rostedt
  2026-01-08  2:51     ` Masami Hiramatsu
  2026-01-08  3:07     ` Masami Hiramatsu
  0 siblings, 2 replies; 8+ messages in thread
From: Steven Rostedt @ 2026-01-07 16:41 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel

On Wed,  7 Jan 2026 23:45:59 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Since there is no reason to reuse the backup instance, make it
> readonly. Note that only backup instances are readonly, because
> other trace instances will be empty unless it is writable.
> Only backup instances have copy entries from the original.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  kernel/trace/trace.c        |   91 ++++++++++++++++++++++++++++++++-----------
>  kernel/trace/trace.h        |    6 +++
>  kernel/trace/trace_events.c |   14 +++++--
>  3 files changed, 84 insertions(+), 27 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 38f7a7a55c23..725930f5980e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4888,6 +4888,9 @@ static int tracing_open(struct inode *inode, struct file *file)
>  		int cpu = tracing_get_cpu(inode);
>  		struct array_buffer *trace_buf = &tr->array_buffer;
>  
> +		if (trace_array_is_readonly(tr))
> +			return -EPERM;

So this fails if someone opens a file in RDONLY mode?

Why?


> +
>  #ifdef CONFIG_TRACER_MAX_TRACE
>  		if (tr->current_trace->print_max)
>  			trace_buf = &tr->max_buffer;
> @@ -6077,6 +6080,9 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
>  ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
>  				  unsigned long size, int cpu_id)
>  {
> +	if (trace_array_is_readonly(tr))
> +		return -EPERM;

In fact, I don't think we need any of these.

> +
>  	guard(mutex)(&trace_types_lock);
>  
>  	if (cpu_id != RING_BUFFER_ALL_CPUS) {



> @@ -9353,12 +9381,16 @@ static void
>  tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
>  {
>  	struct dentry *d_percpu = tracing_dentry_percpu(tr, cpu);
> +	umode_t writable_mode = TRACE_MODE_WRITE;
>  	struct dentry *d_cpu;
>  	char cpu_dir[30]; /* 30 characters should be more than enough */
>  
>  	if (!d_percpu)
>  		return;
>  
> +	if (trace_array_is_readonly(tr))
> +		writable_mode = TRACE_MODE_READ;

This is more like what we should do with all the files in a read-only
instance. Just make all files not allow writes.

We may need to make sure they can't be changed to write as well. But that
will require a change to tracefs (and eventfs).

-- Steve


> +
>  	snprintf(cpu_dir, 30, "cpu%ld", cpu);
>  	d_cpu = tracefs_create_dir(cpu_dir, d_percpu);
>  	if (!d_cpu) {
> @@ -9371,7 +9403,7 @@ tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
>  				tr, cpu, &tracing_pipe_fops);
>  
>  	/* per cpu trace */
> -	trace_create_cpu_file("trace", TRACE_MODE_WRITE, d_cpu,
> +	trace_create_cpu_file("trace", writable_mode, d_cpu,
>  				tr, cpu, &tracing_fops);
>  
>  	trace_create_cpu_file("trace_pipe_raw", TRACE_MODE_READ, d_cpu,


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] tracing: Make the backup instance readonly
  2026-01-07 16:41   ` Steven Rostedt
@ 2026-01-08  2:51     ` Masami Hiramatsu
  2026-01-08  3:05       ` Steven Rostedt
  2026-01-08  3:07     ` Masami Hiramatsu
  1 sibling, 1 reply; 8+ messages in thread
From: Masami Hiramatsu @ 2026-01-08  2:51 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel

On Wed, 7 Jan 2026 11:41:33 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed,  7 Jan 2026 23:45:59 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Since there is no reason to reuse the backup instance, make it
> > readonly. Note that only backup instances are readonly, because
> > other trace instances will be empty unless it is writable.
> > Only backup instances have copy entries from the original.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  kernel/trace/trace.c        |   91 ++++++++++++++++++++++++++++++++-----------
> >  kernel/trace/trace.h        |    6 +++
> >  kernel/trace/trace_events.c |   14 +++++--
> >  3 files changed, 84 insertions(+), 27 deletions(-)
> > 
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 38f7a7a55c23..725930f5980e 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -4888,6 +4888,9 @@ static int tracing_open(struct inode *inode, struct file *file)
> >  		int cpu = tracing_get_cpu(inode);
> >  		struct array_buffer *trace_buf = &tr->array_buffer;
> >  
> > +		if (trace_array_is_readonly(tr))
> > +			return -EPERM;
> 
> So this fails if someone opens a file in RDONLY mode?
> 
> Why?

Ah, that's a bug. Let me fix it.

> 
> 
> > +
> >  #ifdef CONFIG_TRACER_MAX_TRACE
> >  		if (tr->current_trace->print_max)
> >  			trace_buf = &tr->max_buffer;
> > @@ -6077,6 +6080,9 @@ static int __tracing_resize_ring_buffer(struct trace_array *tr,
> >  ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
> >  				  unsigned long size, int cpu_id)
> >  {
> > +	if (trace_array_is_readonly(tr))
> > +		return -EPERM;
> 
> In fact, I don't think we need any of these.

Would you mean we should check readonly 

> 
> > +
> >  	guard(mutex)(&trace_types_lock);
> >  
> >  	if (cpu_id != RING_BUFFER_ALL_CPUS) {
> 
> 
> 
> > @@ -9353,12 +9381,16 @@ static void
> >  tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
> >  {
> >  	struct dentry *d_percpu = tracing_dentry_percpu(tr, cpu);
> > +	umode_t writable_mode = TRACE_MODE_WRITE;
> >  	struct dentry *d_cpu;
> >  	char cpu_dir[30]; /* 30 characters should be more than enough */
> >  
> >  	if (!d_percpu)
> >  		return;
> >  
> > +	if (trace_array_is_readonly(tr))
> > +		writable_mode = TRACE_MODE_READ;
> 
> This is more like what we should do with all the files in a read-only
> instance. Just make all files not allow writes.

Actually, that's my first prototype but it did not work (at least on
tracefs).
Superuser can write anything unless the file does not have .write
operation.

> 
> We may need to make sure they can't be changed to write as well. But that
> will require a change to tracefs (and eventfs).

Ah, you mean the permission check is not correctly done in tracefs/eventfs yet?

Thank you,

> 
> -- Steve
> 
> 
> > +
> >  	snprintf(cpu_dir, 30, "cpu%ld", cpu);
> >  	d_cpu = tracefs_create_dir(cpu_dir, d_percpu);
> >  	if (!d_cpu) {
> > @@ -9371,7 +9403,7 @@ tracing_init_tracefs_percpu(struct trace_array *tr, long cpu)
> >  				tr, cpu, &tracing_pipe_fops);
> >  
> >  	/* per cpu trace */
> > -	trace_create_cpu_file("trace", TRACE_MODE_WRITE, d_cpu,
> > +	trace_create_cpu_file("trace", writable_mode, d_cpu,
> >  				tr, cpu, &tracing_fops);
> >  
> >  	trace_create_cpu_file("trace_pipe_raw", TRACE_MODE_READ, d_cpu,
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] tracing: Make the backup instance readonly
  2026-01-08  2:51     ` Masami Hiramatsu
@ 2026-01-08  3:05       ` Steven Rostedt
  2026-01-08  5:30         ` Masami Hiramatsu
  0 siblings, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2026-01-08  3:05 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel

On Thu, 8 Jan 2026 11:51:25 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > We may need to make sure they can't be changed to write as well. But that
> > will require a change to tracefs (and eventfs).  
> 
> Ah, you mean the permission check is not correctly done in tracefs/eventfs yet?

Actually, i think we could use a different fops for read only instances
that do not have a write callback. And for eventfs we could have it for
read only instances to only create the id and format files.

-- Steve

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] tracing: Make the backup instance readonly
  2026-01-07 16:41   ` Steven Rostedt
  2026-01-08  2:51     ` Masami Hiramatsu
@ 2026-01-08  3:07     ` Masami Hiramatsu
  1 sibling, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2026-01-08  3:07 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel

On Wed, 7 Jan 2026 11:41:33 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> > @@ -4888,6 +4888,9 @@ static int tracing_open(struct inode *inode, struct file *file)
> >  		int cpu = tracing_get_cpu(inode);
> >  		struct array_buffer *trace_buf = &tr->array_buffer;
> >  
> > +		if (trace_array_is_readonly(tr))
> > +			return -EPERM;
> 
> So this fails if someone opens a file in RDONLY mode?
> 
> Why?

This is for `trace` file and this block is to erase the buffer.

 -----
	/* If this file was open for write, then erase contents */
	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
		int cpu = tracing_get_cpu(inode);
		struct array_buffer *trace_buf = &tr->array_buffer;

		if (trace_array_is_readonly(tr))
			return -EPERM;
 -----

Thus, if user opens it RDONLY mode to read the buffer, we don't care
because it is readonly (readable).

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] tracing: Make the backup instance readonly
  2026-01-08  3:05       ` Steven Rostedt
@ 2026-01-08  5:30         ` Masami Hiramatsu
  0 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2026-01-08  5:30 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel

On Wed, 7 Jan 2026 22:05:42 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 8 Jan 2026 11:51:25 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > > We may need to make sure they can't be changed to write as well. But that
> > > will require a change to tracefs (and eventfs).  
> > 
> > Ah, you mean the permission check is not correctly done in tracefs/eventfs yet?
> 
> Actually, i think we could use a different fops for read only instances
> that do not have a write callback. And for eventfs we could have it for
> read only instances to only create the id and format files.

Ah, OK. Let me update it.

Thanks,

> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-01-08  5:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-07 14:45 [PATCH 0/2] tracing: Remove backup instance after read all Masami Hiramatsu (Google)
2026-01-07 14:45 ` [PATCH 1/2] tracing: Make the backup instance readonly Masami Hiramatsu (Google)
2026-01-07 16:41   ` Steven Rostedt
2026-01-08  2:51     ` Masami Hiramatsu
2026-01-08  3:05       ` Steven Rostedt
2026-01-08  5:30         ` Masami Hiramatsu
2026-01-08  3:07     ` Masami Hiramatsu
2026-01-07 14:46 ` [PATCH 2/2] tracing: Add autoremove feature to the backup instance Masami Hiramatsu (Google)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox