[PATCH v2 03/10] perf record: Early auxtrace initialization before event parsing

Ian Rogers posted 10 patches 3 years, 1 month ago
There is a newer version of this series
[PATCH v2 03/10] perf record: Early auxtrace initialization before event parsing
Posted by Ian Rogers 3 years, 1 month ago
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@google.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();
+
 	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);
-- 
2.40.0.rc0.216.gc4246ad0f0-goog
Re: [PATCH v2 03/10] perf record: Early auxtrace initialization before event parsing
Posted by Liang, Kan 3 years, 1 month ago

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@google.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.
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);
Re: [PATCH v2 03/10] perf record: Early auxtrace initialization before event parsing
Posted by Adrian Hunter 3 years, 1 month ago
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@google.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.

> 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);
Re: [PATCH v2 03/10] perf record: Early auxtrace initialization before event parsing
Posted by Adrian Hunter 3 years, 1 month ago
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@google.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);
>
Re: [PATCH v2 03/10] perf record: Early auxtrace initialization before event parsing
Posted by Liang, Kan 3 years, 1 month ago

On 2023-03-06 4:31 a.m., Adrian Hunter wrote:
> 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@google.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:

Yes, I think it should be a better place to initialize them.

Thanks,
Kan
> 
> 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);
>>
>