| 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
| ||
|
Message-ID: <9788f0f1-087f-7f0b-048a-0146afe1f632@intel.com>
Date: Mon, 6 Mar 2023 11:31:52 +0200
From: Adrian Hunter <adrian.hunter@...el.com>
To: "Liang, Kan" <kan.liang@...ux.intel.com>,
Ian Rogers <irogers@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
Ravi Bangoria <ravi.bangoria@....com>,
"Steinar H. Gunderson" <sesse@...gle.com>,
Kim Phillips <kim.phillips@....com>,
Florian Fischer <florian.fischer@...q.space>,
James Clark <james.clark@....com>,
Suzuki Poulouse <suzuki.poulose@....com>,
Sean Christopherson <seanjc@...gle.com>,
Leo Yan <leo.yan@...aro.org>,
John Garry <john.g.garry@...cle.com>,
Kajol Jain <kjain@...ux.ibm.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v2 03/10] perf record: Early auxtrace initialization
before event parsing
On 5/03/23 10:32, Adrian Hunter wrote:
> On 3/03/23 18:40, Liang, Kan wrote:
>>
>>
>> On 2023-03-02 4:25 p.m., Ian Rogers wrote:
>>> This allows event parsing to use the evsel__is_aux_event function,
>>> which is important when determining event grouping.
>>>
>>> Signed-off-by: Ian Rogers <irogers@...gle.com>
>>> ---
>>> tools/perf/arch/x86/util/auxtrace.c | 17 +++++++++++++----
>>> tools/perf/builtin-record.c | 6 ++++++
>>> tools/perf/util/auxtrace.h | 2 ++
>>> 3 files changed, 21 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/perf/arch/x86/util/auxtrace.c b/tools/perf/arch/x86/util/auxtrace.c
>>> index 3da506e13f49..de1e4842ea2e 100644
>>> --- a/tools/perf/arch/x86/util/auxtrace.c
>>> +++ b/tools/perf/arch/x86/util/auxtrace.c
>>> @@ -15,6 +15,19 @@
>>> #include "../../../util/intel-bts.h"
>>> #include "../../../util/evlist.h"
>>>
>>> +void auxtrace__early_init(void)
>>> +{
>>> + struct perf_pmu *intel_pt_pmu;
>>> + struct perf_pmu *intel_bts_pmu;
>>> +
>>> + intel_pt_pmu = perf_pmu__find(INTEL_PT_PMU_NAME);
>>> + if (intel_pt_pmu)
>>> + intel_pt_pmu->auxtrace = true;
>>> + intel_bts_pmu = perf_pmu__find(INTEL_BTS_PMU_NAME);
>>> + if (intel_bts_pmu)
>>> + intel_bts_pmu->auxtrace = true;
>>> +}
>>> +
>>> static
>>> struct auxtrace_record *auxtrace_record__init_intel(struct evlist *evlist,
>>> int *err)
>>> @@ -26,11 +39,7 @@ struct auxtrace_record *auxtrace_record__init_intel(struct evlist *evlist,
>>> bool found_bts = false;
>>>
>>> intel_pt_pmu = perf_pmu__find(INTEL_PT_PMU_NAME);
>>> - if (intel_pt_pmu)
>>> - intel_pt_pmu->auxtrace = true;
>>> intel_bts_pmu = perf_pmu__find(INTEL_BTS_PMU_NAME);
>>> - if (intel_bts_pmu)
>>> - intel_bts_pmu->auxtrace = true;
>>>
>>> evlist__for_each_entry(evlist, evsel) {
>>> if (intel_pt_pmu && evsel->core.attr.type == intel_pt_pmu->type)
>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>> index 8374117e66f6..a0870c076dc0 100644
>>> --- a/tools/perf/builtin-record.c
>>> +++ b/tools/perf/builtin-record.c
>>> @@ -3940,6 +3940,10 @@ static int record__init_thread_masks(struct record *rec)
>>> return ret;
>>> }
>>>
>>> +__weak void auxtrace__early_init(void)
>>> +{
>>> +}
>>> +
>>> int cmd_record(int argc, const char **argv)
>>> {
>>> int err;
>>> @@ -3985,6 +3989,8 @@ int cmd_record(int argc, const char **argv)
>>> if (err)
>>> return err;
>>>
>>> + auxtrace__early_init();
>>
>> So the auxtrace__early_init() will be unconditionally invoked even there
>> is no PT or BTS events, right?
>>
>> Maybe we should move the auxtrace__early_init() to evsel__is_aux_event()
>> and cache the value. The initialization will only be invoked when it's
>> required.
>
> Although perf_pmu__find() will be called unconditionally via
> record__auxtrace_init() anyway.
However auxtrace__early_init() is before parsing 'verbose' so
debug prints don't work anymore.
How about this instead:
diff --git a/tools/perf/arch/x86/util/auxtrace.c
b/tools/perf/arch/x86/util/auxtrace.c
index 3da506e13f49d..330d03216b0e6 100644
--- a/tools/perf/arch/x86/util/auxtrace.c
+++ b/tools/perf/arch/x86/util/auxtrace.c
@@ -26,11 +26,7 @@ struct auxtrace_record
*auxtrace_record__init_intel(struct evlist *evlist,
bool found_bts = false;
intel_pt_pmu = perf_pmu__find(INTEL_PT_PMU_NAME);
- if (intel_pt_pmu)
- intel_pt_pmu->auxtrace = true;
intel_bts_pmu = perf_pmu__find(INTEL_BTS_PMU_NAME);
- if (intel_bts_pmu)
- intel_bts_pmu->auxtrace = true;
evlist__for_each_entry(evlist, evsel) {
if (intel_pt_pmu && evsel->core.attr.type == intel_pt_pmu->type)
diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index 358340b342431..f73b80dcd8bdc 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -27,10 +27,14 @@ static bool cached_list;
struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu
*pmu __maybe_unused)
{
#ifdef HAVE_AUXTRACE_SUPPORT
- if (!strcmp(pmu->name, INTEL_PT_PMU_NAME))
+ if (!strcmp(pmu->name, INTEL_PT_PMU_NAME)) {
+ pmu->auxtrace = true;
return intel_pt_pmu_default_config(pmu);
- if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME))
+ }
+ if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME)) {
+ pmu->auxtrace = true;
pmu->selectable = true;
+ }
#endif
return NULL;
}
>
>> Something as below (not tested.)
>>
>> +void auxtrace__init(void)
>> +{
>> + struct perf_pmu *intel_pt_pmu;
>> + struct perf_pmu *intel_bts_pmu;
>> + static bool cached;
>> +
>> + if (cached)
>> + return;
>> + intel_pt_pmu = perf_pmu__find(INTEL_PT_PMU_NAME);
>> + if (intel_pt_pmu)
>> + intel_pt_pmu->auxtrace = true;
>> + intel_bts_pmu = perf_pmu__find(INTEL_BTS_PMU_NAME);
>> + if (intel_bts_pmu)
>> + intel_bts_pmu->auxtrace = true;
>> +}
>>
>> bool evsel__is_aux_event(struct evsel *evsel)
>> {
>> struct perf_pmu *pmu = evsel__find_pmu(evsel);
>> + auxtrace__init();
>> return pmu && pmu->auxtrace;
>> }
>>
>>
>>
>> Thanks,
>> Kan
>>
>>> +
>>> argc = parse_options(argc, argv, record_options, record_usage,
>>> PARSE_OPT_STOP_AT_NON_OPTION);
>>> if (quiet)
>>> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
>>> index 29eb82dff574..49a86aa6ac94 100644
>>> --- a/tools/perf/util/auxtrace.h
>>> +++ b/tools/perf/util/auxtrace.h
>>> @@ -457,6 +457,8 @@ struct addr_filters {
>>>
>>> struct auxtrace_cache;
>>>
>>> +void auxtrace__early_init(void);
>>> +
>>> #ifdef HAVE_AUXTRACE_SUPPORT
>>>
>>> u64 compat_auxtrace_mmap__read_head(struct auxtrace_mmap *mm);
>
Powered by blists - more mailing lists