diff mbox series

[v3,03/11] perf record: Early auxtrace initialization before event parsing

Message ID 20230308081731.1887278-4-irogers@google.com
State New
Headers
Series Better fixes for grouping of events |

Commit Message

Ian Rogers March 8, 2023, 8:17 a.m. UTC 
  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(-)

Comments

Liang, Kan March 8, 2023, 8:32 p.m. UTC | #1 
On 2023-03-08 3:17 a.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)
> +{
> +}
> +

Does the method which Adrian suggested work for you?

https://lore.kernel.org/lkml/9788f0f1-087f-7f0b-048a-0146afe1f632@intel.com/

With that, we don't need to introduce another weak function.

Thanks,
Kan


>  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);
Ian Rogers March 8, 2023, 9:22 p.m. UTC | #2 
On Wed, Mar 8, 2023 at 12:32 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2023-03-08 3:17 a.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)
> > +{
> > +}
> > +
>
> Does the method which Adrian suggested work for you?
>
> https://lore.kernel.org/lkml/9788f0f1-087f-7f0b-048a-0146afe1f632@intel.com/
>
> With that, we don't need to introduce another weak function.
>
> Thanks,
> Kan

Thanks Kan,

that works and I'm not sure how I missed Adrian's e-mail. I'll put it into v4.

Ian

>
> >  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);
diff mbox series

Patch

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);