* [PATCH 0/2] perf tools: fix parsing with no sample_id_all bit set
@ 2013-09-04 20:18 Adrian Hunter
2013-09-04 20:18 ` [PATCH 1/2] perf tools: add test for parsing with no sample_id_all bit Adrian Hunter
2013-09-04 20:18 ` [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter
0 siblings, 2 replies; 6+ messages in thread
From: Adrian Hunter @ 2013-09-04 20:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker,
Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras,
Stephane Eranian
Hi
Here is a fix, with a test to illustrate the issue.
Adrian Hunter (2):
perf tools: add test for parsing with no sample_id_all bit
perf tools: fix parsing with no sample_id_all bit set
tools/perf/Makefile | 3 +-
tools/perf/tests/builtin-test.c | 4 ++
tools/perf/tests/parse-no-sample-id-all.c | 108 ++++++++++++++++++++++++++++++
tools/perf/tests/tests.h | 1 +
tools/perf/util/evlist.c | 9 ++-
5 files changed, 122 insertions(+), 3 deletions(-)
create mode 100644 tools/perf/tests/parse-no-sample-id-all.c
Regards
Adrian
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] perf tools: add test for parsing with no sample_id_all bit 2013-09-04 20:18 [PATCH 0/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter @ 2013-09-04 20:18 ` Adrian Hunter 2013-09-06 12:13 ` [tip:perf/urgent] perf tools: Add " tip-bot for Adrian Hunter 2013-09-04 20:18 ` [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter 1 sibling, 1 reply; 6+ messages in thread From: Adrian Hunter @ 2013-09-04 20:18 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras, Stephane Eranian Add a test for parsing a non-sample event when there is more than one selected event but no sample_id_all bit set. The test fails because of a bug in the evlist logic. That is fixed in a separate patch. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- tools/perf/Makefile | 3 +- tools/perf/tests/builtin-test.c | 4 ++ tools/perf/tests/parse-no-sample-id-all.c | 108 ++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 4 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 tools/perf/tests/parse-no-sample-id-all.c diff --git a/tools/perf/Makefile b/tools/perf/Makefile index c5dc1ad..3a0ff7f 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -394,6 +394,8 @@ ifeq ($(ARCH),x86) LIB_OBJS += $(OUTPUT)tests/perf-time-to-tsc.o endif LIB_OBJS += $(OUTPUT)tests/code-reading.o +LIB_OBJS += $(OUTPUT)tests/sample-parsing.o +LIB_OBJS += $(OUTPUT)tests/parse-no-sample-id-all.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o @@ -439,7 +441,6 @@ PERFLIBS = $(LIB_FILE) $(LIBLK) $(LIBTRACEEVENT) ifneq ($(OUTPUT),) CFLAGS += -I$(OUTPUT) endif -LIB_OBJS += $(OUTPUT)tests/sample-parsing.o ifdef NO_LIBELF EXTLIBS := $(filter-out -lelf,$(EXTLIBS)) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 8bbeba3..1e67437 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -112,6 +112,10 @@ static struct test { .func = test__keep_tracking, }, { + .desc = "Test parsing with no sample_id_all bit set", + .func = test__parse_no_sample_id_all, + }, + { .func = NULL, }, }; diff --git a/tools/perf/tests/parse-no-sample-id-all.c b/tools/perf/tests/parse-no-sample-id-all.c new file mode 100644 index 0000000..e117b6c --- /dev/null +++ b/tools/perf/tests/parse-no-sample-id-all.c @@ -0,0 +1,108 @@ +#include <sys/types.h> +#include <stddef.h> + +#include "tests.h" + +#include "event.h" +#include "evlist.h" +#include "header.h" +#include "util.h" + +static int process_event(struct perf_evlist **pevlist, union perf_event *event) +{ + struct perf_sample sample; + + if (event->header.type == PERF_RECORD_HEADER_ATTR) { + if (perf_event__process_attr(NULL, event, pevlist)) { + pr_debug("perf_event__process_attr failed\n"); + return -1; + } + return 0; + } + + if (event->header.type >= PERF_RECORD_USER_TYPE_START) + return -1; + + if (!*pevlist) + return -1; + + if (perf_evlist__parse_sample(*pevlist, event, &sample)) { + pr_debug("perf_evlist__parse_sample failed\n"); + return -1; + } + + return 0; +} + +static int process_events(union perf_event **events, size_t count) +{ + struct perf_evlist *evlist = NULL; + int err = 0; + size_t i; + + for (i = 0; i < count && !err; i++) + err = process_event(&evlist, events[i]); + + if (evlist) + perf_evlist__delete(evlist); + + return err; +} + +struct test_attr_event { + struct attr_event attr; + u64 id; +}; + +/** + * test__parse_no_sample_id_all - test parsing with no sample_id_all bit set. + * + * This function tests parsing data produced on kernel's that do not support the + * sample_id_all bit. Without the sample_id_all bit, non-sample events (such as + * mmap events) do not have an id sample appended, and consequently logic + * designed to determine the id will not work. That case happens when there is + * more than one selected event, so this test processes three events: 2 + * attributes representing the selected events and one mmap event. + * + * Return: %0 on success, %-1 if the test fails. + */ +int test__parse_no_sample_id_all(void) +{ + int err; + + struct test_attr_event event1 = { + .attr = { + .header = { + .type = PERF_RECORD_HEADER_ATTR, + .size = sizeof(struct test_attr_event), + }, + }, + .id = 1, + }; + struct test_attr_event event2 = { + .attr = { + .header = { + .type = PERF_RECORD_HEADER_ATTR, + .size = sizeof(struct test_attr_event), + }, + }, + .id = 2, + }; + struct mmap_event event3 = { + .header = { + .type = PERF_RECORD_MMAP, + .size = sizeof(struct mmap_event), + }, + }; + union perf_event *events[] = { + (union perf_event *)&event1, + (union perf_event *)&event2, + (union perf_event *)&event3, + }; + + err = process_events(events, ARRAY_SIZE(events)); + if (err) + return -1; + + return 0; +} diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index c048b58..e0ac713 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -39,5 +39,6 @@ int test__perf_time_to_tsc(void); int test__code_reading(void); int test__sample_parsing(void); int test__keep_tracking(void); +int test__parse_no_sample_id_all(void); #endif /* TESTS_H */ -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:perf/urgent] perf tools: Add test for parsing with no sample_id_all bit 2013-09-04 20:18 ` [PATCH 1/2] perf tools: add test for parsing with no sample_id_all bit Adrian Hunter @ 2013-09-06 12:13 ` tip-bot for Adrian Hunter 0 siblings, 0 replies; 6+ messages in thread From: tip-bot for Adrian Hunter @ 2013-09-06 12:13 UTC (permalink / raw) To: linux-tip-commits Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra, efault, namhyung, jolsa, fweisbec, adrian.hunter, dsahern, tglx Commit-ID: 53a277e5c9b55ed3e91345fb98e5f57d6d70efd6 Gitweb: http://git.kernel.org/tip/53a277e5c9b55ed3e91345fb98e5f57d6d70efd6 Author: Adrian Hunter <adrian.hunter@intel.com> AuthorDate: Wed, 4 Sep 2013 23:18:16 +0300 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Thu, 5 Sep 2013 16:17:46 -0300 perf tools: Add test for parsing with no sample_id_all bit Add a test for parsing a non-sample event when there is more than one selected event but no sample_id_all bit set. The test fails because of a bug in the evlist logic. That is fixed in a separate patch. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Cc: David Ahern <dsahern@gmail.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Mike Galbraith <efault@gmx.de> Cc: Namhyung Kim <namhyung@gmail.com> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/r/1378325897-3840-2-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/Makefile | 3 +- tools/perf/tests/builtin-test.c | 4 ++ tools/perf/tests/parse-no-sample-id-all.c | 108 ++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 4 files changed, 115 insertions(+), 1 deletion(-) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index c5dc1ad..3a0ff7f 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -394,6 +394,8 @@ ifeq ($(ARCH),x86) LIB_OBJS += $(OUTPUT)tests/perf-time-to-tsc.o endif LIB_OBJS += $(OUTPUT)tests/code-reading.o +LIB_OBJS += $(OUTPUT)tests/sample-parsing.o +LIB_OBJS += $(OUTPUT)tests/parse-no-sample-id-all.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o @@ -439,7 +441,6 @@ PERFLIBS = $(LIB_FILE) $(LIBLK) $(LIBTRACEEVENT) ifneq ($(OUTPUT),) CFLAGS += -I$(OUTPUT) endif -LIB_OBJS += $(OUTPUT)tests/sample-parsing.o ifdef NO_LIBELF EXTLIBS := $(filter-out -lelf,$(EXTLIBS)) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 8bbeba3..1e67437 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -112,6 +112,10 @@ static struct test { .func = test__keep_tracking, }, { + .desc = "Test parsing with no sample_id_all bit set", + .func = test__parse_no_sample_id_all, + }, + { .func = NULL, }, }; diff --git a/tools/perf/tests/parse-no-sample-id-all.c b/tools/perf/tests/parse-no-sample-id-all.c new file mode 100644 index 0000000..e117b6c --- /dev/null +++ b/tools/perf/tests/parse-no-sample-id-all.c @@ -0,0 +1,108 @@ +#include <sys/types.h> +#include <stddef.h> + +#include "tests.h" + +#include "event.h" +#include "evlist.h" +#include "header.h" +#include "util.h" + +static int process_event(struct perf_evlist **pevlist, union perf_event *event) +{ + struct perf_sample sample; + + if (event->header.type == PERF_RECORD_HEADER_ATTR) { + if (perf_event__process_attr(NULL, event, pevlist)) { + pr_debug("perf_event__process_attr failed\n"); + return -1; + } + return 0; + } + + if (event->header.type >= PERF_RECORD_USER_TYPE_START) + return -1; + + if (!*pevlist) + return -1; + + if (perf_evlist__parse_sample(*pevlist, event, &sample)) { + pr_debug("perf_evlist__parse_sample failed\n"); + return -1; + } + + return 0; +} + +static int process_events(union perf_event **events, size_t count) +{ + struct perf_evlist *evlist = NULL; + int err = 0; + size_t i; + + for (i = 0; i < count && !err; i++) + err = process_event(&evlist, events[i]); + + if (evlist) + perf_evlist__delete(evlist); + + return err; +} + +struct test_attr_event { + struct attr_event attr; + u64 id; +}; + +/** + * test__parse_no_sample_id_all - test parsing with no sample_id_all bit set. + * + * This function tests parsing data produced on kernel's that do not support the + * sample_id_all bit. Without the sample_id_all bit, non-sample events (such as + * mmap events) do not have an id sample appended, and consequently logic + * designed to determine the id will not work. That case happens when there is + * more than one selected event, so this test processes three events: 2 + * attributes representing the selected events and one mmap event. + * + * Return: %0 on success, %-1 if the test fails. + */ +int test__parse_no_sample_id_all(void) +{ + int err; + + struct test_attr_event event1 = { + .attr = { + .header = { + .type = PERF_RECORD_HEADER_ATTR, + .size = sizeof(struct test_attr_event), + }, + }, + .id = 1, + }; + struct test_attr_event event2 = { + .attr = { + .header = { + .type = PERF_RECORD_HEADER_ATTR, + .size = sizeof(struct test_attr_event), + }, + }, + .id = 2, + }; + struct mmap_event event3 = { + .header = { + .type = PERF_RECORD_MMAP, + .size = sizeof(struct mmap_event), + }, + }; + union perf_event *events[] = { + (union perf_event *)&event1, + (union perf_event *)&event2, + (union perf_event *)&event3, + }; + + err = process_events(events, ARRAY_SIZE(events)); + if (err) + return -1; + + return 0; +} diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index c048b58..e0ac713 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -39,5 +39,6 @@ int test__perf_time_to_tsc(void); int test__code_reading(void); int test__sample_parsing(void); int test__keep_tracking(void); +int test__parse_no_sample_id_all(void); #endif /* TESTS_H */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set 2013-09-04 20:18 [PATCH 0/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter 2013-09-04 20:18 ` [PATCH 1/2] perf tools: add test for parsing with no sample_id_all bit Adrian Hunter @ 2013-09-04 20:18 ` Adrian Hunter 2013-09-04 21:45 ` David Ahern 2013-09-06 12:14 ` [tip:perf/urgent] perf evlist: Fix " tip-bot for Adrian Hunter 1 sibling, 2 replies; 6+ messages in thread From: Adrian Hunter @ 2013-09-04 20:18 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra, linux-kernel, David Ahern, Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras, Stephane Eranian The perf_evlist__event2evsel() is changed to handle non-sample events (such as mmap events) that have no id sample appended i.e. when sample_id_all is not set. Note that such events have a fixed format, so that the selected event (evsel) they are associated with is immaterial. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- tools/perf/util/evlist.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index b8727ae..7101283 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -446,20 +446,25 @@ static int perf_evlist__event2id(struct perf_evlist *evlist, static struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist, union perf_event *event) { + struct perf_evsel *first = perf_evlist__first(evlist); struct hlist_head *head; struct perf_sample_id *sid; int hash; u64 id; if (evlist->nr_entries == 1) - return perf_evlist__first(evlist); + return first; + + if (!first->attr.sample_id_all && + event->header.type != PERF_RECORD_SAMPLE) + return first; if (perf_evlist__event2id(evlist, event, &id)) return NULL; /* Synthesized events have an id of zero */ if (!id) - return perf_evlist__first(evlist); + return first; hash = hash_64(id, PERF_EVLIST__HLIST_BITS); head = &evlist->heads[hash]; -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set 2013-09-04 20:18 ` [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter @ 2013-09-04 21:45 ` David Ahern 2013-09-06 12:14 ` [tip:perf/urgent] perf evlist: Fix " tip-bot for Adrian Hunter 1 sibling, 0 replies; 6+ messages in thread From: David Ahern @ 2013-09-04 21:45 UTC (permalink / raw) To: Adrian Hunter, Arnaldo Carvalho de Melo Cc: Peter Zijlstra, linux-kernel, Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim, Paul Mackerras, Stephane Eranian On 9/4/13 2:18 PM, Adrian Hunter wrote: > The perf_evlist__event2evsel() is changed to handle > non-sample events (such as mmap events) that have no > id sample appended i.e. when sample_id_all is not set. > > Note that such events have a fixed format, so that > the selected event (evsel) they are associated with > is immaterial. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Fixes the regression. Thanks for the fix. Tested-and-Acked-by: David Ahern <dsahern@gmail.com> Arnaldo: this needs to go in perf/urgent and pushed into 3.12. David ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/urgent] perf evlist: Fix parsing with no sample_id_all bit set 2013-09-04 20:18 ` [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter 2013-09-04 21:45 ` David Ahern @ 2013-09-06 12:14 ` tip-bot for Adrian Hunter 1 sibling, 0 replies; 6+ messages in thread From: tip-bot for Adrian Hunter @ 2013-09-06 12:14 UTC (permalink / raw) To: linux-tip-commits Cc: acme, linux-kernel, eranian, paulus, hpa, mingo, a.p.zijlstra, efault, namhyung, jolsa, fweisbec, adrian.hunter, dsahern, tglx Commit-ID: 98be6966ed7ed977881305aff5a1bfb305090f43 Gitweb: http://git.kernel.org/tip/98be6966ed7ed977881305aff5a1bfb305090f43 Author: Adrian Hunter <adrian.hunter@intel.com> AuthorDate: Wed, 4 Sep 2013 23:18:17 +0300 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Thu, 5 Sep 2013 16:18:08 -0300 perf evlist: Fix parsing with no sample_id_all bit set The perf_evlist__event2evsel() is changed to handle non-sample events (such as mmap events) that have no id sample appended i.e. when sample_id_all is not set. Note that such events have a fixed format, so that the selected event (evsel) they are associated with is immaterial. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Tested-by: David Ahern <dsahern@gmail.com> Acked-by: David Ahern <dsahern@gmail.com> Cc: David Ahern <dsahern@gmail.com> Cc: Frederic Weisbecker <fweisbec@gmail.com> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Mike Galbraith <efault@gmx.de> Cc: Namhyung Kim <namhyung@gmail.com> Cc: Paul Mackerras <paulus@samba.org> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Stephane Eranian <eranian@google.com> Link: http://lkml.kernel.org/r/1378325897-3840-3-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/evlist.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index b8727ae..7101283 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -446,20 +446,25 @@ static int perf_evlist__event2id(struct perf_evlist *evlist, static struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist, union perf_event *event) { + struct perf_evsel *first = perf_evlist__first(evlist); struct hlist_head *head; struct perf_sample_id *sid; int hash; u64 id; if (evlist->nr_entries == 1) - return perf_evlist__first(evlist); + return first; + + if (!first->attr.sample_id_all && + event->header.type != PERF_RECORD_SAMPLE) + return first; if (perf_evlist__event2id(evlist, event, &id)) return NULL; /* Synthesized events have an id of zero */ if (!id) - return perf_evlist__first(evlist); + return first; hash = hash_64(id, PERF_EVLIST__HLIST_BITS); head = &evlist->heads[hash]; ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-09-06 12:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-04 20:18 [PATCH 0/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter 2013-09-04 20:18 ` [PATCH 1/2] perf tools: add test for parsing with no sample_id_all bit Adrian Hunter 2013-09-06 12:13 ` [tip:perf/urgent] perf tools: Add " tip-bot for Adrian Hunter 2013-09-04 20:18 ` [PATCH 2/2] perf tools: fix parsing with no sample_id_all bit set Adrian Hunter 2013-09-04 21:45 ` David Ahern 2013-09-06 12:14 ` [tip:perf/urgent] perf evlist: Fix " tip-bot for Adrian Hunter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.