Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1209854 > unrolled thread

[PATCH 3/3] perf tools: Fix buildid processing

Started byAdrian Hunter <adrian.hunter@intel.com>
First post2015-08-19 16:40 +0200
Last post2015-08-20 12:10 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 3/3] perf tools: Fix buildid processing Adrian Hunter <adrian.hunter@intel.com> - 2015-08-19 16:40 +0200
    Re: [PATCH 3/3] perf tools: Fix buildid processing Adrian Hunter <adrian.hunter@intel.com> - 2015-08-19 17:30 +0200
      Re: [PATCH 3/3] perf tools: Fix buildid processing Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-08-19 17:50 +0200
    [tip:perf/core] perf tools: Fix buildid processing tip-bot for Adrian Hunter <tipbot@zytor.com> - 2015-08-20 12:10 +0200

#1209854 — [PATCH 3/3] perf tools: Fix buildid processing

FromAdrian Hunter <adrian.hunter@intel.com>
Date2015-08-19 16:40 +0200
Subject[PATCH 3/3] perf tools: Fix buildid processing
Message-ID<pZcGZ-eH-15@gated-at.bofh.it>
After recording, 'perf record' post-processes the data to
determine which buildids are needed. That processing must
process the data in time order, if possible, because
otherwise dependent events, like forks and mmaps, will
not make sense.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index de165a1b9240..f36b88938499 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,6 +500,15 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		return -1;
 	}
 
+	/*
+	 * Normally perf_session__new would do this, but it doesn't have the
+	 * evlist.
+	 */
+	if (rec->tool.ordered_events && !perf_evlist__sample_id_all(rec->evlist)) {
+		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
+		rec->tool.ordered_events = false;
+	}
+
 	fd = perf_data_file__fd(file);
 	rec->session = session;
 
@@ -965,9 +974,11 @@ static struct record record = {
 	.tool = {
 		.sample		= process_sample_event,
 		.fork		= perf_event__process_fork,
+		.exit		= perf_event__process_exit,
 		.comm		= perf_event__process_comm,
 		.mmap		= perf_event__process_mmap,
 		.mmap2		= perf_event__process_mmap2,
+		.ordered_events	= true,
 	},
 };
 
-- 
1.9.1

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

[toc] | [next] | [standalone]


#1209899

FromAdrian Hunter <adrian.hunter@intel.com>
Date2015-08-19 17:30 +0200
Message-ID<pZdto-1si-15@gated-at.bofh.it>
In reply to#1209854
On 19/08/2015 5:29 p.m., Adrian Hunter wrote:
> After recording, 'perf record' post-processes the data to
> determine which buildids are needed. That processing must
> process the data in time order, if possible, because
> otherwise dependent events, like forks and mmaps, will
> not make sense.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>   tools/perf/builtin-record.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index de165a1b9240..f36b88938499 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -500,6 +500,15 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>   		return -1;
>   	}
>
> +	/*
> +	 * Normally perf_session__new would do this, but it doesn't have the
> +	 * evlist.
> +	 */
> +	if (rec->tool.ordered_events && !perf_evlist__sample_id_all(rec->evlist)) {
> +		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
> +		rec->tool.ordered_events = false;
> +	}
> +

In fact this chunk is misplaced, it needs to be below the call to record__open()
after which the attributes are correctly configured.  And dump_printf() is no good
for 'perf record', pr_debug would be better. Don't have time to do that right now,
I'll do it tommorow unless Arnaldo fixes it up.

>   	fd = perf_data_file__fd(file);
>   	rec->session = session;
>
> @@ -965,9 +974,11 @@ static struct record record = {
>   	.tool = {
>   		.sample		= process_sample_event,
>   		.fork		= perf_event__process_fork,
> +		.exit		= perf_event__process_exit,
>   		.comm		= perf_event__process_comm,
>   		.mmap		= perf_event__process_mmap,
>   		.mmap2		= perf_event__process_mmap2,
> +		.ordered_events	= true,
>   	},
>   };
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1209922

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-08-19 17:50 +0200
Message-ID<pZdMK-1OT-29@gated-at.bofh.it>
In reply to#1209899
Em Wed, Aug 19, 2015 at 06:23:51PM +0300, Adrian Hunter escreveu:
> On 19/08/2015 5:29 p.m., Adrian Hunter wrote:
> >After recording, 'perf record' post-processes the data to
> >determine which buildids are needed. That processing must
> >process the data in time order, if possible, because
> >otherwise dependent events, like forks and mmaps, will
> >not make sense.
> >
> >Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >---
> >  tools/perf/builtin-record.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> >diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> >index de165a1b9240..f36b88938499 100644
> >--- a/tools/perf/builtin-record.c
> >+++ b/tools/perf/builtin-record.c
> >@@ -500,6 +500,15 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >  		return -1;
> >  	}
> >
> >+	/*
> >+	 * Normally perf_session__new would do this, but it doesn't have the
> >+	 * evlist.
> >+	 */
> >+	if (rec->tool.ordered_events && !perf_evlist__sample_id_all(rec->evlist)) {
> >+		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
> >+		rec->tool.ordered_events = false;
> >+	}
> >+
> 
> In fact this chunk is misplaced, it needs to be below the call to record__open()
> after which the attributes are correctly configured.  And dump_printf() is no good
> for 'perf record', pr_debug would be better. Don't have time to do that right now,
> I'll do it tommorow unless Arnaldo fixes it up.

I'll fix it up, thanks!

- Arnaldo
 
> >  	fd = perf_data_file__fd(file);
> >  	rec->session = session;
> >
> >@@ -965,9 +974,11 @@ static struct record record = {
> >  	.tool = {
> >  		.sample		= process_sample_event,
> >  		.fork		= perf_event__process_fork,
> >+		.exit		= perf_event__process_exit,
> >  		.comm		= perf_event__process_comm,
> >  		.mmap		= perf_event__process_mmap,
> >  		.mmap2		= perf_event__process_mmap2,
> >+		.ordered_events	= true,
> >  	},
> >  };
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210385 — [tip:perf/core] perf tools: Fix buildid processing

Fromtip-bot for Adrian Hunter <tipbot@zytor.com>
Date2015-08-20 12:10 +0200
Subject[tip:perf/core] perf tools: Fix buildid processing
Message-ID<pZuXh-1Hx-45@gated-at.bofh.it>
In reply to#1209854
Commit-ID:  cca8482c0651cea97aade58cc22109ce9fffbfa2
Gitweb:     http://git.kernel.org/tip/cca8482c0651cea97aade58cc22109ce9fffbfa2
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Wed, 19 Aug 2015 17:29:21 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 19 Aug 2015 14:15:26 -0300

perf tools: Fix buildid processing

After recording, 'perf record' post-processes the data to determine
which buildids are needed.

That processing must process the data in time order, if possible,
because otherwise dependent events, like forks and mmaps, will not make
sense.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/1439994561-27436-4-git-send-email-adrian.hunter@intel.com
[ Moved the sample_id_add to after trying to open the events, use pr_warning ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index de165a1..20b56eb 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -521,6 +521,15 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		goto out_child;
 	}
 
+	/*
+	 * Normally perf_session__new would do this, but it doesn't have the
+	 * evlist.
+	 */
+	if (rec->tool.ordered_events && !perf_evlist__sample_id_all(rec->evlist)) {
+		pr_warning("WARNING: No sample_id_all support, falling back to unordered processing\n");
+		rec->tool.ordered_events = false;
+	}
+
 	if (!rec->evlist->nr_groups)
 		perf_header__clear_feat(&session->header, HEADER_GROUP_DESC);
 
@@ -965,9 +974,11 @@ static struct record record = {
 	.tool = {
 		.sample		= process_sample_event,
 		.fork		= perf_event__process_fork,
+		.exit		= perf_event__process_exit,
 		.comm		= perf_event__process_comm,
 		.mmap		= perf_event__process_mmap,
 		.mmap2		= perf_event__process_mmap2,
+		.ordered_events	= true,
 	},
 };
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web