Messages in this thread | | | Date | Fri, 29 Mar 2024 11:02:33 +0800 | Subject | Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure | From | Yang Jihong <> |
| |
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
| |