[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c62985530812190247i705d9c3er7662a1b133c6eb29@mail.gmail.com>
Date: Fri, 19 Dec 2008 11:47:06 +0100
From: "Frédéric Weisbecker" <fweisbec@...il.com>
To: "Pekka J Enberg" <penberg@...helsinki.fi>
Cc: rostedt@...dmis.org, mingo@...e.hu, linux-kernel@...r.kernel.org,
"Pekka Paalanen" <pq@....fi>,
"Markus Metzger" <markus.t.metzger@...il.com>
Subject: Re: [PATCH] ftrace: introduce tracing_reset_online_cpus() helper
2008/12/19 Pekka J Enberg <penberg@...helsinki.fi>:
> From: Pekka Enberg <penberg@...helsinki.fi>
>
> This patch factors out common code from multiple tracers into a
> tracing_reset_online_cpus() function and converts the tracers to use it.
>
> Signed-off-by: Pekka Enberg <penberg@...helsinki.fi>
> ---
> kernel/trace/trace.c | 10 ++++++++++
> kernel/trace/trace.h | 1 +
> kernel/trace/trace_boot.c | 12 +-----------
> kernel/trace/trace_functions.c | 14 ++------------
> kernel/trace/trace_hw_branches.c | 14 ++------------
> kernel/trace/trace_mmiotrace.c | 6 +-----
> kernel/trace/trace_sched_switch.c | 14 ++------------
> kernel/trace/trace_sysprof.c | 12 +-----------
> 8 files changed, 20 insertions(+), 63 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 1a3d6b3..c39a0d8 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -679,6 +679,16 @@ void tracing_reset(struct trace_array *tr, int cpu)
> ftrace_enable_cpu();
> }
>
> +void tracing_reset_online_cpus(struct trace_array *tr)
> +{
> + int cpu;
> +
> + tr->time_start = ftrace_now(tr->cpu);
> +
> + for_each_online_cpu(cpu)
> + tracing_reset(tr, cpu);
> +}
> +
> #define SAVED_CMDLINES 128
> static unsigned map_pid_to_cmdline[PID_MAX_DEFAULT+1];
> static unsigned map_cmdline_to_pid[SAVED_CMDLINES];
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index fc75dce..cc7a4f8 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -374,6 +374,7 @@ struct trace_iterator {
> 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);
> int tracing_open_generic(struct inode *inode, struct file *filp);
> struct dentry *tracing_init_dentry(void);
> void init_tracer_sysprof_debugfs(struct dentry *d_tracer);
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index a4fa2c5..3ccebde 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -37,16 +37,6 @@ void disable_boot_trace(void)
> tracing_stop_sched_switch_record();
> }
>
> -static void reset_boot_trace(struct trace_array *tr)
> -{
> - int cpu;
> -
> - tr->time_start = ftrace_now(tr->cpu);
> -
> - for_each_online_cpu(cpu)
> - tracing_reset(tr, cpu);
> -}
> -
> static int boot_trace_init(struct trace_array *tr)
> {
> int cpu;
> @@ -130,7 +120,7 @@ struct tracer boot_tracer __read_mostly =
> {
> .name = "initcall",
> .init = boot_trace_init,
> - .reset = reset_boot_trace,
> + .reset = tracing_reset_online_cpus,
> .print_line = initcall_print_line,
> };
>
> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> index e74f6d0..9236d7e 100644
> --- a/kernel/trace/trace_functions.c
> +++ b/kernel/trace/trace_functions.c
> @@ -16,20 +16,10 @@
>
> #include "trace.h"
>
> -static void function_reset(struct trace_array *tr)
> -{
> - int cpu;
> -
> - tr->time_start = ftrace_now(tr->cpu);
> -
> - for_each_online_cpu(cpu)
> - tracing_reset(tr, cpu);
> -}
> -
> static void start_function_trace(struct trace_array *tr)
> {
> tr->cpu = get_cpu();
> - function_reset(tr);
> + tracing_reset_online_cpus(tr);
> put_cpu();
>
> tracing_start_cmdline_record();
> @@ -55,7 +45,7 @@ static void function_trace_reset(struct trace_array *tr)
>
> static void function_trace_start(struct trace_array *tr)
> {
> - function_reset(tr);
> + tracing_reset_online_cpus(tr);
> }
>
> static struct tracer function_trace __read_mostly =
> diff --git a/kernel/trace/trace_hw_branches.c b/kernel/trace/trace_hw_branches.c
> index ee29e01..b6a3e20 100644
> --- a/kernel/trace/trace_hw_branches.c
> +++ b/kernel/trace/trace_hw_branches.c
> @@ -25,16 +25,6 @@ static DEFINE_PER_CPU(unsigned char[SIZEOF_BTS], buffer);
> #define this_buffer per_cpu(buffer, smp_processor_id())
>
>
> -static void bts_trace_reset(struct trace_array *tr)
> -{
> - int cpu;
> -
> - tr->time_start = ftrace_now(tr->cpu);
> -
> - for_each_online_cpu(cpu)
> - tracing_reset(tr, cpu);
> -}
> -
> static void bts_trace_start_cpu(void *arg)
> {
> if (this_tracer)
> @@ -54,7 +44,7 @@ static void bts_trace_start(struct trace_array *tr)
> {
> int cpu;
>
> - bts_trace_reset(tr);
> + tracing_reset_online_cpus(tr);
>
> for_each_cpu_mask(cpu, cpu_possible_map)
> smp_call_function_single(cpu, bts_trace_start_cpu, NULL, 1);
> @@ -78,7 +68,7 @@ static void bts_trace_stop(struct trace_array *tr)
>
> static int bts_trace_init(struct trace_array *tr)
> {
> - bts_trace_reset(tr);
> + tracing_reset_online_cpus(tr);
> bts_trace_start(tr);
>
> return 0;
> diff --git a/kernel/trace/trace_mmiotrace.c b/kernel/trace/trace_mmiotrace.c
> index 2fb6da6..fffcb06 100644
> --- a/kernel/trace/trace_mmiotrace.c
> +++ b/kernel/trace/trace_mmiotrace.c
> @@ -22,14 +22,10 @@ static unsigned long prev_overruns;
>
> static void mmio_reset_data(struct trace_array *tr)
> {
> - int cpu;
> -
> overrun_detected = false;
> prev_overruns = 0;
> - tr->time_start = ftrace_now(tr->cpu);
>
> - for_each_online_cpu(cpu)
> - tracing_reset(tr, cpu);
> + tracing_reset_online_cpus(tr);
> }
>
> static int mmio_trace_init(struct trace_array *tr)
> diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
> index 8633905..7f59f91 100644
> --- a/kernel/trace/trace_sched_switch.c
> +++ b/kernel/trace/trace_sched_switch.c
> @@ -72,16 +72,6 @@ probe_sched_wakeup(struct rq *__rq, struct task_struct *wakee)
> local_irq_restore(flags);
> }
>
> -static void sched_switch_reset(struct trace_array *tr)
> -{
> - int cpu;
> -
> - tr->time_start = ftrace_now(tr->cpu);
> -
> - for_each_online_cpu(cpu)
> - tracing_reset(tr, cpu);
> -}
> -
> static int tracing_sched_register(void)
> {
> int ret;
> @@ -197,7 +187,7 @@ void tracing_sched_switch_assign_trace(struct trace_array *tr)
>
> static void start_sched_trace(struct trace_array *tr)
> {
> - sched_switch_reset(tr);
> + tracing_reset_online_cpus(tr);
> tracing_start_sched_switch_record();
> }
>
> @@ -221,7 +211,7 @@ static void sched_switch_trace_reset(struct trace_array *tr)
>
> static void sched_switch_trace_start(struct trace_array *tr)
> {
> - sched_switch_reset(tr);
> + tracing_reset_online_cpus(tr);
> tracing_start_sched_switch();
> }
>
> diff --git a/kernel/trace/trace_sysprof.c b/kernel/trace/trace_sysprof.c
> index 54960ed..01becf1 100644
> --- a/kernel/trace/trace_sysprof.c
> +++ b/kernel/trace/trace_sysprof.c
> @@ -234,20 +234,10 @@ static void stop_stack_timers(void)
> stop_stack_timer(cpu);
> }
>
> -static void stack_reset(struct trace_array *tr)
> -{
> - int cpu;
> -
> - tr->time_start = ftrace_now(tr->cpu);
> -
> - for_each_online_cpu(cpu)
> - tracing_reset(tr, cpu);
> -}
> -
> static void start_stack_trace(struct trace_array *tr)
> {
> mutex_lock(&sample_timer_lock);
> - stack_reset(tr);
> + tracing_reset_online_cpus(tr);
> start_stack_timers();
> tracer_enabled = 1;
> mutex_unlock(&sample_timer_lock);
That's looks good.
By the past, I also suggested Steven to automatically reset the traces
buffer each time a tracer is started, that
would factor out the code a bit more. I don't think one tracer would
avoid to reset the buffer once it is started, and
I don't think it is needed to reset twice on tracer switching: on stop
of the old tracer and on start on the new. Only
on start should be enough.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists