linux-kernel - Re: [External] Re: [PATCH 1/2] perf sched timehist: Fix -g/--call-graph option failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 29 Mar 2024 11:02:33 +0800
From: Yang Jihong <yangjihong@...edance.com>
To: Ian Rogers <irogers@...gle.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

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.

Thanks,
Yang