From: Frederic Weisbecker <fweisbec@gmail.com>
To: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@elte.hu>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH tip 1/1] tracing: Use a single trace_enabled variable
Date: Tue, 10 Feb 2009 00:49:20 +0100 [thread overview]
Message-ID: <20090209234919.GA5070@nowhere> (raw)
In-Reply-To: <20090209211535.GA12820@ghostprotocols.net>
On Mon, Feb 09, 2009 at 07:15:35PM -0200, Arnaldo Carvalho de Melo wrote:
> Impact: API change/cleanup
>
> $ codiff kernel/trace/built-in.o.before kernel/trace/built-in.o
> kernel/trace/trace.c:
> tracing_is_enabled | -12
> __find_next_entry | -5
> 2 functions changed, 17 bytes removed, diff: -17
>
> kernel/trace/trace_sched_switch.c:
> tracing_sched_switch_assign_trace | -13
> tracing_stop_sched_switch_record | -74
> tracing_start_sched_switch_record | -30
> 3 functions changed, 117 bytes removed, diff: -117
>
> kernel/trace/trace_sysprof.c:
> stack_trace_reset | -10
> stack_trace_init | -10
> 2 functions changed, 20 bytes removed, diff: -20
>
> kernel/trace/trace_irqsoff.c:
> irqsoff_tracer_start | -16
> irqsoff_tracer_stop | -16
> irqsoff_tracer_reset | -10
> irqsoff_tracer_init | -19
> 4 functions changed, 61 bytes removed, diff: -61
>
> kernel/trace/trace_sched_wakeup.c:
> wakeup_tracer_stop | -16
> wakeup_tracer_start | -10
> start_wakeup_tracer | -19
> wakeup_tracer_reset | -10
> 4 functions changed, 55 bytes removed, diff: -55
>
> kernel/trace/built-in.o:
> 15 functions changed, 270 bytes removed, diff: -270
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
> kernel/trace/trace.c | 15 +-------------
> kernel/trace/trace.h | 3 +-
> kernel/trace/trace_irqsoff.c | 22 --------------------
> kernel/trace/trace_sched_switch.c | 40 +------------------------------------
> kernel/trace/trace_sched_wakeup.c | 26 ------------------------
> kernel/trace/trace_sysprof.c | 5 +---
> 6 files changed, 5 insertions(+), 106 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 5b1e9a9..37bb6a9 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -182,20 +182,7 @@ static struct trace_array max_tr;
> static DEFINE_PER_CPU(struct trace_array_cpu, max_data);
>
> /* tracer_enabled is used to toggle activation of a tracer */
> -static int tracer_enabled = 1;
> -
> -/**
> - * tracing_is_enabled - return tracer_enabled status
> - *
> - * This function is used by other tracers to know the status
> - * of the tracer_enabled flag. Tracers may use this function
> - * to know if it should enable their features when starting
> - * up. See irqsoff tracer for an example (start_irqsoff_tracer).
> - */
> -int tracing_is_enabled(void)
> -{
> - return tracer_enabled;
> -}
> +int tracer_enabled __read_mostly;
>
> /*
> * trace_buf_size is the size in bytes that is allocated
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 7b0518a..faa4791 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -396,8 +396,9 @@ struct trace_iterator {
> cpumask_var_t started;
> };
>
> +extern int tracer_enabled;
> +
> int tracer_init(struct tracer *t, struct trace_array *tr);
> -int tracing_is_enabled(void);
> void trace_wake_up(void);
> void tracing_reset(struct trace_array *tr, int cpu);
> void tracing_reset_online_cpus(struct trace_array *tr);
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index c6b442d..42364e8 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -19,7 +19,6 @@
> #include "trace.h"
>
> static struct trace_array *irqsoff_trace __read_mostly;
> -static int tracer_enabled __read_mostly;
>
> static DEFINE_PER_CPU(int, tracing_cpu);
>
> @@ -356,15 +355,10 @@ void trace_preempt_off(unsigned long a0, unsigned long a1)
> static void start_irqsoff_tracer(struct trace_array *tr)
> {
> register_ftrace_function(&trace_ops);
> - if (tracing_is_enabled())
> - tracer_enabled = 1;
> - else
> - tracer_enabled = 0;
> }
>
> static void stop_irqsoff_tracer(struct trace_array *tr)
> {
> - tracer_enabled = 0;
> unregister_ftrace_function(&trace_ops);
> }
>
> @@ -382,16 +376,6 @@ static void irqsoff_tracer_reset(struct trace_array *tr)
> stop_irqsoff_tracer(tr);
> }
>
> -static void irqsoff_tracer_start(struct trace_array *tr)
> -{
> - tracer_enabled = 1;
> -}
> -
> -static void irqsoff_tracer_stop(struct trace_array *tr)
> -{
> - tracer_enabled = 0;
> -}
> -
> #ifdef CONFIG_IRQSOFF_TRACER
> static int irqsoff_tracer_init(struct trace_array *tr)
> {
> @@ -405,8 +389,6 @@ static struct tracer irqsoff_tracer __read_mostly =
> .name = "irqsoff",
> .init = irqsoff_tracer_init,
> .reset = irqsoff_tracer_reset,
> - .start = irqsoff_tracer_start,
> - .stop = irqsoff_tracer_stop,
> .print_max = 1,
> #ifdef CONFIG_FTRACE_SELFTEST
> .selftest = trace_selftest_startup_irqsoff,
> @@ -431,8 +413,6 @@ static struct tracer preemptoff_tracer __read_mostly =
> .name = "preemptoff",
> .init = preemptoff_tracer_init,
> .reset = irqsoff_tracer_reset,
> - .start = irqsoff_tracer_start,
> - .stop = irqsoff_tracer_stop,
> .print_max = 1,
> #ifdef CONFIG_FTRACE_SELFTEST
> .selftest = trace_selftest_startup_preemptoff,
> @@ -459,8 +439,6 @@ static struct tracer preemptirqsoff_tracer __read_mostly =
> .name = "preemptirqsoff",
> .init = preemptirqsoff_tracer_init,
> .reset = irqsoff_tracer_reset,
> - .start = irqsoff_tracer_start,
> - .stop = irqsoff_tracer_stop,
> .print_max = 1,
> #ifdef CONFIG_FTRACE_SELFTEST
> .selftest = trace_selftest_startup_preemptirqsoff,
> diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
> index 30e14fe..828a70b 100644
> --- a/kernel/trace/trace_sched_switch.c
> +++ b/kernel/trace/trace_sched_switch.c
> @@ -15,7 +15,6 @@
> #include "trace.h"
>
> static struct trace_array *ctx_trace;
> -static int __read_mostly tracer_enabled;
> static int sched_ref;
> static DEFINE_MUTEX(sched_register_mutex);
>
> @@ -151,43 +150,6 @@ void tracing_start_sched_switch_record(void)
> }
>
> tracing_start_sched_switch();
> -
> - mutex_lock(&sched_register_mutex);
> - tracer_enabled++;
> - mutex_unlock(&sched_register_mutex);
> -}
> -
> -/**
> - * tracing_stop_sched_switch_record - start tracing context switches
> - *
> - * Turns off context switch tracing for a tracer.
> - */
> -void tracing_stop_sched_switch_record(void)
> -{
> - mutex_lock(&sched_register_mutex);
> - tracer_enabled--;
> - WARN_ON(tracer_enabled < 0);
> - mutex_unlock(&sched_register_mutex);
> -
> - tracing_stop_sched_switch();
> -}
> -
> -/**
> - * tracing_sched_switch_assign_trace - assign a trace array for ctx switch
> - * @tr: trace array pointer to assign
> - *
> - * Some tracers might want to record the context switches in their
> - * trace. This function lets those tracers assign the trace array
> - * to use.
> - */
> -void tracing_sched_switch_assign_trace(struct trace_array *tr)
> -{
> - ctx_trace = tr;
> -}
> -
> -static void stop_sched_trace(struct trace_array *tr)
> -{
> - tracing_stop_sched_switch_record();
> }
>
> static int sched_switch_trace_init(struct trace_array *tr)
> @@ -200,7 +162,7 @@ static int sched_switch_trace_init(struct trace_array *tr)
> static void sched_switch_trace_reset(struct trace_array *tr)
> {
> if (sched_ref)
> - stop_sched_trace(tr);
> + tracing_stop_sched_switch();
> }
>
> static void sched_switch_trace_start(struct trace_array *tr)
> diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
> index 96d7164..c526a1b 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -20,7 +20,6 @@
> #include "trace.h"
>
> static struct trace_array *wakeup_trace;
> -static int __read_mostly tracer_enabled;
>
> static struct task_struct *wakeup_task;
> static int wakeup_cpu;
> @@ -289,23 +288,7 @@ static void start_wakeup_tracer(struct trace_array *tr)
> }
>
> wakeup_reset(tr);
> -
> - /*
> - * Don't let the tracer_enabled = 1 show up before
> - * the wakeup_task is reset. This may be overkill since
> - * wakeup_reset does a spin_unlock after setting the
> - * wakeup_task to NULL, but I want to be safe.
> - * This is a slow path anyway.
> - */
> - smp_wmb();
> -
> register_ftrace_function(&trace_ops);
> -
> - if (tracing_is_enabled())
> - tracer_enabled = 1;
> - else
> - tracer_enabled = 0;
> -
> return;
> fail_deprobe_wake_new:
> unregister_trace_sched_wakeup_new(probe_wakeup);
> @@ -315,7 +298,6 @@ fail_deprobe:
>
> static void stop_wakeup_tracer(struct trace_array *tr)
> {
> - tracer_enabled = 0;
> unregister_ftrace_function(&trace_ops);
> unregister_trace_sched_switch(probe_wakeup_sched_switch);
> unregister_trace_sched_wakeup_new(probe_wakeup);
> @@ -352,12 +334,6 @@ static void wakeup_tracer_reset(struct trace_array *tr)
> static void wakeup_tracer_start(struct trace_array *tr)
> {
> wakeup_reset(tr);
> - tracer_enabled = 1;
> -}
> -
> -static void wakeup_tracer_stop(struct trace_array *tr)
> -{
> - tracer_enabled = 0;
> }
>
> static struct tracer wakeup_tracer __read_mostly =
> @@ -366,7 +342,6 @@ static struct tracer wakeup_tracer __read_mostly =
> .init = wakeup_tracer_init,
> .reset = wakeup_tracer_reset,
> .start = wakeup_tracer_start,
> - .stop = wakeup_tracer_stop,
> .print_max = 1,
> #ifdef CONFIG_FTRACE_SELFTEST
> .selftest = trace_selftest_startup_wakeup,
> @@ -379,7 +354,6 @@ static struct tracer wakeup_rt_tracer __read_mostly =
> .init = wakeup_rt_tracer_init,
> .reset = wakeup_tracer_reset,
> .start = wakeup_tracer_start,
> - .stop = wakeup_tracer_stop,
> .print_max = 1,
> #ifdef CONFIG_FTRACE_SELFTEST
> .selftest = trace_selftest_startup_wakeup,
> diff --git a/kernel/trace/trace_sysprof.c b/kernel/trace/trace_sysprof.c
> index 84ca9d8..5c986b4 100644
> --- a/kernel/trace/trace_sysprof.c
> +++ b/kernel/trace/trace_sysprof.c
> @@ -18,8 +18,7 @@
>
> #include "trace.h"
>
> -static struct trace_array *sysprof_trace;
> -static int __read_mostly tracer_enabled;
> +static struct trace_array *sysprof_trace;
>
> /*
> * 1 msec sample interval by default:
> @@ -230,7 +229,6 @@ static void stop_stack_trace(struct trace_array *tr)
> {
> mutex_lock(&sample_timer_lock);
> stop_stack_timers();
> - tracer_enabled = 0;
> mutex_unlock(&sample_timer_lock);
> }
>
> @@ -240,7 +238,6 @@ static int stack_trace_init(struct trace_array *tr)
>
> mutex_lock(&sample_timer_lock);
> start_stack_timers();
> - tracer_enabled = 1;
> mutex_unlock(&sample_timer_lock);
> return 0;
> }
> --
> 1.6.0.6
>
That looks good :-)
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
next prev parent reply other threads:[~2009-02-09 23:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-09 21:15 [PATCH tip 1/1] tracing: Use a single trace_enabled variable Arnaldo Carvalho de Melo
2009-02-09 23:49 ` Frederic Weisbecker [this message]
2009-02-11 12:06 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090209234919.GA5070@nowhere \
--to=fweisbec@gmail.com \
--cc=acme@ghostprotocols.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.