Re: [PATCH 03/10] ftrace: fix sched_switch API

[Frédéric Weisbecker]

2008/11/8 Steven Rostedt <rostedt@xxxxxxxxxxx>:
> Impact: fix for sched_switch that broke dynamic ftrace startup
>
> The commit: tracing/fastboot: use sched switch tracer from boot tracer
> broke the API of the sched_switch trace. The use of the
> tracing_start/stop_cmdline record is for only recording the cmdline,
> NOT recording the schedule switches themselves.
>
> Seeing that the boot tracer broke the API to do something that it
> wanted, this patch adds a new interface for the API while
> puting back the original interface of the old API.
>
> Signed-off-by: Steven Rostedt <srostedt@xxxxxxxxxx>
> CC: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> ---
>  kernel/trace/trace.h              |    4 ++-
>  kernel/trace/trace_boot.c         |    6 ++--
>  kernel/trace/trace_sched_switch.c |   50 +++++++++++++++++++++++++++++-------
>  3 files changed, 46 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index db12e16..25abfc4 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -324,7 +324,9 @@ void trace_function(struct trace_array *tr,
>
>  void tracing_start_cmdline_record(void);
>  void tracing_stop_cmdline_record(void);
> -void tracing_cmdline_assign_trace(struct trace_array *tr);
> +void tracing_sched_switch_assign_trace(struct trace_array *tr);
> +void tracing_stop_sched_switch_record(void);
> +void tracing_start_sched_switch_record(void);
>  int register_tracer(struct tracer *type);
>  void unregister_tracer(struct tracer *type);
>
> diff --git a/kernel/trace/trace_boot.c b/kernel/trace/trace_boot.c
> index 662cb91..0203c10 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -28,13 +28,13 @@ void start_boot_trace(void)
>  void enable_boot_trace(void)
>  {
>        if (pre_initcalls_finished)
> -               tracing_start_cmdline_record();
> +               tracing_start_sched_switch_record();
>  }
>
>  void disable_boot_trace(void)
>  {
>        if (pre_initcalls_finished)
> -               tracing_stop_cmdline_record();
> +               tracing_stop_sched_switch_record();
>  }
>
>  static void reset_boot_trace(struct trace_array *tr)
> @@ -55,7 +55,7 @@ static void boot_trace_init(struct trace_array *tr)
>        for_each_cpu_mask(cpu, cpu_possible_map)
>                tracing_reset(tr, cpu);
>
> -       tracing_cmdline_assign_trace(tr);
> +       tracing_sched_switch_assign_trace(tr);
>  }
>
>  static void boot_trace_ctrl_update(struct trace_array *tr)
> diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
> index fbf05df..79410db 100644
> --- a/kernel/trace/trace_sched_switch.c
> +++ b/kernel/trace/trace_sched_switch.c
> @@ -125,20 +125,16 @@ static void tracing_sched_unregister(void)
>  static void tracing_start_sched_switch(void)
>  {
>        mutex_lock(&sched_register_mutex);
> -       if (!(sched_ref++)) {
> -               tracer_enabled = 1;
> +       if (!(sched_ref++))
>                tracing_sched_register();
> -       }
>        mutex_unlock(&sched_register_mutex);
>  }
>
>  static void tracing_stop_sched_switch(void)
>  {
>        mutex_lock(&sched_register_mutex);
> -       if (!(--sched_ref)) {
> +       if (!(--sched_ref))
>                tracing_sched_unregister();
> -               tracer_enabled = 0;
> -       }
>        mutex_unlock(&sched_register_mutex);
>  }
>
> @@ -153,14 +149,48 @@ void tracing_stop_cmdline_record(void)
>  }
>
>  /**
> - * tracing_cmdline_assign_trace - assign a trace array for ctx switch
> + * tracing_start_sched_switch_record - start tracing context switches
> + *
> + * Turns on context switch tracing for a tracer.
> + */
> +void tracing_start_sched_switch_record(void)
> +{
> +       if (unlikely(!ctx_trace)) {
> +               WARN_ON(1);
> +               return;
> +       }
> +
> +       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_cmdline_assign_trace(struct trace_array *tr)
> +void tracing_sched_switch_assign_trace(struct trace_array *tr)
>  {
>        ctx_trace = tr;
>  }
> @@ -168,12 +198,12 @@ void tracing_cmdline_assign_trace(struct trace_array *tr)
>  static void start_sched_trace(struct trace_array *tr)
>  {
>        sched_switch_reset(tr);
> -       tracing_start_cmdline_record();
> +       tracing_start_sched_switch_record();
>  }
>
>  static void stop_sched_trace(struct trace_array *tr)
>  {
> -       tracing_stop_cmdline_record();
> +       tracing_stop_sched_switch_record();
>  }
>
>  static void sched_switch_trace_init(struct trace_array *tr)


Sorry, I misunderstood the sense of these functions and then used them only for
what they did: start/stop sched event capture.

Acked-by: Frederic Weisbecker < fweisbec@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/