| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1516bba1-15ea-4b8a-ac83-e7db98b37397@efficios.com>
Date: Wed, 9 Apr 2025 12:18:22 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Steven Rostedt <rostedt@...dmis.org>, LKML
<linux-kernel@...r.kernel.org>,
Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH] tracing: Allow the top level trace_marker to write into
another instance
On 2025-04-08 11:27, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@...dmis.org>
>
> There are applications that have it hard coded to write into the top level
> trace_marker instance (/sys/kernel/tracing/trace_marker). This can be
> annoying if a profiler is using that instance for other work, or if it
> needs all writes to go into a new instance.
>
> A new option is created called "redirect_trace_marker". By default, the
> top level has this set, as that is the default buffer that writing into
> the top level trace_marker file will go to. But now if an instance is
> created and sets this option, all writes into the top level trace_marker
> will be redirected into that instance buffer just as if an application
> were to write into the instance's trace_marker file.
>
> Only one instance can have this option set. If the option is set in
> another instance, then it will clear the option from the previous
> instance.
>
> The top level instance can set this option but it can not clear it, and
> it will bring the writing of the trace_marker back to its buffer.
Why choose to redirect the top level trace marker from the global
buffer to the specific one rather than allow it to iterate on a
set of "interested" buffers ?
Thanks,
Mathieu
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> Documentation/trace/ftrace.rst | 15 +++++++++
> kernel/trace/trace.c | 60 +++++++++++++++++++++++++++++++---
> kernel/trace/trace.h | 1 +
> 3 files changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
> index c9e88bf65709..6b3915139347 100644
> --- a/Documentation/trace/ftrace.rst
> +++ b/Documentation/trace/ftrace.rst
> @@ -1205,6 +1205,21 @@ Here are the available options:
> default instance. The only way the top level instance has this flag
> cleared, is by it being set in another instance.
>
> + redirect_trace_marker
> + This is similar to the trace_printk_dest option above, but it works
> + for the top level trace_marker (/sys/kernel/tracing/trace_marker).
> + If there are applications that hard code writing into the top level
> + trace_marker file, and the tooling would like it to go into an
> + instance, this option can be used. Create an instance and set this
> + option, and then all writes into the top level trace_marker file will
> + be redirected into that instance.
> +
> + This flag cannot be cleared by the top level instance, as it is the
> + default instance. The only way the top level instance has this flag
> + cleared, is by it being set in another instance. By setting this flag
> + in the top level instance, it will put the writing into the
> + trace_marker file back into the top level instance buffer.
> +
> annotate
> It is sometimes confusing when the CPU buffers are full
> and one CPU buffer had a lot of events recently, thus
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 8ddf6b17215c..ec7e15434759 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -493,7 +493,8 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_export);
> TRACE_ITER_ANNOTATE | TRACE_ITER_CONTEXT_INFO | \
> TRACE_ITER_RECORD_CMD | TRACE_ITER_OVERWRITE | \
> TRACE_ITER_IRQ_INFO | TRACE_ITER_MARKERS | \
> - TRACE_ITER_HASH_PTR | TRACE_ITER_TRACE_PRINTK)
> + TRACE_ITER_HASH_PTR | TRACE_ITER_TRACE_PRINTK | \
> + TRACE_ITER_REDIRECT_MARKER)
>
> /* trace_options that are only supported by global_trace */
> #define TOP_LEVEL_TRACE_FLAGS (TRACE_ITER_PRINTK | \
> @@ -501,7 +502,8 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_export);
>
> /* trace_flags that are default zero for instances */
> #define ZEROED_TRACE_FLAGS \
> - (TRACE_ITER_EVENT_FORK | TRACE_ITER_FUNC_FORK | TRACE_ITER_TRACE_PRINTK)
> + (TRACE_ITER_EVENT_FORK | TRACE_ITER_FUNC_FORK | TRACE_ITER_TRACE_PRINTK | \
> + TRACE_ITER_REDIRECT_MARKER)
>
> /*
> * The global_trace is the descriptor that holds the top-level tracing
> @@ -512,6 +514,7 @@ static struct trace_array global_trace = {
> };
>
> static struct trace_array *printk_trace = &global_trace;
> +static struct trace_array *marker_trace = &global_trace;
>
> static __always_inline bool printk_binsafe(struct trace_array *tr)
> {
> @@ -534,6 +537,16 @@ static void update_printk_trace(struct trace_array *tr)
> tr->trace_flags |= TRACE_ITER_TRACE_PRINTK;
> }
>
> +static void update_marker_trace(struct trace_array *tr)
> +{
> + if (marker_trace == tr)
> + return;
> +
> + marker_trace->trace_flags &= ~TRACE_ITER_REDIRECT_MARKER;
> + marker_trace = tr;
> + tr->trace_flags |= TRACE_ITER_REDIRECT_MARKER;
> +}
> +
> void trace_set_ring_buffer_expanded(struct trace_array *tr)
> {
> if (!tr)
> @@ -4755,8 +4768,22 @@ int tracing_single_release_file_tr(struct inode *inode, struct file *filp)
>
> static int tracing_mark_open(struct inode *inode, struct file *filp)
> {
> + struct trace_array *tr = inode->i_private;
> + int ret;
> +
> stream_open(inode, filp);
> - return tracing_open_generic_tr(inode, filp);
> +
> + /* The top level marker can have it redirected to an instance */
> + if (tr == &global_trace)
> + tr = marker_trace;
> +
> + ret = tracing_check_open_get_tr(tr);
> + if (ret)
> + return ret;
> +
> + filp->private_data = tr;
> +
> + return 0;
> }
>
> static int tracing_release(struct inode *inode, struct file *file)
> @@ -4799,7 +4826,7 @@ static int tracing_release(struct inode *inode, struct file *file)
>
> int tracing_release_generic_tr(struct inode *inode, struct file *file)
> {
> - struct trace_array *tr = inode->i_private;
> + struct trace_array *tr = file->private_data;
>
> trace_array_put(tr);
> return 0;
> @@ -5189,7 +5216,8 @@ int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled)
> {
> if ((mask == TRACE_ITER_RECORD_TGID) ||
> (mask == TRACE_ITER_RECORD_CMD) ||
> - (mask == TRACE_ITER_TRACE_PRINTK))
> + (mask == TRACE_ITER_TRACE_PRINTK) ||
> + (mask == TRACE_ITER_REDIRECT_MARKER))
> lockdep_assert_held(&event_mutex);
>
> /* do nothing if flag is already set */
> @@ -5220,6 +5248,25 @@ int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled)
> }
> }
>
> + if (mask == TRACE_ITER_REDIRECT_MARKER) {
> + if (enabled) {
> + update_marker_trace(tr);
> + } else {
> + /*
> + * The global_trace cannot clear this.
> + * It's flag only gets cleared if another instance sets it.
> + */
> + if (marker_trace == &global_trace)
> + return -EINVAL;
> + /*
> + * An instance must always have it set.
> + * by default, that's the global_trace instane.
> + */
> + if (marker_trace == tr)
> + update_marker_trace(&global_trace);
> + }
> + }
> +
> if (enabled)
> tr->trace_flags |= mask;
> else
> @@ -9897,6 +9944,9 @@ static int __remove_instance(struct trace_array *tr)
> if (printk_trace == tr)
> update_printk_trace(&global_trace);
>
> + if (marker_trace == tr)
> + update_marker_trace(&global_trace);
> +
> 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 79be1995db44..602457eda8e8 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1368,6 +1368,7 @@ extern int trace_get_user(struct trace_parser *parser, const char __user *ubuf,
> C(MARKERS, "markers"), \
> C(EVENT_FORK, "event-fork"), \
> C(TRACE_PRINTK, "trace_printk_dest"), \
> + C(REDIRECT_MARKER, "redirect_trace_marker"),\
> C(PAUSE_ON_TRACE, "pause-on-trace"), \
> C(HASH_PTR, "hash-ptr"), /* Print hashed pointer */ \
> FUNCTION_FLAGS \
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
Powered by blists - more mailing lists