linux-kernel - Re: [PATCH 02/10] ftrace: fix boot trace sched startup

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 8 Nov 2008 05:12:40 +0100
From:	"Frédéric Weisbecker" <fweisbec@...il.com>
To:	"Steven Rostedt" <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, "Ingo Molnar" <mingo@...e.hu>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Peter Zijlstra" <peterz@...radead.org>,
	"Steven Rostedt" <srostedt@...hat.com>
Subject: Re: [PATCH 02/10] ftrace: fix boot trace sched startup

2008/11/8 Steven Rostedt <rostedt@...dmis.org>:
> Impact: boot tracer startup modified
>
> The boot tracer calls into some of the schedule tracing private functions
> that should not be exported. This patch cleans it up, and makes
> way for further changes in the ftrace infrastructure.
>
> This patch adds a api to assign a tracer array to the schedule
> context switch tracer.
>
> Signed-off-by: Steven Rostedt <srostedt@...hat.com>
> CC:  Frederic Weisbecker <fweisbec@...il.com>
> ---
>  kernel/trace/trace.h              |    2 +-
>  kernel/trace/trace_boot.c         |    9 +++++++--
>  kernel/trace/trace_sched_switch.c |   15 ++++++++++++++-
>  3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 3422489..db12e16 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -49,7 +49,6 @@ struct ftrace_entry {
>        unsigned long           parent_ip;
>  };
>  extern struct tracer boot_tracer;
> -extern struct tracer sched_switch_trace; /* Used by the boot tracer */
>
>  /*
>  * Context switch trace entry - which task (and prio) we switched from/to:
> @@ -325,6 +324,7 @@ 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);
>  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 bd5046c..662cb91 100644
> --- a/kernel/trace/trace_boot.c
> +++ b/kernel/trace/trace_boot.c
> @@ -39,7 +39,12 @@ void disable_boot_trace(void)
>
>  static void reset_boot_trace(struct trace_array *tr)
>  {
> -       sched_switch_trace.reset(tr);
> +       int cpu;
> +
> +       tr->time_start = ftrace_now(tr->cpu);
> +
> +       for_each_online_cpu(cpu)
> +               tracing_reset(tr, cpu);
>  }
>
>  static void boot_trace_init(struct trace_array *tr)
> @@ -50,7 +55,7 @@ static void boot_trace_init(struct trace_array *tr)
>        for_each_cpu_mask(cpu, cpu_possible_map)
>                tracing_reset(tr, cpu);
>
> -       sched_switch_trace.init(tr);
> +       tracing_cmdline_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 91c699b..fbf05df 100644
> --- a/kernel/trace/trace_sched_switch.c
> +++ b/kernel/trace/trace_sched_switch.c
> @@ -152,6 +152,19 @@ void tracing_stop_cmdline_record(void)
>        tracing_stop_sched_switch();
>  }
>
> +/**
> + * tracing_cmdline_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)
> +{
> +       ctx_trace = tr;
> +}
> +
>  static void start_sched_trace(struct trace_array *tr)
>  {
>        sched_switch_reset(tr);
> @@ -197,7 +210,7 @@ static void sched_switch_trace_stop(struct trace_array *tr)
>        tracing_stop_sched_switch();
>  }
>
> -struct tracer sched_switch_trace __read_mostly =
> +static struct tracer sched_switch_trace __read_mostly =
>  {
>        .name           = "sched_switch",
>        .init           = sched_switch_trace_init,

Righ that's much proper.

BTW: I see that tracing_reset is called even on start and reset
callback by the tracers. Wouldn't it better to do it
only on reset? Perhaps it can even be done automatically by the
tracing Api itself when reset is called on
every tracers. Or perhaps there could be special cases I don't see
where one tracer wouldn't wish to erase is entries
on reset?

Acked-by: Frederic Weisbecker <fweisbec@...il.com>
--
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/