* [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure
@ 2024-03-28 5:58 Yang Jihong
2024-03-28 5:58 ` [PATCH 2/2] perf evsel: Use evsel__name_is() helper Yang Jihong
2024-03-28 16:02 ` [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Ian Rogers
0 siblings, 2 replies; 8+ messages in thread
From: Yang Jihong @ 2024-03-28 5:58 UTC (permalink / raw)
To: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin,
jolsa, irogers, adrian.hunter, kan.liang, james.clark,
linux-perf-users, linux-kernel
Cc: yangjihong
When perf-sched enables the call-graph recording, sample_type of dummy
event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks
that the evsel does not have a callchain, and set show_callchain to 0.
Currently perf sched timehist only saves callchain when processing
sched:sched_switch event, timehist_check_attr() only needs to determine
whether the event has PERF_SAMPLE_CALLCHAIN.
Before:
# perf sched record -g true
[ perf record: Woken up 0 times to write data ]
[ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ]
# perf sched timehist
Samples do not have callchains.
time cpu task name wait time sch delay run time
[tid/pid] (msec) (msec) (msec)
--------------- ------ ------------------------------ --------- --------- ---------
147851.826019 [0000] perf[285035] 0.000 0.000 0.000
147851.826029 [0000] migration/0[15] 0.000 0.003 0.009
147851.826063 [0001] perf[285035] 0.000 0.000 0.000
147851.826069 [0001] migration/1[21] 0.000 0.003 0.006
<SNIP>
After:
# perf sched record -g true
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ]
# perf sched timehist
time cpu task name wait time sch delay run time
[tid/pid] (msec) (msec) (msec)
--------------- ------ ------------------------------ --------- --------- ---------
144193.035164 [0000] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion
144193.035174 [0000] migration/0[15] 0.000 0.003 0.009 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork
144193.035207 [0001] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion
144193.035214 [0001] migration/1[21] 0.000 0.003 0.007 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork
<SNIP>
Signed-off-by: Yang Jihong <yangjihong@bytedance.com>
---
tools/perf/builtin-sched.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index b248c433529a..1bfb22347371 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2963,8 +2963,11 @@ static int timehist_check_attr(struct perf_sched *sched,
return -1;
}
- if (sched->show_callchain && !evsel__has_callchain(evsel)) {
- pr_info("Samples do not have callchains.\n");
+ /* only need to save callchain related to sched_switch event */
+ if (sched->show_callchain &&
+ evsel__name_is(evsel, "sched:sched_switch") &&
+ !evsel__has_callchain(evsel)) {
+ pr_info("Samples of sched_switch event do not have callchains.\n");
sched->show_callchain = 0;
symbol_conf.use_callchain = 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/2] perf evsel: Use evsel__name_is() helper 2024-03-28 5:58 [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Yang Jihong @ 2024-03-28 5:58 ` Yang Jihong 2024-03-28 16:06 ` Ian Rogers 2024-03-28 16:02 ` [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Ian Rogers 1 sibling, 1 reply; 8+ messages in thread From: Yang Jihong @ 2024-03-28 5:58 UTC (permalink / raw) To: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel Cc: yangjihong Code cleanup, replace strcmp(evsel__name(evsel, {NAME})) with evsel__name_is() helper. No functional change. Signed-off-by: Yang Jihong <yangjihong@bytedance.com> --- tools/perf/builtin-kmem.c | 2 +- tools/perf/builtin-sched.c | 4 +-- tools/perf/builtin-script.c | 2 +- tools/perf/builtin-trace.c | 4 +-- tools/perf/tests/evsel-roundtrip-name.c | 4 +-- tools/perf/tests/parse-events.c | 39 +++++++++---------------- 6 files changed, 22 insertions(+), 33 deletions(-) diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c index 9714327fd0ea..6fd95be5032b 100644 --- a/tools/perf/builtin-kmem.c +++ b/tools/perf/builtin-kmem.c @@ -1408,7 +1408,7 @@ static int __cmd_kmem(struct perf_session *session) } evlist__for_each_entry(session->evlist, evsel) { - if (!strcmp(evsel__name(evsel), "kmem:mm_page_alloc") && + if (evsel__name_is(evsel, "kmem:mm_page_alloc") && evsel__field(evsel, "pfn")) { use_pfn = true; break; diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index 1bfb22347371..0fce7d8986c0 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -2148,7 +2148,7 @@ static bool is_idle_sample(struct perf_sample *sample, struct evsel *evsel) { /* pid 0 == swapper == idle task */ - if (strcmp(evsel__name(evsel), "sched:sched_switch") == 0) + if (evsel__name_is(evsel, "sched:sched_switch")) return evsel__intval(evsel, sample, "prev_pid") == 0; return sample->pid == 0; @@ -2375,7 +2375,7 @@ static bool timehist_skip_sample(struct perf_sched *sched, } if (sched->idle_hist) { - if (strcmp(evsel__name(evsel), "sched:sched_switch")) + if (!evsel__name_is(evsel, "sched:sched_switch")) rc = true; else if (evsel__intval(evsel, sample, "prev_pid") != 0 && evsel__intval(evsel, sample, "next_pid") != 0) diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 37088cc0ff1b..cc981531ec00 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -3471,7 +3471,7 @@ static int check_ev_match(char *dir_name, char *scriptname, match = 0; evlist__for_each_entry(session->evlist, pos) { - if (!strcmp(evsel__name(pos), evname)) { + if (evsel__name_is(pos, evname)) { match = 1; break; } diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 90eaff8c0f6e..9b93807a1906 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -4902,7 +4902,7 @@ int cmd_trace(int argc, const char **argv) goto out; } trace.syscalls.events.bpf_output = evlist__last(trace.evlist); - assert(!strcmp(evsel__name(trace.syscalls.events.bpf_output), "__augmented_syscalls__")); + assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__"); skip_augmentation: #endif err = -1; @@ -4959,7 +4959,7 @@ int cmd_trace(int argc, const char **argv) */ if (trace.syscalls.events.bpf_output) { evlist__for_each_entry(trace.evlist, evsel) { - bool raw_syscalls_sys_exit = strcmp(evsel__name(evsel), "raw_syscalls:sys_exit") == 0; + bool raw_syscalls_sys_exit = evsel__name_is(evsel, "raw_syscalls:sys_exit"); if (raw_syscalls_sys_exit) { trace.raw_augmented_syscalls = true; diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c index 15ff86f9da0b..1922cac13a24 100644 --- a/tools/perf/tests/evsel-roundtrip-name.c +++ b/tools/perf/tests/evsel-roundtrip-name.c @@ -37,7 +37,7 @@ static int perf_evsel__roundtrip_cache_name_test(void) continue; } evlist__for_each_entry(evlist, evsel) { - if (strcmp(evsel__name(evsel), name)) { + if (!evsel__name_is(evsel, name)) { pr_debug("%s != %s\n", evsel__name(evsel), name); ret = TEST_FAIL; } @@ -71,7 +71,7 @@ static int perf_evsel__name_array_test(const char *const names[], int nr_names) continue; } evlist__for_each_entry(evlist, evsel) { - if (strcmp(evsel__name(evsel), names[i])) { + if (!evsel__name_is(evsel, names[i])) { pr_debug("%s != %s\n", evsel__name(evsel), names[i]); ret = TEST_FAIL; } diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index feb5727584d1..0b70451451b3 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -470,8 +470,7 @@ static int test__checkevent_breakpoint_modifier(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "mem:0:u")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:u")); return test__checkevent_breakpoint(evlist); } @@ -484,8 +483,7 @@ static int test__checkevent_breakpoint_x_modifier(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "mem:0:x:k")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:x:k")); return test__checkevent_breakpoint_x(evlist); } @@ -498,8 +496,7 @@ static int test__checkevent_breakpoint_r_modifier(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "mem:0:r:hp")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:r:hp")); return test__checkevent_breakpoint_r(evlist); } @@ -512,8 +509,7 @@ static int test__checkevent_breakpoint_w_modifier(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "mem:0:w:up")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:w:up")); return test__checkevent_breakpoint_w(evlist); } @@ -526,8 +522,7 @@ static int test__checkevent_breakpoint_rw_modifier(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "mem:0:rw:kp")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:rw:kp")); return test__checkevent_breakpoint_rw(evlist); } @@ -540,8 +535,7 @@ static int test__checkevent_breakpoint_modifier_name(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "breakpoint")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); return test__checkevent_breakpoint(evlist); } @@ -554,8 +548,7 @@ static int test__checkevent_breakpoint_x_modifier_name(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "breakpoint")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); return test__checkevent_breakpoint_x(evlist); } @@ -568,8 +561,7 @@ static int test__checkevent_breakpoint_r_modifier_name(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "breakpoint")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); return test__checkevent_breakpoint_r(evlist); } @@ -582,8 +574,7 @@ static int test__checkevent_breakpoint_w_modifier_name(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "breakpoint")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); return test__checkevent_breakpoint_w(evlist); } @@ -596,8 +587,7 @@ static int test__checkevent_breakpoint_rw_modifier_name(struct evlist *evlist) TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "breakpoint")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); return test__checkevent_breakpoint_rw(evlist); } @@ -609,12 +599,12 @@ static int test__checkevent_breakpoint_2_events(struct evlist *evlist) TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type); - TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint1")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint1")); evsel = evsel__next(evsel); TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type); - TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint2")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint2")); return TEST_OK; } @@ -691,15 +681,14 @@ static int test__checkevent_pmu_name(struct evlist *evlist) TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type); TEST_ASSERT_VAL("wrong config", test_config(evsel, 1)); - TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "krava")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "krava")); /* cpu/config=2/u" */ evsel = evsel__next(evsel); TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type); TEST_ASSERT_VAL("wrong config", test_config(evsel, 2)); - TEST_ASSERT_VAL("wrong name", - !strcmp(evsel__name(evsel), "cpu/config=2/u")); + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "cpu/config=2/u")); return TEST_OK; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] perf evsel: Use evsel__name_is() helper 2024-03-28 5:58 ` [PATCH 2/2] perf evsel: Use evsel__name_is() helper Yang Jihong @ 2024-03-28 16:06 ` Ian Rogers 0 siblings, 0 replies; 8+ messages in thread From: Ian Rogers @ 2024-03-28 16:06 UTC (permalink / raw) To: Yang Jihong Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel On Wed, Mar 27, 2024 at 10:59 PM Yang Jihong <yangjihong@bytedance.com> wrote: > > Code cleanup, replace strcmp(evsel__name(evsel, {NAME})) with > evsel__name_is() helper. > > No functional change. > > Signed-off-by: Yang Jihong <yangjihong@bytedance.com> Reviewed-by: Ian Rogers <irogers@google.com> Thanks, Ian > --- > tools/perf/builtin-kmem.c | 2 +- > tools/perf/builtin-sched.c | 4 +-- > tools/perf/builtin-script.c | 2 +- > tools/perf/builtin-trace.c | 4 +-- > tools/perf/tests/evsel-roundtrip-name.c | 4 +-- > tools/perf/tests/parse-events.c | 39 +++++++++---------------- > 6 files changed, 22 insertions(+), 33 deletions(-) > > diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c > index 9714327fd0ea..6fd95be5032b 100644 > --- a/tools/perf/builtin-kmem.c > +++ b/tools/perf/builtin-kmem.c > @@ -1408,7 +1408,7 @@ static int __cmd_kmem(struct perf_session *session) > } > > evlist__for_each_entry(session->evlist, evsel) { > - if (!strcmp(evsel__name(evsel), "kmem:mm_page_alloc") && > + if (evsel__name_is(evsel, "kmem:mm_page_alloc") && > evsel__field(evsel, "pfn")) { > use_pfn = true; > break; > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c > index 1bfb22347371..0fce7d8986c0 100644 > --- a/tools/perf/builtin-sched.c > +++ b/tools/perf/builtin-sched.c > @@ -2148,7 +2148,7 @@ static bool is_idle_sample(struct perf_sample *sample, > struct evsel *evsel) > { > /* pid 0 == swapper == idle task */ > - if (strcmp(evsel__name(evsel), "sched:sched_switch") == 0) > + if (evsel__name_is(evsel, "sched:sched_switch")) > return evsel__intval(evsel, sample, "prev_pid") == 0; > > return sample->pid == 0; > @@ -2375,7 +2375,7 @@ static bool timehist_skip_sample(struct perf_sched *sched, > } > > if (sched->idle_hist) { > - if (strcmp(evsel__name(evsel), "sched:sched_switch")) > + if (!evsel__name_is(evsel, "sched:sched_switch")) > rc = true; > else if (evsel__intval(evsel, sample, "prev_pid") != 0 && > evsel__intval(evsel, sample, "next_pid") != 0) > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 37088cc0ff1b..cc981531ec00 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -3471,7 +3471,7 @@ static int check_ev_match(char *dir_name, char *scriptname, > > match = 0; > evlist__for_each_entry(session->evlist, pos) { > - if (!strcmp(evsel__name(pos), evname)) { > + if (evsel__name_is(pos, evname)) { > match = 1; > break; > } > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c > index 90eaff8c0f6e..9b93807a1906 100644 > --- a/tools/perf/builtin-trace.c > +++ b/tools/perf/builtin-trace.c > @@ -4902,7 +4902,7 @@ int cmd_trace(int argc, const char **argv) > goto out; > } > trace.syscalls.events.bpf_output = evlist__last(trace.evlist); > - assert(!strcmp(evsel__name(trace.syscalls.events.bpf_output), "__augmented_syscalls__")); > + assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__"); > skip_augmentation: > #endif > err = -1; > @@ -4959,7 +4959,7 @@ int cmd_trace(int argc, const char **argv) > */ > if (trace.syscalls.events.bpf_output) { > evlist__for_each_entry(trace.evlist, evsel) { > - bool raw_syscalls_sys_exit = strcmp(evsel__name(evsel), "raw_syscalls:sys_exit") == 0; > + bool raw_syscalls_sys_exit = evsel__name_is(evsel, "raw_syscalls:sys_exit"); > > if (raw_syscalls_sys_exit) { > trace.raw_augmented_syscalls = true; > diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c > index 15ff86f9da0b..1922cac13a24 100644 > --- a/tools/perf/tests/evsel-roundtrip-name.c > +++ b/tools/perf/tests/evsel-roundtrip-name.c > @@ -37,7 +37,7 @@ static int perf_evsel__roundtrip_cache_name_test(void) > continue; > } > evlist__for_each_entry(evlist, evsel) { > - if (strcmp(evsel__name(evsel), name)) { > + if (!evsel__name_is(evsel, name)) { > pr_debug("%s != %s\n", evsel__name(evsel), name); > ret = TEST_FAIL; > } > @@ -71,7 +71,7 @@ static int perf_evsel__name_array_test(const char *const names[], int nr_names) > continue; > } > evlist__for_each_entry(evlist, evsel) { > - if (strcmp(evsel__name(evsel), names[i])) { > + if (!evsel__name_is(evsel, names[i])) { > pr_debug("%s != %s\n", evsel__name(evsel), names[i]); > ret = TEST_FAIL; > } > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c > index feb5727584d1..0b70451451b3 100644 > --- a/tools/perf/tests/parse-events.c > +++ b/tools/perf/tests/parse-events.c > @@ -470,8 +470,7 @@ static int test__checkevent_breakpoint_modifier(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "mem:0:u")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:u")); > > return test__checkevent_breakpoint(evlist); > } > @@ -484,8 +483,7 @@ static int test__checkevent_breakpoint_x_modifier(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "mem:0:x:k")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:x:k")); > > return test__checkevent_breakpoint_x(evlist); > } > @@ -498,8 +496,7 @@ static int test__checkevent_breakpoint_r_modifier(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "mem:0:r:hp")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:r:hp")); > > return test__checkevent_breakpoint_r(evlist); > } > @@ -512,8 +509,7 @@ static int test__checkevent_breakpoint_w_modifier(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "mem:0:w:up")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:w:up")); > > return test__checkevent_breakpoint_w(evlist); > } > @@ -526,8 +522,7 @@ static int test__checkevent_breakpoint_rw_modifier(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "mem:0:rw:kp")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:rw:kp")); > > return test__checkevent_breakpoint_rw(evlist); > } > @@ -540,8 +535,7 @@ static int test__checkevent_breakpoint_modifier_name(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "breakpoint")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); > > return test__checkevent_breakpoint(evlist); > } > @@ -554,8 +548,7 @@ static int test__checkevent_breakpoint_x_modifier_name(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "breakpoint")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); > > return test__checkevent_breakpoint_x(evlist); > } > @@ -568,8 +561,7 @@ static int test__checkevent_breakpoint_r_modifier_name(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "breakpoint")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); > > return test__checkevent_breakpoint_r(evlist); > } > @@ -582,8 +574,7 @@ static int test__checkevent_breakpoint_w_modifier_name(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "breakpoint")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); > > return test__checkevent_breakpoint_w(evlist); > } > @@ -596,8 +587,7 @@ static int test__checkevent_breakpoint_rw_modifier_name(struct evlist *evlist) > TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel); > TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv); > TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "breakpoint")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint")); > > return test__checkevent_breakpoint_rw(evlist); > } > @@ -609,12 +599,12 @@ static int test__checkevent_breakpoint_2_events(struct evlist *evlist) > TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); > > TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type); > - TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint1")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint1")); > > evsel = evsel__next(evsel); > > TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type); > - TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint2")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint2")); > > return TEST_OK; > } > @@ -691,15 +681,14 @@ static int test__checkevent_pmu_name(struct evlist *evlist) > TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); > TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type); > TEST_ASSERT_VAL("wrong config", test_config(evsel, 1)); > - TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "krava")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "krava")); > > /* cpu/config=2/u" */ > evsel = evsel__next(evsel); > TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries); > TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type); > TEST_ASSERT_VAL("wrong config", test_config(evsel, 2)); > - TEST_ASSERT_VAL("wrong name", > - !strcmp(evsel__name(evsel), "cpu/config=2/u")); > + TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "cpu/config=2/u")); > > return TEST_OK; > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure 2024-03-28 5:58 [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Yang Jihong 2024-03-28 5:58 ` [PATCH 2/2] perf evsel: Use evsel__name_is() helper Yang Jihong @ 2024-03-28 16:02 ` Ian Rogers 2024-03-29 3:02 ` [External] " Yang Jihong 1 sibling, 1 reply; 8+ messages in thread From: Ian Rogers @ 2024-03-28 16:02 UTC (permalink / raw) To: Yang Jihong Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel On Wed, Mar 27, 2024 at 10:59 PM Yang Jihong <yangjihong@bytedance.com> wrote: > > When perf-sched enables the call-graph recording, sample_type of dummy > event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks > that the evsel does not have a callchain, and set show_callchain to 0. > > Currently perf sched timehist only saves callchain when processing > sched:sched_switch event, timehist_check_attr() only needs to determine > whether the event has PERF_SAMPLE_CALLCHAIN. > > Before: > # perf sched record -g true > [ perf record: Woken up 0 times to write data ] > [ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ] > # perf sched timehist > Samples do not have callchains. > time cpu task name wait time sch delay run time > [tid/pid] (msec) (msec) (msec) > --------------- ------ ------------------------------ --------- --------- --------- > 147851.826019 [0000] perf[285035] 0.000 0.000 0.000 > 147851.826029 [0000] migration/0[15] 0.000 0.003 0.009 > 147851.826063 [0001] perf[285035] 0.000 0.000 0.000 > 147851.826069 [0001] migration/1[21] 0.000 0.003 0.006 > <SNIP> > > After: > # perf sched record -g true > [ perf record: Woken up 1 times to write data ] > [ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ] > # perf sched timehist > time cpu task name wait time sch delay run time > [tid/pid] (msec) (msec) (msec) > --------------- ------ ------------------------------ --------- --------- --------- > 144193.035164 [0000] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion > 144193.035174 [0000] migration/0[15] 0.000 0.003 0.009 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork > 144193.035207 [0001] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion > 144193.035214 [0001] migration/1[21] 0.000 0.003 0.007 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork > <SNIP> This looks good, should there be a Fixes tag for the sake of backports? Thanks, Ian > Signed-off-by: Yang Jihong <yangjihong@bytedance.com> > --- > tools/perf/builtin-sched.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c > index b248c433529a..1bfb22347371 100644 > --- a/tools/perf/builtin-sched.c > +++ b/tools/perf/builtin-sched.c > @@ -2963,8 +2963,11 @@ static int timehist_check_attr(struct perf_sched *sched, > return -1; > } > > - if (sched->show_callchain && !evsel__has_callchain(evsel)) { > - pr_info("Samples do not have callchains.\n"); > + /* only need to save callchain related to sched_switch event */ > + if (sched->show_callchain && > + evsel__name_is(evsel, "sched:sched_switch") && > + !evsel__has_callchain(evsel)) { > + pr_info("Samples of sched_switch event do not have callchains.\n"); > sched->show_callchain = 0; > symbol_conf.use_callchain = 0; > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure 2024-03-28 16:02 ` [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Ian Rogers @ 2024-03-29 3:02 ` Yang Jihong 2024-03-29 16:08 ` Ian Rogers 0 siblings, 1 reply; 8+ messages in thread From: Yang Jihong @ 2024-03-29 3:02 UTC (permalink / raw) To: Ian Rogers Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel Hello, Sorry, due to the new email settings, the last reply email was in html format, resend it now. On 3/29/24 00:02, Ian Rogers wrote: > On Wed, Mar 27, 2024 at 10:59 PM Yang Jihong <yangjihong@bytedance.com> wrote: >> >> When perf-sched enables the call-graph recording, sample_type of dummy >> event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks >> that the evsel does not have a callchain, and set show_callchain to 0. >> >> Currently perf sched timehist only saves callchain when processing >> sched:sched_switch event, timehist_check_attr() only needs to determine >> whether the event has PERF_SAMPLE_CALLCHAIN. >> >> Before: >> # perf sched record -g true >> [ perf record: Woken up 0 times to write data ] >> [ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ] >> # perf sched timehist >> Samples do not have callchains. >> time cpu task name wait time sch delay run time >> [tid/pid] (msec) (msec) (msec) >> --------------- ------ ------------------------------ --------- --------- --------- >> 147851.826019 [0000] perf[285035] 0.000 0.000 0.000 >> 147851.826029 [0000] migration/0[15] 0.000 0.003 0.009 >> 147851.826063 [0001] perf[285035] 0.000 0.000 0.000 >> 147851.826069 [0001] migration/1[21] 0.000 0.003 0.006 >> <SNIP> >> >> After: >> # perf sched record -g true >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ] >> # perf sched timehist >> time cpu task name wait time sch delay run time >> [tid/pid] (msec) (msec) (msec) >> --------------- ------ ------------------------------ --------- --------- --------- >> 144193.035164 [0000] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion >> 144193.035174 [0000] migration/0[15] 0.000 0.003 0.009 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork >> 144193.035207 [0001] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion >> 144193.035214 [0001] migration/1[21] 0.000 0.003 0.007 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork >> <SNIP> > > This looks good, should there be a Fixes tag for the sake of backports? > The direct cause is commit 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() helper"). perf-record uses evlist__add_aux_dummy() to replace evlist__add_dummy() to add a dummy event. The difference is that evlist__add_aux_dummy() sets no_aux_samples to true (this is expected behavior, for dummy event, no need to sample aux data), resulting in evsel__config() not adding the PERF_SAMPLE_CALLCHAIN bit to dummy's sample_type. In summary, the direct cause is the problem introduced by commit 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() helper"), but the root cause is the timehist_check_attr() logic problem, The dummy event itself does not need to have PERF_SAMPLE_CALLCHAIN, so there is no need to check it. So, maybe add fixes-tag: Fixes: 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() helper") If it is ok, I will send v2 version with this fixes-tag. Thanks, Yang ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure 2024-03-29 3:02 ` [External] " Yang Jihong @ 2024-03-29 16:08 ` Ian Rogers 2024-03-30 14:48 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Ian Rogers @ 2024-03-29 16:08 UTC (permalink / raw) To: Yang Jihong Cc: peterz, mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel On Thu, Mar 28, 2024 at 8:02 PM Yang Jihong <yangjihong@bytedance.com> wrote: > > Hello, > > Sorry, due to the new email settings, the last reply email was in html > format, resend it now. > > On 3/29/24 00:02, Ian Rogers wrote: > > On Wed, Mar 27, 2024 at 10:59 PM Yang Jihong <yangjihong@bytedance.com> wrote: > >> > >> When perf-sched enables the call-graph recording, sample_type of dummy > >> event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks > >> that the evsel does not have a callchain, and set show_callchain to 0. > >> > >> Currently perf sched timehist only saves callchain when processing > >> sched:sched_switch event, timehist_check_attr() only needs to determine > >> whether the event has PERF_SAMPLE_CALLCHAIN. > >> > >> Before: > >> # perf sched record -g true > >> [ perf record: Woken up 0 times to write data ] > >> [ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ] > >> # perf sched timehist > >> Samples do not have callchains. > >> time cpu task name wait time sch delay run time > >> [tid/pid] (msec) (msec) (msec) > >> --------------- ------ ------------------------------ --------- --------- --------- > >> 147851.826019 [0000] perf[285035] 0.000 0.000 0.000 > >> 147851.826029 [0000] migration/0[15] 0.000 0.003 0.009 > >> 147851.826063 [0001] perf[285035] 0.000 0.000 0.000 > >> 147851.826069 [0001] migration/1[21] 0.000 0.003 0.006 > >> <SNIP> > >> > >> After: > >> # perf sched record -g true > >> [ perf record: Woken up 1 times to write data ] > >> [ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ] > >> # perf sched timehist > >> time cpu task name wait time sch delay run time > >> [tid/pid] (msec) (msec) (msec) > >> --------------- ------ ------------------------------ --------- --------- --------- > >> 144193.035164 [0000] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion > >> 144193.035174 [0000] migration/0[15] 0.000 0.003 0.009 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork > >> 144193.035207 [0001] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion > >> 144193.035214 [0001] migration/1[21] 0.000 0.003 0.007 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork > >> <SNIP> > > > > This looks good, should there be a Fixes tag for the sake of backports? > > > The direct cause is commit 9c95e4ef0657 ("perf evlist: Add > evlist__findnew_tracking_event() helper"). perf-record uses > evlist__add_aux_dummy() to replace evlist__add_dummy() to add a dummy > event. The difference is that evlist__add_aux_dummy() sets > no_aux_samples to true (this is expected behavior, for dummy event, no > need to sample aux data), resulting in evsel__config() not adding the > PERF_SAMPLE_CALLCHAIN bit to dummy's sample_type. > > In summary, the direct cause is the problem introduced by commit > 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() > helper"), but the root cause is the timehist_check_attr() logic problem, > The dummy event itself does not need to have PERF_SAMPLE_CALLCHAIN, so > there is no need to check it. > > > So, maybe add fixes-tag: > > Fixes: 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() > helper") > > If it is ok, I will send v2 version with this fixes-tag. I think the maintainer can add the fixes tag when they add the reviewed-by tag: Reviewed-by: Ian Rogers <irogers@google.com> Thanks, Ian > Thanks, > Yang ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure 2024-03-29 16:08 ` Ian Rogers @ 2024-03-30 14:48 ` Arnaldo Carvalho de Melo 2024-04-01 1:50 ` Yang Jihong 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2024-03-30 14:48 UTC (permalink / raw) To: Ian Rogers Cc: Yang Jihong, peterz, mingo, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel On Fri, Mar 29, 2024 at 09:08:01AM -0700, Ian Rogers wrote: > On Thu, Mar 28, 2024 at 8:02 PM Yang Jihong <yangjihong@bytedance.com> wrote: > > > > Hello, > > > > Sorry, due to the new email settings, the last reply email was in html > > format, resend it now. > > > > On 3/29/24 00:02, Ian Rogers wrote: > > > On Wed, Mar 27, 2024 at 10:59 PM Yang Jihong <yangjihong@bytedance.com> wrote: > > >> > > >> When perf-sched enables the call-graph recording, sample_type of dummy > > >> event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks > > >> that the evsel does not have a callchain, and set show_callchain to 0. > > >> > > >> Currently perf sched timehist only saves callchain when processing > > >> sched:sched_switch event, timehist_check_attr() only needs to determine > > >> whether the event has PERF_SAMPLE_CALLCHAIN. > > >> > > >> Before: > > >> # perf sched record -g true > > >> [ perf record: Woken up 0 times to write data ] > > >> [ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ] > > >> # perf sched timehist > > >> Samples do not have callchains. > > >> time cpu task name wait time sch delay run time > > >> [tid/pid] (msec) (msec) (msec) > > >> --------------- ------ ------------------------------ --------- --------- --------- > > >> 147851.826019 [0000] perf[285035] 0.000 0.000 0.000 > > >> 147851.826029 [0000] migration/0[15] 0.000 0.003 0.009 > > >> 147851.826063 [0001] perf[285035] 0.000 0.000 0.000 > > >> 147851.826069 [0001] migration/1[21] 0.000 0.003 0.006 > > >> <SNIP> > > >> > > >> After: > > >> # perf sched record -g true > > >> [ perf record: Woken up 1 times to write data ] > > >> [ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ] > > >> # perf sched timehist > > >> time cpu task name wait time sch delay run time > > >> [tid/pid] (msec) (msec) (msec) > > >> --------------- ------ ------------------------------ --------- --------- --------- > > >> 144193.035164 [0000] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion > > >> 144193.035174 [0000] migration/0[15] 0.000 0.003 0.009 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork > > >> 144193.035207 [0001] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion > > >> 144193.035214 [0001] migration/1[21] 0.000 0.003 0.007 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork > > >> <SNIP> > > > > > > This looks good, should there be a Fixes tag for the sake of backports? > > > > > The direct cause is commit 9c95e4ef0657 ("perf evlist: Add > > evlist__findnew_tracking_event() helper"). perf-record uses > > evlist__add_aux_dummy() to replace evlist__add_dummy() to add a dummy > > event. The difference is that evlist__add_aux_dummy() sets > > no_aux_samples to true (this is expected behavior, for dummy event, no > > need to sample aux data), resulting in evsel__config() not adding the > > PERF_SAMPLE_CALLCHAIN bit to dummy's sample_type. > > > > In summary, the direct cause is the problem introduced by commit > > 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() > > helper"), but the root cause is the timehist_check_attr() logic problem, > > The dummy event itself does not need to have PERF_SAMPLE_CALLCHAIN, so > > there is no need to check it. > > > > > > So, maybe add fixes-tag: > > > > Fixes: 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() > > helper") > > > > If it is ok, I will send v2 version with this fixes-tag. > > I think the maintainer can add the fixes tag when they add the reviewed-by tag: I usually do this, but if the submitter does it I'll have just to check that it is the right tag, helps a bit in processing. - Arnaldo > Reviewed-by: Ian Rogers <irogers@google.com> > > Thanks, > Ian > > > Thanks, > > Yang ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure 2024-03-30 14:48 ` Arnaldo Carvalho de Melo @ 2024-04-01 1:50 ` Yang Jihong 0 siblings, 0 replies; 8+ messages in thread From: Yang Jihong @ 2024-04-01 1:50 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ian Rogers Cc: peterz, mingo, namhyung, mark.rutland, alexander.shishkin, jolsa, adrian.hunter, kan.liang, james.clark, linux-perf-users, linux-kernel Hello, On 3/30/24 22:48, Arnaldo Carvalho de Melo wrote: > On Fri, Mar 29, 2024 at 09:08:01AM -0700, Ian Rogers wrote: >> On Thu, Mar 28, 2024 at 8:02 PM Yang Jihong <yangjihong@bytedance.com> wrote: >>> >>> Hello, >>> >>> Sorry, due to the new email settings, the last reply email was in html >>> format, resend it now. >>> >>> On 3/29/24 00:02, Ian Rogers wrote: >>>> On Wed, Mar 27, 2024 at 10:59 PM Yang Jihong <yangjihong@bytedance.com> wrote: >>>>> >>>>> When perf-sched enables the call-graph recording, sample_type of dummy >>>>> event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks >>>>> that the evsel does not have a callchain, and set show_callchain to 0. >>>>> >>>>> Currently perf sched timehist only saves callchain when processing >>>>> sched:sched_switch event, timehist_check_attr() only needs to determine >>>>> whether the event has PERF_SAMPLE_CALLCHAIN. >>>>> >>>>> Before: >>>>> # perf sched record -g true >>>>> [ perf record: Woken up 0 times to write data ] >>>>> [ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ] >>>>> # perf sched timehist >>>>> Samples do not have callchains. >>>>> time cpu task name wait time sch delay run time >>>>> [tid/pid] (msec) (msec) (msec) >>>>> --------------- ------ ------------------------------ --------- --------- --------- >>>>> 147851.826019 [0000] perf[285035] 0.000 0.000 0.000 >>>>> 147851.826029 [0000] migration/0[15] 0.000 0.003 0.009 >>>>> 147851.826063 [0001] perf[285035] 0.000 0.000 0.000 >>>>> 147851.826069 [0001] migration/1[21] 0.000 0.003 0.006 >>>>> <SNIP> >>>>> >>>>> After: >>>>> # perf sched record -g true >>>>> [ perf record: Woken up 1 times to write data ] >>>>> [ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ] >>>>> # perf sched timehist >>>>> time cpu task name wait time sch delay run time >>>>> [tid/pid] (msec) (msec) (msec) >>>>> --------------- ------ ------------------------------ --------- --------- --------- >>>>> 144193.035164 [0000] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion >>>>> 144193.035174 [0000] migration/0[15] 0.000 0.003 0.009 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork >>>>> 144193.035207 [0001] perf[277062] 0.000 0.000 0.000 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion >>>>> 144193.035214 [0001] migration/1[21] 0.000 0.003 0.007 __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork >>>>> <SNIP> >>>> >>>> This looks good, should there be a Fixes tag for the sake of backports? >>>> >>> The direct cause is commit 9c95e4ef0657 ("perf evlist: Add >>> evlist__findnew_tracking_event() helper"). perf-record uses >>> evlist__add_aux_dummy() to replace evlist__add_dummy() to add a dummy >>> event. The difference is that evlist__add_aux_dummy() sets >>> no_aux_samples to true (this is expected behavior, for dummy event, no >>> need to sample aux data), resulting in evsel__config() not adding the >>> PERF_SAMPLE_CALLCHAIN bit to dummy's sample_type. >>> >>> In summary, the direct cause is the problem introduced by commit >>> 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() >>> helper"), but the root cause is the timehist_check_attr() logic problem, >>> The dummy event itself does not need to have PERF_SAMPLE_CALLCHAIN, so >>> there is no need to check it. >>> >>> >>> So, maybe add fixes-tag: >>> >>> Fixes: 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() >>> helper") >>> >>> If it is ok, I will send v2 version with this fixes-tag. >> >> I think the maintainer can add the fixes tag when they add the reviewed-by tag: > > I usually do this, but if the submitter does it I'll have just to check > that it is the right tag, helps a bit in processing. Okay, will send v2 version. > > - Arnaldo > >> Reviewed-by: Ian Rogers <irogers@google.com> Thanks for reviewed-by tag. Thanks, Yang ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-04-01 1:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-28 5:58 [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Yang Jihong 2024-03-28 5:58 ` [PATCH 2/2] perf evsel: Use evsel__name_is() helper Yang Jihong 2024-03-28 16:06 ` Ian Rogers 2024-03-28 16:02 ` [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure Ian Rogers 2024-03-29 3:02 ` [External] " Yang Jihong 2024-03-29 16:08 ` Ian Rogers 2024-03-30 14:48 ` Arnaldo Carvalho de Melo 2024-04-01 1:50 ` Yang Jihong
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.