[v4,14/25] perf stat: Add default hybrid events
diff mbox series

Message ID 20210416140517.18206-15-yao.jin@linux.intel.com
State New
Headers show
Series
  • perf tool: AlderLake hybrid support series 1
Related show

Commit Message

Jin Yao April 16, 2021, 2:05 p.m. UTC
Previously if '-e' is not specified in perf stat, some software events
and hardware events are added to evlist by default.

Before:

  # ./perf stat -a -- sleep 1

   Performance counter stats for 'system wide':

           24,044.40 msec cpu-clock                 #   23.946 CPUs utilized
                  99      context-switches          #    4.117 /sec
                  24      cpu-migrations            #    0.998 /sec
                   3      page-faults               #    0.125 /sec
           7,000,244      cycles                    #    0.000 GHz
           2,955,024      instructions              #    0.42  insn per cycle
             608,941      branches                  #   25.326 K/sec
              31,991      branch-misses             #    5.25% of all branches

         1.004106859 seconds time elapsed

Among the events, cycles, instructions, branches and branch-misses
are hardware events.

One hybrid platform, two hardware events are created for one
hardware event.

cpu_core/cycles/,
cpu_atom/cycles/,
cpu_core/instructions/,
cpu_atom/instructions/,
cpu_core/branches/,
cpu_atom/branches/,
cpu_core/branch-misses/,
cpu_atom/branch-misses/

These events would be added to evlist on hybrid platform.

Since parse_events() has been supported to create two hardware events
for one event on hybrid platform, so we just use parse_events(evlist,
"cycles,instructions,branches,branch-misses") to create the default
events and add them to evlist.

After:

  # ./perf stat -a -- sleep 1

   Performance counter stats for 'system wide':

           24,048.60 msec task-clock                #   23.947 CPUs utilized
                 438      context-switches          #   18.213 /sec
                  24      cpu-migrations            #    0.998 /sec
                   6      page-faults               #    0.249 /sec
          24,813,157      cpu_core/cycles/          #    1.032 M/sec
           8,072,687      cpu_atom/cycles/          #  335.682 K/sec
          20,731,286      cpu_core/instructions/    #  862.058 K/sec
           3,737,203      cpu_atom/instructions/    #  155.402 K/sec
           2,620,924      cpu_core/branches/        #  108.984 K/sec
             381,186      cpu_atom/branches/        #   15.851 K/sec
              93,248      cpu_core/branch-misses/   #    3.877 K/sec
              36,515      cpu_atom/branch-misses/   #    1.518 K/sec

         1.004235472 seconds time elapsed

We can see two events are created for one hardware event.

One TODO is, the shadow stats looks a bit different, now it's just
'M/sec'.

The perf_stat__update_shadow_stats and perf_stat__print_shadow_stats
need to be improved in future if we want to get the original shadow
stats.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
v4:
 - No change.

 tools/perf/builtin-stat.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Jiri Olsa April 21, 2021, 6:29 p.m. UTC | #1
On Fri, Apr 16, 2021 at 10:05:06PM +0800, Jin Yao wrote:

SNIP

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 1255af4751c2..0351b99d17a7 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1145,6 +1145,13 @@ static int parse_stat_cgroups(const struct option *opt,
>  	return parse_cgroups(opt, str, unset);
>  }
>  
> +static int add_default_hybrid_events(struct evlist *evlist)
> +{
> +	struct parse_events_error err;
> +
> +	return parse_events(evlist, "cycles,instructions,branches,branch-misses", &err);
> +}
> +
>  static struct option stat_options[] = {
>  	OPT_BOOLEAN('T', "transaction", &transaction_run,
>  		    "hardware transaction statistics"),
> @@ -1626,6 +1633,12 @@ static int add_default_attributes(void)
>    { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS	},
>    { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_MISSES		},
>  
> +};
> +	struct perf_event_attr default_sw_attrs[] = {
> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK		},
> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CONTEXT_SWITCHES	},
> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_MIGRATIONS		},
> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_PAGE_FAULTS		},

hum, why not use default_attrs0, it's the same, no?

>  };
>  
>  /*
> @@ -1863,6 +1876,14 @@ static int add_default_attributes(void)
>  	}
>  
>  	if (!evsel_list->core.nr_entries) {
> +		if (perf_pmu__has_hybrid()) {
> +			if (evlist__add_default_attrs(evsel_list,
> +						      default_sw_attrs) < 0) {
> +				return -1;
> +			}
> +			return add_default_hybrid_events(evsel_list);

please do it the same way like when topdown calls parse events,
we don't need to check for cycles, but please check result and
display the error


> +		}
> +
>  		if (target__has_cpu(&target))
>  			default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;

also you still want this change for hybrid pmus as well

thanks,
jirka

>  
> -- 
> 2.17.1
>
Jin Yao April 22, 2021, 2:12 a.m. UTC | #2
Hi Jiri,

On 4/22/2021 2:29 AM, Jiri Olsa wrote:
> On Fri, Apr 16, 2021 at 10:05:06PM +0800, Jin Yao wrote:
> 
> SNIP
> 
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 1255af4751c2..0351b99d17a7 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1145,6 +1145,13 @@ static int parse_stat_cgroups(const struct option *opt,
>>   	return parse_cgroups(opt, str, unset);
>>   }
>>   
>> +static int add_default_hybrid_events(struct evlist *evlist)
>> +{
>> +	struct parse_events_error err;
>> +
>> +	return parse_events(evlist, "cycles,instructions,branches,branch-misses", &err);
>> +}
>> +
>>   static struct option stat_options[] = {
>>   	OPT_BOOLEAN('T', "transaction", &transaction_run,
>>   		    "hardware transaction statistics"),
>> @@ -1626,6 +1633,12 @@ static int add_default_attributes(void)
>>     { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS	},
>>     { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_MISSES		},
>>   
>> +};
>> +	struct perf_event_attr default_sw_attrs[] = {
>> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK		},
>> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CONTEXT_SWITCHES	},
>> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_MIGRATIONS		},
>> +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_PAGE_FAULTS		},
> 
> hum, why not use default_attrs0, it's the same, no?
> 

The default_attrs0 has one more item " {.type = PERF_TYPE_HARDWARE, .config = 
PERF_COUNT_HW_CPU_CYCLES },"

So I have to only pick out the sw attrs and save them to default_sw_attrs.

>>   };
>>   
>>   /*
>> @@ -1863,6 +1876,14 @@ static int add_default_attributes(void)
>>   	}
>>   
>>   	if (!evsel_list->core.nr_entries) {
>> +		if (perf_pmu__has_hybrid()) {
>> +			if (evlist__add_default_attrs(evsel_list,
>> +						      default_sw_attrs) < 0) {
>> +				return -1;
>> +			}
>> +			return add_default_hybrid_events(evsel_list);
> 
> please do it the same way like when topdown calls parse events,
> we don't need to check for cycles, but please check result and
> display the error
> 

Something like this?

err = parse_events(evsel_list, "cycles,instructions,branches,branch-misses", &errinfo);
if (err) {
	fprintf(stderr,...);
	parse_events_print_error(&errinfo, ...);
	return -1;
}

> 
>> +		}
>> +
>>   		if (target__has_cpu(&target))
>>   			default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;
> 
> also you still want this change for hybrid pmus as well
> 

Yes, the default_sw_attr only uses 'PERF_COUNT_SW_TASK_CLOCK', we do need to change it to 
PERF_COUNT_SW_CPU_CLOCK for system wide.

> thanks,
> jirka
>

Thanks
Jin Yao

>>   
>> -- 
>> 2.17.1
>>
>
Jiri Olsa April 22, 2021, 10:25 a.m. UTC | #3
On Thu, Apr 22, 2021 at 10:12:49AM +0800, Jin, Yao wrote:
> Hi Jiri,
> 
> On 4/22/2021 2:29 AM, Jiri Olsa wrote:
> > On Fri, Apr 16, 2021 at 10:05:06PM +0800, Jin Yao wrote:
> > 
> > SNIP
> > 
> > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > index 1255af4751c2..0351b99d17a7 100644
> > > --- a/tools/perf/builtin-stat.c
> > > +++ b/tools/perf/builtin-stat.c
> > > @@ -1145,6 +1145,13 @@ static int parse_stat_cgroups(const struct option *opt,
> > >   	return parse_cgroups(opt, str, unset);
> > >   }
> > > +static int add_default_hybrid_events(struct evlist *evlist)
> > > +{
> > > +	struct parse_events_error err;
> > > +
> > > +	return parse_events(evlist, "cycles,instructions,branches,branch-misses", &err);
> > > +}
> > > +
> > >   static struct option stat_options[] = {
> > >   	OPT_BOOLEAN('T', "transaction", &transaction_run,
> > >   		    "hardware transaction statistics"),
> > > @@ -1626,6 +1633,12 @@ static int add_default_attributes(void)
> > >     { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS	},
> > >     { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_MISSES		},
> > > +};
> > > +	struct perf_event_attr default_sw_attrs[] = {
> > > +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK		},
> > > +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CONTEXT_SWITCHES	},
> > > +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_MIGRATIONS		},
> > > +  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_PAGE_FAULTS		},
> > 
> > hum, why not use default_attrs0, it's the same, no?
> > 
> 
> The default_attrs0 has one more item " {.type = PERF_TYPE_HARDWARE, .config
> = PERF_COUNT_HW_CPU_CYCLES },"
> 
> So I have to only pick out the sw attrs and save them to default_sw_attrs.
> 
> > >   };
> > >   /*
> > > @@ -1863,6 +1876,14 @@ static int add_default_attributes(void)
> > >   	}
> > >   	if (!evsel_list->core.nr_entries) {
> > > +		if (perf_pmu__has_hybrid()) {
> > > +			if (evlist__add_default_attrs(evsel_list,
> > > +						      default_sw_attrs) < 0) {
> > > +				return -1;
> > > +			}
> > > +			return add_default_hybrid_events(evsel_list);
> > 
> > please do it the same way like when topdown calls parse events,
> > we don't need to check for cycles, but please check result and
> > display the error
> > 
> 
> Something like this?
> 
> err = parse_events(evsel_list, "cycles,instructions,branches,branch-misses", &errinfo);
> if (err) {
> 	fprintf(stderr,...);
> 	parse_events_print_error(&errinfo, ...);
> 	return -1;
> }

yes

jirka

Patch
diff mbox series

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 1255af4751c2..0351b99d17a7 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1145,6 +1145,13 @@  static int parse_stat_cgroups(const struct option *opt,
 	return parse_cgroups(opt, str, unset);
 }
 
+static int add_default_hybrid_events(struct evlist *evlist)
+{
+	struct parse_events_error err;
+
+	return parse_events(evlist, "cycles,instructions,branches,branch-misses", &err);
+}
+
 static struct option stat_options[] = {
 	OPT_BOOLEAN('T', "transaction", &transaction_run,
 		    "hardware transaction statistics"),
@@ -1626,6 +1633,12 @@  static int add_default_attributes(void)
   { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS	},
   { .type = PERF_TYPE_HARDWARE, .config = PERF_COUNT_HW_BRANCH_MISSES		},
 
+};
+	struct perf_event_attr default_sw_attrs[] = {
+  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK		},
+  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CONTEXT_SWITCHES	},
+  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_MIGRATIONS		},
+  { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_PAGE_FAULTS		},
 };
 
 /*
@@ -1863,6 +1876,14 @@  static int add_default_attributes(void)
 	}
 
 	if (!evsel_list->core.nr_entries) {
+		if (perf_pmu__has_hybrid()) {
+			if (evlist__add_default_attrs(evsel_list,
+						      default_sw_attrs) < 0) {
+				return -1;
+			}
+			return add_default_hybrid_events(evsel_list);
+		}
+
 		if (target__has_cpu(&target))
 			default_attrs0[0].config = PERF_COUNT_SW_CPU_CLOCK;