Re: [PATCH v5 2/4] tracing: Make the backup instance non-reusable
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2026-01-29 20:07:35
Also in:
lkml
On Wed, 28 Jan 2026 09:09:56 +0900 "Masami Hiramatsu (Google)" [off-list ref] wrote:
quoted hunk ↗ jump to hunk
@@ -10632,17 +10658,23 @@ 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); + tr, &show_traces_fops); - trace_create_file("current_tracer", TRACE_MODE_WRITE, d_tracer, - tr, &set_tracer_fops); + 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); + /* Options are used for changing print-format even for readonly instance. */ trace_create_file("trace_options", TRACE_MODE_WRITE, d_tracer, tr, &tracing_iter_fops);@@ -10652,27 +10684,35 @@ init_tracer_tracefs(struct trace_array *tr, struct dentry *d_tracer) 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);
Hmm, why remove the free_buffer. It just shrinks the buffer down to a minimum. Perhaps its useless, but I it doesn't write to the buffer. Sure it removes data but so does trace_pipe.
- 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("buffer_percent", TRACE_MODE_WRITE, d_tracer,
+ tr, &buffer_percent_fops);
+
+ trace_create_file("syscall_user_buf_size", TRACE_MODE_WRITE, d_tracer,
+ tr, &tracing_syscall_buf_fops);
+ }
+
+ 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);Hmm, should tracing_on exist in read only mode?
quoted hunk ↗ jump to hunk
trace_create_file("timestamp_mode", TRACE_MODE_READ, d_tracer, tr,@@ -10680,41 +10720,38 @@ 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, - 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, - 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) { trace_create_file("last_boot_info", TRACE_MODE_READ, d_tracer, tr, &last_boot_fops); #ifdef CONFIG_TRACER_SNAPSHOT - } else { + } else if (!readonly) { trace_create_file("snapshot", TRACE_MODE_WRITE, d_tracer, tr, &snapshot_fops); #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);
Why not move this up into the "if (!readonly) {" block above?
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);
Same here. Or just move the readonly block down to the end of the function.
quoted hunk ↗ jump to hunk
} #ifdef CONFIG_TRACEFS_AUTOMOUNT_DEPRECATEDdiff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index 69e7defba6c6..0adc644084bf 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h@@ -33,6 +33,7 @@ #define TRACE_MODE_WRITE 0640 #define TRACE_MODE_READ 0440 +#define TRACE_MODE_WRITE_MASK (TRACE_MODE_WRITE & ~TRACE_MODE_READ) enum trace_type { __TRACE_FIRST_TYPE = 0,@@ -483,6 +484,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;
Hmm, I wonder if we should create a RDONLY flag for the trace_array? -- Steve
quoted hunk ↗ jump to hunk
+} + /* * The global tracer (top) should be the first trace array added, * but we check the flag anyway.@@ -681,7 +688,6 @@ struct dentry *trace_create_file(const char *name, void *data, const struct file_operations *fops); - /** * tracer_tracing_is_on_cpu - show real state of ring buffer enabled on for a cpu * @tr : the trace array to know if ring buffer is enableddiff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c index dbe29b4c6a7a..2ca2541c8a58 100644 --- a/kernel/trace/trace_boot.c +++ b/kernel/trace/trace_boot.c@@ -61,7 +61,8 @@ trace_boot_set_instance_options(struct trace_array *tr, struct xbc_node *node) v = memparse(p, NULL); if (v < PAGE_SIZE) pr_err("Buffer size is too small: %s\n", p); - if (tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0) + if (trace_array_is_readonly(tr) || + tracing_resize_ring_buffer(tr, v, RING_BUFFER_ALL_CPUS) < 0) pr_err("Failed to resize trace buffer to %s\n", p); }@@ -597,7 +598,7 @@ trace_boot_enable_tracer(struct trace_array *tr, struct xbc_node *node) p = xbc_node_find_value(node, "tracer", NULL); if (p && *p != '\0') { - if (tracing_set_tracer(tr, p) < 0) + if (trace_array_is_readonly(tr) || tracing_set_tracer(tr, p) < 0) pr_err("Failed to set given tracer: %s\n", p); }