| 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: <304748287.27041685.1457530725205.JavaMail.zimbra@redhat.com>
Date: Wed, 9 Mar 2016 08:38:45 -0500 (EST)
From: Chunyu Hu <chuhu@...hat.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: liwan@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] tracing: make tracer_flags use the right set_flag
callback
----- Original Message -----
> From: "Steven Rostedt" <rostedt@...dmis.org>
> To: "Chunyu Hu" <chuhu@...hat.com>
> Cc: liwan@...hat.com, linux-kernel@...r.kernel.org
> Sent: Wednesday, March 9, 2016 12:23:12 AM
> Subject: Re: [PATCH 1/2] tracing: make tracer_flags use the right set_flag callback
>
> On Tue, 8 Mar 2016 10:55:30 -0500 (EST)
> Chunyu Hu <chuhu@...hat.com> wrote:
>
>
> > Sure! But my patch/build work are all manual, so I guess I can't keep with
> > your speed, but i will do. Thx.
>
> Here's the latest version of your patch ;-)
Thanks. It's amazing, all the call trace in ftrace.c are gone. I did
the ltp ftrace stress tests and some regression tests on it. I didn't hit
any call trace. Thanks for the modification of the code. Just hit several
dmesg like: kernel: failed to start wakeup tracer.
BTW, may I ask the question that what's the difference of func_stack_trace
and stacktrace? And Is there a case that the tracing_on can't be setup? I
haven't touch many parts of the code. Thanks ;)
> -- Steve
>
> From d39cdd2036a63eef17a14efbd969405ca5612886 Mon Sep 17 00:00:00 2001
> From: Chunyu Hu <chuhu@...hat.com>
> Date: Tue, 8 Mar 2016 21:37:01 +0800
> Subject: [PATCH] tracing: Make tracer_flags use the right set_flag callback
>
> When I was updating the ftrace_stress test of ltp. I encountered
> a strange phenomemon, excute following steps:
>
> echo nop > /sys/kernel/debug/tracing/current_tracer
> echo 0 > /sys/kernel/debug/tracing/options/funcgraph-cpu
> bash: echo: write error: Invalid argument
>
> check dmesg:
> [ 1024.903855] nop_test_refuse flag set to 0: we refuse.Now cat trace_options
> to see the result
>
> The reason is that the trace option test will randomly setup trace
> option under tracing/options no matter what the current_tracer is.
> but the set_tracer_option is always using the set_flag callback
> from the current_tracer. This patch adds a pointer to tracer_flags
> and make it point to the tracer it belongs to. When the option is
> setup, the set_flag of the right tracer will be used no matter
> what the the current_tracer is.
>
> And the old dummy_tracer_flags is used for all the tracers which
> doesn't have a tracer_flags, having issue to use it to save the
> pointer of a tracer. So remove it and use dynamic dummy tracer_flags
> for tracers needing a dummy tracer_flags, as a result, there are no
> tracers sharing tracer_flags, so remove the check code.
>
> And save the current tracer to trace_option_dentry seems not good as
> it may waste mem space when mount the debug/trace fs more than one time.
>
> Link:
> http://lkml.kernel.org/r/1457444222-8654-1-git-send-email-chuhu@redhat.com
>
> Signed-off-by: Chunyu Hu <chuhu@...hat.com>
> [ Fixed up function tracer options to work with the change ]
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
> kernel/trace/trace.c | 28 ++++++++++++++--------------
> kernel/trace/trace.h | 1 +
> kernel/trace/trace_functions.c | 6 ++++++
> 3 files changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d9293402ee68..b401a1892dc6 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -74,11 +74,6 @@ static struct tracer_opt dummy_tracer_opt[] = {
> { }
> };
>
> -static struct tracer_flags dummy_tracer_flags = {
> - .val = 0,
> - .opts = dummy_tracer_opt
> -};
> -
> static int
> dummy_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
> {
> @@ -1258,12 +1253,20 @@ int __init register_tracer(struct tracer *type)
>
> if (!type->set_flag)
> type->set_flag = &dummy_set_flag;
> - if (!type->flags)
> - type->flags = &dummy_tracer_flags;
> - else
> + if (!type->flags) {
> + /*allocate a dummy tracer_flags*/
> + type->flags = kmalloc(sizeof(*type->flags), GFP_KERNEL);
> + if (!type->flags)
> + return -ENOMEM;
> + type->flags->val = 0;
> + type->flags->opts = dummy_tracer_opt;
> + } else
> if (!type->flags->opts)
> type->flags->opts = dummy_tracer_opt;
>
> + /* store the tracer for __set_tracer_option */
> + type->flags->trace = type;
> +
> ret = run_tracer_selftest(type);
> if (ret < 0)
> goto out;
> @@ -3505,7 +3508,7 @@ static int __set_tracer_option(struct trace_array *tr,
> struct tracer_flags *tracer_flags,
> struct tracer_opt *opts, int neg)
> {
> - struct tracer *trace = tr->current_trace;
> + struct tracer *trace = tracer_flags->trace;
> int ret;
>
> ret = trace->set_flag(tr, tracer_flags->val, opts->bit, !neg);
> @@ -6391,11 +6394,8 @@ create_trace_option_files(struct trace_array *tr,
> struct tracer *tracer)
> return;
>
> for (i = 0; i < tr->nr_topts; i++) {
> - /*
> - * Check if these flags have already been added.
> - * Some tracers share flags.
> - */
> - if (tr->topts[i].tracer->flags == tracer->flags)
> + /* Make sure there's no duplicate flags. */
> + if (WARN_ON_ONCE(tr->topts[i].tracer->flags == tracer->flags))
Thanks! It's really a safer way to throw out this warn if there is duplicate.
> return;
> }
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 8414fa40bf27..b4cae47f283e 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -345,6 +345,7 @@ struct tracer_opt {
> struct tracer_flags {
> u32 val;
> struct tracer_opt *opts;
> + struct tracer *trace;
> };
>
> /* Makes more easy to define a tracer opt */
> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> index fcd41a166405..5a095c2e4b69 100644
> --- a/kernel/trace/trace_functions.c
> +++ b/kernel/trace/trace_functions.c
> @@ -219,6 +219,8 @@ static void tracing_stop_function_trace(struct
> trace_array *tr)
> unregister_ftrace_function(tr->ops);
> }
>
> +static struct tracer function_trace;
> +
> static int
> func_set_flag(struct trace_array *tr, u32 old_flags, u32 bit, int set)
> {
> @@ -228,6 +230,10 @@ func_set_flag(struct trace_array *tr, u32 old_flags, u32
> bit, int set)
> if (!!set == !!(func_flags.val & TRACE_FUNC_OPT_STACK))
> break;
>
> + /* We can change this flag when not running. */
> + if (tr->current_trace != &function_trace)
> + break;
> +
At first I thought it should have some relation with the funcgraph trace. so i
guessed was wrong. Thx.
> unregister_ftrace_function(tr->ops);
>
> if (set) {
> --
> 1.8.3.1
>
>
--
Regards,
Chunyu Hu
Powered by blists - more mailing lists