2012-08-06 10:02:46

by Andrei Vagin

[permalink] [raw]
Subject: [PATCH 0/3] perf: Teach perf tool to profile sleep times

This functionality helps to analize where a task sleeps or waits locks.
This feature can help to investigate a scalability problems.

The main idea is that we can combine sched_switch and sched_stat_sleep events.
sched_switch contains a callchain, when a task starts sleeping.
sched_stat_sleep contains a time period for which a task slept.

This series teaches "perf inject" to combine this events.

All kernel related patches were committed committed in 3.6-rc1.

Here is an example of a report:
$ cat ~/foo.c
....
for (i = 0; i < 10; i++) {
ts1.tv_sec = 0;
ts1.tv_nsec = 10000000;
nanosleep(&ts1, NULL);

tv1.tv_sec = 0;
tv1.tv_usec = 40000;
select(0, NULL, NULL, NULL,&tv1);
}
...

$ ./perf record -e sched:sched_stat_sleep -e sched:sched_switch \
-e sched:sched_process_exit -gP -o ~/perf.data.raw ~/foo
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB /root/perf.data.raw (~661 samples) ]
$ ./perf inject -v -s -i ~/perf.data.raw -o ~/perf.data
$ ./perf report -i ~/perf.data
# Samples: 40 of event 'sched:sched_switch'
# Event count (approx.): 1005527702
#
# Overhead Command Shared Object Symbol
# ........ ....... ................. ..............
#
100.00% foo [kernel.kallsyms] [k] __schedule
|
--- __schedule
schedule
|
|--79.81%-- schedule_hrtimeout_range_clock
| schedule_hrtimeout_range
| poll_schedule_timeout
| do_select
| core_sys_select
| sys_select
| system_call_fastpath
| __select
| __libc_start_main
|
--20.19%-- do_nanosleep
hrtimer_nanosleep
sys_nanosleep
system_call_fastpath
__GI___libc_nanosleep
__libc_start_main

Andrew Vagin (3):
perf: teach "perf inject" to work with files
perf: teach perf inject to merge sched_stat_* and sched_switch events
perf: mark a dso if it's used

tools/perf/builtin-inject.c | 139 ++++++++++++++++++++++++++++++++++++++++---
tools/perf/util/build-id.c | 2 +-
tools/perf/util/build-id.h | 5 ++
3 files changed, 137 insertions(+), 9 deletions(-)


2012-08-06 10:02:09

by Andrei Vagin

[permalink] [raw]
Subject: [PATCH 3/3] perf: mark a dso if it's used

Otherwise they will be not written in an output file.

Signed-off-by: Andrew Vagin <[email protected]>
---
tools/perf/builtin-inject.c | 11 +++++++++--
tools/perf/util/build-id.c | 2 +-
tools/perf/util/build-id.h | 5 +++++
3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 247f41c..cb2fd77 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -14,6 +14,7 @@

#include "util/parse-options.h"
#include "util/trace-event.h"
+#include "util/build-id.h"


static char const *input_name = "-";
@@ -281,13 +282,17 @@ static int perf_event__sched_stat(struct perf_tool *tool,

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);
+
+ build_id__mark_dso_hit(tool, event_sw, &sample_sw, evsel, machine);
perf_event__repipe(tool, event_sw, &sample_sw, machine);
return 0;
}

+ build_id__mark_dso_hit(tool, event, sample, evsel, machine);
perf_event__repipe(tool, event, sample, machine);

return 0;
@@ -321,12 +326,14 @@ static int __cmd_inject(void)

signal(SIGINT, sig_handler);

- if (inject_build_ids) {
+ if (inject_build_ids || inject_sched_stat) {
perf_inject.sample = perf_event__inject_buildid;
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) {
+ }
+
+ if (inject_sched_stat) {
perf_inject.sample = perf_event__sched_stat;
perf_inject.ordered_samples = true;
}
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index fd9a594..9ce0e11 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -16,7 +16,7 @@
#include "session.h"
#include "tool.h"

-static int build_id__mark_dso_hit(struct perf_tool *tool __used,
+int build_id__mark_dso_hit(struct perf_tool *tool __used,
union perf_event *event,
struct perf_sample *sample __used,
struct perf_evsel *evsel __used,
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index a993ba8..032a968 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -7,4 +7,9 @@ extern struct perf_tool build_id__mark_dso_hit_ops;

char *dso__build_id_filename(struct dso *self, char *bf, size_t size);

+int build_id__mark_dso_hit(struct perf_tool *tool __used,
+ union perf_event *event,
+ struct perf_sample *sample __used,
+ struct perf_evsel *evsel __used,
+ struct machine *machine);
#endif
--
1.7.1

2012-08-06 10:02:12

by Andrei Vagin

[permalink] [raw]
Subject: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events

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 <[email protected]>
---
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;

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)
{
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)
{
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);

return err;
}
@@ -210,6 +215,83 @@ repipe:
return 0;
}

+struct event_entry {
+ struct list_head list;
+ 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) {
+ 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));
+ 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);
+ 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-*"),
OPT_STRING('i', "input", &input_name, "file",
"input file name"),
OPT_STRING('o', "output", &output_name, "file",
--
1.7.1

2012-08-06 10:02:10

by Andrei Vagin

[permalink] [raw]
Subject: [PATCH 1/3] perf: teach "perf inject" to work with files

Before this patch "perf inject" can only handle data from pipe.

I want to use "perf inject" for reworking events. Look at my following patch.

Signed-off-by: Andrew Vagin <[email protected]>
---
tools/perf/builtin-inject.c | 33 +++++++++++++++++++++++++++++++--
1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 3beab48..d04b7a4 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -14,7 +14,12 @@

#include "util/parse-options.h"

-static char const *input_name = "-";
+static char const *input_name = "-";
+static const char *output_name = "-";
+static int pipe_output;
+static int output;
+static u64 bytes_written;
+
static bool inject_build_ids;

static int perf_event__repipe_synth(struct perf_tool *tool __used,
@@ -27,12 +32,14 @@ static int perf_event__repipe_synth(struct perf_tool *tool __used,
size = event->header.size;

while (size) {
- int ret = write(STDOUT_FILENO, buf, size);
+ int ret = write(output, buf, size);
if (ret < 0)
return -errno;

size -= ret;
buf += ret;
+
+ bytes_written += ret;
}

return 0;
@@ -244,8 +251,14 @@ static int __cmd_inject(void)
if (session == NULL)
return -ENOMEM;

+ if (!pipe_output)
+ lseek(output, session->header.data_offset, SEEK_SET);
ret = perf_session__process_events(session, &perf_inject);

+ if (!pipe_output) {
+ session->header.data_size = bytes_written;
+ perf_session__write_header(session, session->evlist, output, true);
+ }
perf_session__delete(session);

return ret;
@@ -259,6 +272,10 @@ 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_STRING('i', "input", &input_name, "file",
+ "input file name"),
+ OPT_STRING('o', "output", &output_name, "file",
+ "output file name"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show build ids, etc)"),
OPT_END()
@@ -274,6 +291,18 @@ int cmd_inject(int argc, const char **argv, const char *prefix __used)
if (argc)
usage_with_options(report_usage, options);

+ if (!strcmp(output_name, "-")) {
+ pipe_output = 1;
+ output = STDOUT_FILENO;
+ } else {
+ output = open(output_name, O_CREAT | O_WRONLY | O_TRUNC,
+ S_IRUSR | S_IWUSR);
+ if (output < 0) {
+ perror("failed to create output file");
+ exit(-1);
+ }
+ }
+
if (symbol__init() < 0)
return -1;

--
1.7.1

2012-08-06 18:12:10

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 1/3] perf: teach "perf inject" to work with files

Em Mon, Aug 06, 2012 at 02:01:57PM +0400, Andrew Vagin escreveu:
> Before this patch "perf inject" can only handle data from pipe.
>
> I want to use "perf inject" for reworking events. Look at my following patch.
>
> Signed-off-by: Andrew Vagin <[email protected]>

Patches that add options to commands should include updates to the docs
(tools/perf/Documentation/).

Also something I saw was the const * char versus const char * stuff for
the foo_file variables, please make them consistent.

- Arnaldo

2012-08-06 18:14:44

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf: mark a dso if it's used

Em Mon, Aug 06, 2012 at 02:01:59PM +0400, Andrew Vagin escreveu:
> - if (inject_build_ids) {
> + if (inject_build_ids || inject_sched_stat) {
> perf_inject.sample = perf_event__inject_buildid;
> 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) {
> + }
> +
> + if (inject_sched_stat) {
> perf_inject.sample = perf_event__sched_stat;
> perf_inject.ordered_samples = true;
> }

Huh? so if inject_sched_stat is true we will first set
perf_inject.sample to something, then to another?

- Arnaldo

2012-08-06 18:19:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
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 <[email protected]>
> ---
> 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

2012-08-06 19:43:08

by Andrei Vagin

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events

Hello Arnaldo,

Thanks for comments, I will correct them. I need a bit more details
about two of them.

2012/8/6 Arnaldo Carvalho de Melo <[email protected]>:
>> @@ -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;

perf_event__sched_stat (perf_inject.sample) uses "session" for getting
an event name. I don't know how to get it by another way

>
> 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?

"session" is global, for this reason I renamed all arguments.

p.s. Arnaldo, sorry for the personal message with the same content.
It's my mistake.

2012-08-06 19:50:56

by Andrei Vagin

[permalink] [raw]
Subject: Re: [PATCH 3/3] perf: mark a dso if it's used

2012/8/6 Arnaldo Carvalho de Melo <[email protected]>:
> Em Mon, Aug 06, 2012 at 02:01:59PM +0400, Andrew Vagin escreveu:
>> - if (inject_build_ids) {
>> + if (inject_build_ids || inject_sched_stat) {
>> perf_inject.sample = perf_event__inject_buildid;
>> 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) {
>> + }
>> +
>> + if (inject_sched_stat) {
>> perf_inject.sample = perf_event__sched_stat;
>> perf_inject.ordered_samples = true;
>> }
>
> Huh? so if inject_sched_stat is true we will first set
> perf_inject.sample to something, then to another?

Yes, we will. I though that it will be better then this:

if (inject_build_ids || inject_sched_stat) {
perf_inject.mmap = perf_event__repipe_mmap;
perf_inject.fork = perf_event__repipe_task;
perf_inject.tracing_data = perf_event__repipe_tracing_data;
}

if (inject_build_ids) {
perf_inject.sample = perf_event__inject_buildid;
} else if (inject_sched_stat) {
perf_inject.sample = perf_event__sched_stat;
perf_inject.ordered_samples = true;
}

2012-08-06 22:00:09

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events

Em Mon, Aug 06, 2012 at 11:43:04PM +0400, Andrey Wagin escreveu:
> 2012/8/6 Arnaldo Carvalho de Melo <[email protected]>:
> >> +struct perf_session *session;
>
> perf_event__sched_stat (perf_inject.sample) uses "session" for getting
> an event name. I don't know how to get it by another way

Can you try with the attached patch? We already lookup the event_format
entries when we read the perf.data header so that we can cache
evsel->name, we might as well cache the event_format in
evsel->tp_format, so that tools don't have to relookup this for each
sample.

It would look like:

static int perf_event__sched_stat(struct perf_tool *tool,
union perf_event *event,
struct perf_sample *sample,
struct perf_evsel *evsel,
struct machine *machine)
{
int type;
struct event_format *e = evsel->tp_format;
const char *evname = e->name;

- Arnaldo


Attachments:
(No filename) (885.00 B)
a.patch (660.00 B)
Download all attachments

2012-08-06 23:31:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events

Em Mon, Aug 06, 2012 at 07:00:00PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 06, 2012 at 11:43:04PM +0400, Andrey Wagin escreveu:
> > 2012/8/6 Arnaldo Carvalho de Melo <[email protected]>:
> > >> +struct perf_session *session;

> > perf_event__sched_stat (perf_inject.sample) uses "session" for getting
> > an event name. I don't know how to get it by another way
>
> Can you try with the attached patch? We already lookup the event_format
> entries when we read the perf.data header so that we can cache
> evsel->name, we might as well cache the event_format in
> evsel->tp_format, so that tools don't have to relookup this for each
> sample.

Attached goes a more complete patch that removes the pevent_find_event
calls from several tools, David, could you give it some testing?

- Arnaldo


Attachments:
(No filename) (814.00 B)
perf_evsel_tp_format.patch (15.99 kB)
Download all attachments

2012-08-13 09:17:55

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/3] perf: teach perf inject to merge sched_stat_* and sched_switch events


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> static int process_sample_event(struct perf_tool *tool __used,
> union perf_event *event,
> struct perf_sample *sample,
> struct perf_evsel *evsel,
> struct machine *machine)

Just saw this 5-parameter function signature fly by: as a
separate clean-up it would be really neat to stick most of these
into an intuitively named helper structure or so.

struct event_context ectx?

That would make extension of the context easier as well in the
future. Or so.

Thanks,

Ingo