[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fWDY9Aj+qHNOuwJ9yE==G=vmCzECXYEZAifOhGHX7yr6w@mail.gmail.com>
Date: Fri, 29 Mar 2024 09:08:01 -0700
From: Ian Rogers <irogers@...gle.com>
To: Yang Jihong <yangjihong@...edance.com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
namhyung@...nel.org, mark.rutland@....com, alexander.shishkin@...ux.intel.com,
jolsa@...nel.org, adrian.hunter@...el.com, kan.liang@...ux.intel.com,
james.clark@....com, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix
-g/--call-graph option failure
On Thu, Mar 28, 2024 at 8:02 PM Yang Jihong <yangjihong@...edance.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@...edance.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@...gle.com>
Thanks,
Ian
> Thanks,
> Yang
Powered by blists - more mailing lists