| 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: <20120806181941.GG21441@infradead.org>
Date: Mon, 6 Aug 2012 15:19:41 -0300
From: Arnaldo Carvalho de Melo <acme@...stprotocols.net>
To: Andrew Vagin <avagin@...nvz.org>
Cc: linux-kernel@...r.kernel.org,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Paul Mackerras <paulus@...ba.org>,
Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and
sched_switch events
Em Mon, Aug 06, 2012 at 02:01:58PM +0400, Andrew Vagin escreveu:
> You may want to know where and how long a task is sleeping. A callchain
> may be found in sched_switch and a time slice in stat_iowait, so I add
> handler in perf inject for merging this events.
>
> My code saves sched_switch event for each process and when it meets
> stat_iowait, it reports the sched_switch event, because this event
> contains a correct callchain. By another words it replaces all
> stat_iowait events on proper sched_switch events.
>
> Signed-off-by: Andrew Vagin <avagin@...nvz.org>
> ---
> tools/perf/builtin-inject.c | 96 ++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 91 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index d04b7a4..247f41c 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -13,6 +13,8 @@
> #include "util/debug.h"
>
> #include "util/parse-options.h"
> +#include "util/trace-event.h"
> +
>
> static char const *input_name = "-";
> static const char *output_name = "-";
> @@ -21,6 +23,9 @@ static int output;
> static u64 bytes_written;
>
> static bool inject_build_ids;
> +static bool inject_sched_stat;
> +
> +struct perf_session *session;
Why do we need to insert even more globals?
> static int perf_event__repipe_synth(struct perf_tool *tool __used,
> union perf_event *event,
> @@ -47,7 +52,7 @@ static int perf_event__repipe_synth(struct perf_tool *tool __used,
>
> static int perf_event__repipe_op2_synth(struct perf_tool *tool,
> union perf_event *event,
> - struct perf_session *session __used)
> + struct perf_session *s __used)
What is the point of the above hunk?
> {
> return perf_event__repipe_synth(tool, event, NULL);
> }
> @@ -59,7 +64,7 @@ static int perf_event__repipe_event_type_synth(struct perf_tool *tool,
> }
>
> static int perf_event__repipe_tracing_data_synth(union perf_event *event,
> - struct perf_session *session __used)
> + struct perf_session *s __used)
Ditto
> {
> return perf_event__repipe_synth(NULL, event, NULL);
> }
> @@ -119,12 +124,12 @@ static int perf_event__repipe_task(struct perf_tool *tool,
> }
>
> static int perf_event__repipe_tracing_data(union perf_event *event,
> - struct perf_session *session)
> + struct perf_session *s)
> {
> int err;
>
> perf_event__repipe_synth(NULL, event, NULL);
> - err = perf_event__process_tracing_data(event, session);
> + err = perf_event__process_tracing_data(event, s);
Ditto
>
> return err;
> }
> @@ -210,6 +215,83 @@ repipe:
> return 0;
> }
>
> +struct event_entry {
> + struct list_head list;
Is this really the head of a list? Or is this a node that will allow
event_entry instances to be added to a head of a list? If the former,
please rename this to "node".
> + u32 pid;
> + union perf_event event[0];
> +};
> +
> +static LIST_HEAD(samples);
> +
> +static int perf_event__sched_stat(struct perf_tool *tool,
> + union perf_event *event,
> + struct perf_sample *sample,
> + struct perf_evsel *evsel __used,
> + struct machine *machine)
> +{
> + int type;
> + struct event_format *e;
> + const char *evname = NULL;
> + uint32_t size;
> + struct event_entry *ent;
> + union perf_event *event_sw = NULL;
> + struct perf_sample sample_sw;
> + int sched_process_exit;
> +
> + size = event->header.size;
> +
> + type = trace_parse_common_type(session->pevent, sample->raw_data);
> + e = pevent_find_event(session->pevent, type);
> + if (e)
> + evname = e->name;
> +
> + sched_process_exit = !strcmp(evname, "sched_process_exit");
> +
> + if (!strcmp(evname, "sched_switch") || sched_process_exit) {
extra space
> + list_for_each_entry(ent, &samples, list)
> + if (sample->pid == ent->pid)
> + break;
> +
> + if (&ent->list != &samples) {
> + list_del(&ent->list);
> + free(ent);
> + }
> +
> + if (sched_process_exit)
> + return 0;
> +
> + ent = malloc(size + sizeof(struct event_entry));
Can malloc fail?
> + ent->pid = sample->pid;
> + memcpy(&ent->event, event, size);
> + list_add(&ent->list, &samples);
> + return 0;
> +
> + } else if (!strncmp(evname, "sched_stat_", 11)) {
> + u32 pid;
> +
> + pid = raw_field_value(e, "pid", sample->raw_data);
> +
> + list_for_each_entry(ent, &samples, list) {
> + if (pid == ent->pid)
> + break;
> + }
> +
> + if (&ent->list == &samples)
> + return 0;
> +
> + event_sw = &ent->event[0];
> + perf_session__parse_sample(session, event_sw, &sample_sw);
> + sample_sw.period = sample->period;
> + sample_sw.time = sample->time;
> + perf_session__synthesize_sample(session, event_sw, &sample_sw);
Please use perf_evsel__parse_sample, recently introduced.
> + perf_event__repipe(tool, event_sw, &sample_sw, machine);
> + return 0;
> + }
> +
> + perf_event__repipe(tool, event, sample, machine);
> +
> + return 0;
> +}
> struct perf_tool perf_inject = {
> .sample = perf_event__repipe_sample,
> .mmap = perf_event__repipe,
> @@ -235,7 +317,6 @@ static void sig_handler(int sig __attribute__((__unused__)))
>
> static int __cmd_inject(void)
> {
> - struct perf_session *session;
> int ret = -EINVAL;
>
> signal(SIGINT, sig_handler);
> @@ -245,6 +326,9 @@ static int __cmd_inject(void)
> perf_inject.mmap = perf_event__repipe_mmap;
> perf_inject.fork = perf_event__repipe_task;
> perf_inject.tracing_data = perf_event__repipe_tracing_data;
> + } else if (inject_sched_stat) {
> + perf_inject.sample = perf_event__sched_stat;
> + perf_inject.ordered_samples = true;
> }
>
> session = perf_session__new(input_name, O_RDONLY, false, true, &perf_inject);
> @@ -272,6 +356,8 @@ static const char * const report_usage[] = {
> static const struct option options[] = {
> OPT_BOOLEAN('b', "build-ids", &inject_build_ids,
> "Inject build-ids into the output stream"),
> + OPT_BOOLEAN('s', "sched-stat", &inject_sched_stat,
> + "Set source call-chains for sched:shed-stat-*"),
You're adding an option, needs to be documented on
perf/tools/Documentation/
> OPT_STRING('i', "input", &input_name, "file",
> "input file name"),
> OPT_STRING('o', "output", &output_name, "file",
> --
> 1.7.1
--
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