* [PATCH 1/2] perf: Reset hwc->last_period on sw clock events
@ 2013-03-18 2:41 Namhyung Kim
2013-03-18 2:41 ` [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency Namhyung Kim
2013-03-18 11:06 ` [tip:perf/urgent] perf: Reset hwc->last_period on sw clock events tip-bot for Namhyung Kim
0 siblings, 2 replies; 6+ messages in thread
From: Namhyung Kim @ 2013-03-18 2:41 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, LKML, Namhyung Kim
From: Namhyung Kim <namhyung.kim@lge.com>
When cpu/task clock events are initialized, their sampling frequencies
are converted to have a fixed value. However it missed to update the
hwc->last_period which was set to 1 for initial sampling frequency
calibration.
Because this hwc->last_period value is used as a period in perf_swevent_
hrtime(), every recorded sample will have an incorrected period of 1.
$ perf record -e task-clock noploop 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.158 MB perf.data (~6919 samples) ]
$ perf report -n --show-total-period --stdio
# Samples: 4K of event 'task-clock'
# Event count (approx.): 4000
#
# Overhead Samples Period Command Shared Object Symbol
# ........ ............ ............ ....... ............. ..................
#
99.95% 3998 3998 noploop noploop [.] main
0.03% 1 1 noploop libc-2.15.so [.] init_cacheinfo
0.03% 1 1 noploop ld-2.15.so [.] open_verify
Note that it doesn't affect the non-sampling event so that the perf
stat still gets correct value with or without this patch.
$ perf stat -e task-clock noploop 1
Performance counter stats for 'noploop 1':
1000.272525 task-clock # 1.000 CPUs utilized
1.000560605 seconds time elapsed
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
kernel/events/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c38feef0d683..ac418e88b8a9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5659,6 +5659,7 @@ static void perf_swevent_init_hrtimer(struct perf_event *event)
event->attr.sample_period = NSEC_PER_SEC / freq;
hwc->sample_period = event->attr.sample_period;
local64_set(&hwc->period_left, hwc->sample_period);
+ hwc->last_period = hwc->sample_period;
event->attr.freq = 0;
}
}
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency 2013-03-18 2:41 [PATCH 1/2] perf: Reset hwc->last_period on sw clock events Namhyung Kim @ 2013-03-18 2:41 ` Namhyung Kim 2013-03-18 14:42 ` Arnaldo Carvalho de Melo 2013-03-21 11:51 ` [tip:perf/core] " tip-bot for Namhyung Kim 2013-03-18 11:06 ` [tip:perf/urgent] perf: Reset hwc->last_period on sw clock events tip-bot for Namhyung Kim 1 sibling, 2 replies; 6+ messages in thread From: Namhyung Kim @ 2013-03-18 2:41 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar Cc: Arnaldo Carvalho de Melo, Jiri Olsa, LKML, Namhyung Kim From: Namhyung Kim <namhyung.kim@lge.com> This test case checks frequency conversion of hrtimer-based software clock events (cpu-clock, task-clock) have valid (non-1) periods. Cc: Jiri Olsa <jolsa@redhat.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/Makefile | 1 + tools/perf/tests/builtin-test.c | 4 ++ tools/perf/tests/sw-clock.c | 118 ++++++++++++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 4 files changed, 124 insertions(+) create mode 100644 tools/perf/tests/sw-clock.c diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 8e1bba35a1ee..0230b75ed7f9 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -514,6 +514,7 @@ LIB_OBJS += $(OUTPUT)tests/python-use.o LIB_OBJS += $(OUTPUT)tests/bp_signal.o LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o LIB_OBJS += $(OUTPUT)tests/task-exit.o +LIB_OBJS += $(OUTPUT)tests/sw-clock.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 9b5c70a180d2..0918ada4cc41 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -90,6 +90,10 @@ static struct test { .func = test__task_exit, }, { + .desc = "Test software clock events have valid period values", + .func = test__sw_clock_freq, + }, + { .func = NULL, }, }; diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c new file mode 100644 index 000000000000..a45d72618096 --- /dev/null +++ b/tools/perf/tests/sw-clock.c @@ -0,0 +1,118 @@ +#include <unistd.h> +#include <stdlib.h> +#include <signal.h> +#include <sys/mman.h> + +#include "tests.h" +#include "util/evsel.h" +#include "util/evlist.h" +#include "util/cpumap.h" +#include "util/thread_map.h" + +#define NR_LOOPS 1000000 + +/* + * This test will open software clock events (cpu-clock, task-clock) + * then check their frequency -> period conversion has no artifact of + * setting period to 1 forcefully. + */ +static int __test__sw_clock_freq(enum perf_sw_ids clock_id) +{ + int i, err = -1; + volatile int tmp = 0; + u64 total_periods = 0; + int nr_samples = 0; + union perf_event *event; + struct perf_evsel *evsel; + struct perf_evlist *evlist; + struct perf_event_attr attr = { + .type = PERF_TYPE_SOFTWARE, + .config = clock_id, + .sample_type = PERF_SAMPLE_PERIOD, + .exclude_kernel = 1, + .disabled = 1, + .freq = 1, + .sample_freq = 10000, + }; + + evlist = perf_evlist__new(); + if (evlist == NULL) { + pr_debug("perf_evlist__new\n"); + return -1; + } + + evsel = perf_evsel__new(&attr, 0); + if (evsel == NULL) { + pr_debug("perf_evsel__new\n"); + goto out_free_evlist; + } + perf_evlist__add(evlist, evsel); + + evlist->cpus = cpu_map__dummy_new(); + evlist->threads = thread_map__new_by_tid(getpid()); + if (!evlist->cpus || !evlist->threads) { + err = -ENOMEM; + pr_debug("Not enough memory to create thread/cpu maps\n"); + goto out_delete_maps; + } + + perf_evlist__open(evlist); + + err = perf_evlist__mmap(evlist, 128, true); + if (err < 0) { + pr_debug("failed to mmap event: %d (%s)\n", errno, + strerror(errno)); + goto out_close_evlist; + } + + perf_evlist__enable(evlist); + + /* collect samples */ + for (i = 0; i < NR_LOOPS; i++) + tmp++; + + perf_evlist__disable(evlist); + + while ((event = perf_evlist__mmap_read(evlist, 0)) != NULL) { + struct perf_sample sample; + + if (event->header.type != PERF_RECORD_SAMPLE) + continue; + + err = perf_evlist__parse_sample(evlist, event, &sample); + if (err < 0) { + pr_debug("Error during parse sample\n"); + goto out_unmap_evlist; + } + + total_periods += sample.period; + nr_samples++; + } + + if ((u64) nr_samples == total_periods) { + pr_debug("All (%d) samples have period value of 1!\n", + nr_samples); + err = -1; + } + +out_unmap_evlist: + perf_evlist__munmap(evlist); +out_close_evlist: + perf_evlist__close(evlist); +out_delete_maps: + perf_evlist__delete_maps(evlist); +out_free_evlist: + perf_evlist__delete(evlist); + return err; +} + +int test__sw_clock_freq(void) +{ + int ret; + + ret = __test__sw_clock_freq(PERF_COUNT_SW_CPU_CLOCK); + if (!ret) + ret = __test__sw_clock_freq(PERF_COUNT_SW_TASK_CLOCK); + + return ret; +} diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index b33b3286ad6e..dd7feae2d37b 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -26,5 +26,6 @@ int test__python_use(void); int test__bp_signal(void); int test__bp_signal_overflow(void); int test__task_exit(void); +int test__sw_clock_freq(void); #endif /* TESTS_H */ -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency 2013-03-18 2:41 ` [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency Namhyung Kim @ 2013-03-18 14:42 ` Arnaldo Carvalho de Melo 2013-03-19 5:10 ` Namhyung Kim 2013-03-21 11:51 ` [tip:perf/core] " tip-bot for Namhyung Kim 1 sibling, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2013-03-18 14:42 UTC (permalink / raw) To: Namhyung Kim; +Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, LKML, Namhyung Kim Em Mon, Mar 18, 2013 at 11:41:47AM +0900, Namhyung Kim escreveu: > +static int __test__sw_clock_freq(enum perf_sw_ids clock_id) > +{ > + int i, err = -1; > + volatile int tmp = 0; > + u64 total_periods = 0; > + int nr_samples = 0; > + union perf_event *event; > + struct perf_evsel *evsel; > + struct perf_evlist *evlist; > + struct perf_event_attr attr = { > + .type = PERF_TYPE_SOFTWARE, > + .config = clock_id, > + .sample_type = PERF_SAMPLE_PERIOD, > + .exclude_kernel = 1, > + .disabled = 1, > + .freq = 1, > + .sample_freq = 10000, > + }; In some compilers we get: tests/sw-clock.c: In function ‘__test__sw_clock_freq’: tests/sw-clock.c:35: error: unknown field ‘sample_freq’ specified in initializer So I'm moving the initialization to outside the struct named initialization block, i.e.: @@ -32,9 +32,10 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id) .exclude_kernel = 1, .disabled = 1, .freq = 1, - .sample_freq = 10000, }; + attr.sample_freq = 10000; + - Arnaldo ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency 2013-03-18 14:42 ` Arnaldo Carvalho de Melo @ 2013-03-19 5:10 ` Namhyung Kim 0 siblings, 0 replies; 6+ messages in thread From: Namhyung Kim @ 2013-03-19 5:10 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Peter Zijlstra, Ingo Molnar, Jiri Olsa, LKML, Namhyung Kim Hi Arnaldo, On Mon, 18 Mar 2013 11:42:43 -0300, Arnaldo Carvalho de Melo wrote: > Em Mon, Mar 18, 2013 at 11:41:47AM +0900, Namhyung Kim escreveu: >> +static int __test__sw_clock_freq(enum perf_sw_ids clock_id) >> +{ >> + int i, err = -1; >> + volatile int tmp = 0; >> + u64 total_periods = 0; >> + int nr_samples = 0; >> + union perf_event *event; >> + struct perf_evsel *evsel; >> + struct perf_evlist *evlist; >> + struct perf_event_attr attr = { >> + .type = PERF_TYPE_SOFTWARE, >> + .config = clock_id, >> + .sample_type = PERF_SAMPLE_PERIOD, >> + .exclude_kernel = 1, >> + .disabled = 1, >> + .freq = 1, >> + .sample_freq = 10000, >> + }; > > In some compilers we get: > > tests/sw-clock.c: In function ‘__test__sw_clock_freq’: > tests/sw-clock.c:35: error: unknown field ‘sample_freq’ specified in initializer > > So I'm moving the initialization to outside the struct named initialization block, i.e.: > > @@ -32,9 +32,10 @@ static int __test__sw_clock_freq(enum perf_sw_ids clock_id) > .exclude_kernel = 1, > .disabled = 1, > .freq = 1, > - .sample_freq = 10000, > }; > > + attr.sample_freq = 10000; > + > Thanks for fixing this! Namhyung ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/core] perf tests: Add a test case for checking sw clock event frequency 2013-03-18 2:41 ` [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency Namhyung Kim 2013-03-18 14:42 ` Arnaldo Carvalho de Melo @ 2013-03-21 11:51 ` tip-bot for Namhyung Kim 1 sibling, 0 replies; 6+ messages in thread From: tip-bot for Namhyung Kim @ 2013-03-21 11:51 UTC (permalink / raw) To: linux-tip-commits Cc: acme, linux-kernel, hpa, mingo, a.p.zijlstra, namhyung.kim, namhyung, jolsa, tglx Commit-ID: bc96b361cbf90e61d2665b1305cd2c4ac1fd9cfc Gitweb: http://git.kernel.org/tip/bc96b361cbf90e61d2665b1305cd2c4ac1fd9cfc Author: Namhyung Kim <namhyung.kim@lge.com> AuthorDate: Mon, 18 Mar 2013 11:41:47 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Mon, 18 Mar 2013 11:43:16 -0300 perf tests: Add a test case for checking sw clock event frequency This test case checks frequency conversion of hrtimer-based software clock events (cpu-clock, task-clock) have valid (non-1) periods. Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Ingo Molnar <mingo@kernel.org> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Link: http://lkml.kernel.org/r/1363574507-18808-2-git-send-email-namhyung@kernel.org [ committer note: Moved .sample_freq to outside named init block to cope with some gcc versions ] Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/Makefile | 1 + tools/perf/tests/builtin-test.c | 4 ++ tools/perf/tests/sw-clock.c | 119 ++++++++++++++++++++++++++++++++++++++++ tools/perf/tests/tests.h | 1 + 4 files changed, 125 insertions(+) diff --git a/tools/perf/Makefile b/tools/perf/Makefile index 8e1bba3..0230b75 100644 --- a/tools/perf/Makefile +++ b/tools/perf/Makefile @@ -514,6 +514,7 @@ LIB_OBJS += $(OUTPUT)tests/python-use.o LIB_OBJS += $(OUTPUT)tests/bp_signal.o LIB_OBJS += $(OUTPUT)tests/bp_signal_overflow.o LIB_OBJS += $(OUTPUT)tests/task-exit.o +LIB_OBJS += $(OUTPUT)tests/sw-clock.o BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o BUILTIN_OBJS += $(OUTPUT)builtin-bench.o diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 9b5c70a..0918ada 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -90,6 +90,10 @@ static struct test { .func = test__task_exit, }, { + .desc = "Test software clock events have valid period values", + .func = test__sw_clock_freq, + }, + { .func = NULL, }, }; diff --git a/tools/perf/tests/sw-clock.c b/tools/perf/tests/sw-clock.c new file mode 100644 index 0000000..2e41e2d --- /dev/null +++ b/tools/perf/tests/sw-clock.c @@ -0,0 +1,119 @@ +#include <unistd.h> +#include <stdlib.h> +#include <signal.h> +#include <sys/mman.h> + +#include "tests.h" +#include "util/evsel.h" +#include "util/evlist.h" +#include "util/cpumap.h" +#include "util/thread_map.h" + +#define NR_LOOPS 1000000 + +/* + * This test will open software clock events (cpu-clock, task-clock) + * then check their frequency -> period conversion has no artifact of + * setting period to 1 forcefully. + */ +static int __test__sw_clock_freq(enum perf_sw_ids clock_id) +{ + int i, err = -1; + volatile int tmp = 0; + u64 total_periods = 0; + int nr_samples = 0; + union perf_event *event; + struct perf_evsel *evsel; + struct perf_evlist *evlist; + struct perf_event_attr attr = { + .type = PERF_TYPE_SOFTWARE, + .config = clock_id, + .sample_type = PERF_SAMPLE_PERIOD, + .exclude_kernel = 1, + .disabled = 1, + .freq = 1, + }; + + attr.sample_freq = 10000; + + evlist = perf_evlist__new(); + if (evlist == NULL) { + pr_debug("perf_evlist__new\n"); + return -1; + } + + evsel = perf_evsel__new(&attr, 0); + if (evsel == NULL) { + pr_debug("perf_evsel__new\n"); + goto out_free_evlist; + } + perf_evlist__add(evlist, evsel); + + evlist->cpus = cpu_map__dummy_new(); + evlist->threads = thread_map__new_by_tid(getpid()); + if (!evlist->cpus || !evlist->threads) { + err = -ENOMEM; + pr_debug("Not enough memory to create thread/cpu maps\n"); + goto out_delete_maps; + } + + perf_evlist__open(evlist); + + err = perf_evlist__mmap(evlist, 128, true); + if (err < 0) { + pr_debug("failed to mmap event: %d (%s)\n", errno, + strerror(errno)); + goto out_close_evlist; + } + + perf_evlist__enable(evlist); + + /* collect samples */ + for (i = 0; i < NR_LOOPS; i++) + tmp++; + + perf_evlist__disable(evlist); + + while ((event = perf_evlist__mmap_read(evlist, 0)) != NULL) { + struct perf_sample sample; + + if (event->header.type != PERF_RECORD_SAMPLE) + continue; + + err = perf_evlist__parse_sample(evlist, event, &sample); + if (err < 0) { + pr_debug("Error during parse sample\n"); + goto out_unmap_evlist; + } + + total_periods += sample.period; + nr_samples++; + } + + if ((u64) nr_samples == total_periods) { + pr_debug("All (%d) samples have period value of 1!\n", + nr_samples); + err = -1; + } + +out_unmap_evlist: + perf_evlist__munmap(evlist); +out_close_evlist: + perf_evlist__close(evlist); +out_delete_maps: + perf_evlist__delete_maps(evlist); +out_free_evlist: + perf_evlist__delete(evlist); + return err; +} + +int test__sw_clock_freq(void) +{ + int ret; + + ret = __test__sw_clock_freq(PERF_COUNT_SW_CPU_CLOCK); + if (!ret) + ret = __test__sw_clock_freq(PERF_COUNT_SW_TASK_CLOCK); + + return ret; +} diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h index b33b328..dd7feae 100644 --- a/tools/perf/tests/tests.h +++ b/tools/perf/tests/tests.h @@ -26,5 +26,6 @@ int test__python_use(void); int test__bp_signal(void); int test__bp_signal_overflow(void); int test__task_exit(void); +int test__sw_clock_freq(void); #endif /* TESTS_H */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:perf/urgent] perf: Reset hwc->last_period on sw clock events 2013-03-18 2:41 [PATCH 1/2] perf: Reset hwc->last_period on sw clock events Namhyung Kim 2013-03-18 2:41 ` [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency Namhyung Kim @ 2013-03-18 11:06 ` tip-bot for Namhyung Kim 1 sibling, 0 replies; 6+ messages in thread From: tip-bot for Namhyung Kim @ 2013-03-18 11:06 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, a.p.zijlstra, acme, namhyung.kim, namhyung, jolsa, tglx Commit-ID: 778141e3cf0bf29f91cd3cb5c314ea477b9402a7 Gitweb: http://git.kernel.org/tip/778141e3cf0bf29f91cd3cb5c314ea477b9402a7 Author: Namhyung Kim <namhyung.kim@lge.com> AuthorDate: Mon, 18 Mar 2013 11:41:46 +0900 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Mon, 18 Mar 2013 09:15:18 +0100 perf: Reset hwc->last_period on sw clock events When cpu/task clock events are initialized, their sampling frequencies are converted to have a fixed value. However it missed to update the hwc->last_period which was set to 1 for initial sampling frequency calibration. Because this hwc->last_period value is used as a period in perf_swevent_ hrtime(), every recorded sample will have an incorrected period of 1. $ perf record -e task-clock noploop 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.158 MB perf.data (~6919 samples) ] $ perf report -n --show-total-period --stdio # Samples: 4K of event 'task-clock' # Event count (approx.): 4000 # # Overhead Samples Period Command Shared Object Symbol # ........ ............ ............ ....... ............. .................. # 99.95% 3998 3998 noploop noploop [.] main 0.03% 1 1 noploop libc-2.15.so [.] init_cacheinfo 0.03% 1 1 noploop ld-2.15.so [.] open_verify Note that it doesn't affect the non-sampling event so that the perf stat still gets correct value with or without this patch. $ perf stat -e task-clock noploop 1 Performance counter stats for 'noploop 1': 1000.272525 task-clock # 1.000 CPUs utilized 1.000560605 seconds time elapsed Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net> Cc: Jiri Olsa <jolsa@redhat.com> Cc: Namhyung Kim <namhyung.kim@lge.com> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Link: http://lkml.kernel.org/r/1363574507-18808-1-git-send-email-namhyung@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- kernel/events/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index b0cd865..fa79c37 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -5647,6 +5647,7 @@ static void perf_swevent_init_hrtimer(struct perf_event *event) event->attr.sample_period = NSEC_PER_SEC / freq; hwc->sample_period = event->attr.sample_period; local64_set(&hwc->period_left, hwc->sample_period); + hwc->last_period = hwc->sample_period; event->attr.freq = 0; } } ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-21 11:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-18 2:41 [PATCH 1/2] perf: Reset hwc->last_period on sw clock events Namhyung Kim 2013-03-18 2:41 ` [PATCH 2/2] perf tests: Add a test case for checking sw clock event frequency Namhyung Kim 2013-03-18 14:42 ` Arnaldo Carvalho de Melo 2013-03-19 5:10 ` Namhyung Kim 2013-03-21 11:51 ` [tip:perf/core] " tip-bot for Namhyung Kim 2013-03-18 11:06 ` [tip:perf/urgent] perf: Reset hwc->last_period on sw clock events tip-bot for Namhyung Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox