Re: [PATCH v2 09/27] perf parse-events: Create two hybrid hardware events

[Jin, Yao]

Hi Jiri,

On 3/16/2021 7:05 AM, Jiri Olsa wrote:
On Thu, Mar 11, 2021 at 03:07:24PM +0800, Jin Yao wrote:
For hardware events, they have pre-defined configs. The kernel
needs to know where the event comes from (e.g. from cpu_core pmu
or from cpu_atom pmu). But the perf type 'PERF_TYPE_HARDWARE'
can't carry pmu information.
So the kernel introduces a new type 'PERF_TYPE_HARDWARE_PMU'.
The new attr.config layout for PERF_TYPE_HARDWARE_PMU is:

0xDD000000AA
AA: original hardware event ID
DD: PMU type ID

PMU type ID is retrieved from sysfs. For example,

   cat /sys/devices/cpu_atom/type
   10

   cat /sys/devices/cpu_core/type
   4

When enabling a hybrid hardware event without specified pmu, such as,
'perf stat -e cycles -a', two events are created automatically. One
is for atom, the other is for core.

ok I think I understand the need for this (and the following) patch
the perf_hw_id counters could be global, so when you specify only
event like:

    -e cycles

you want all the cycles, which on hybrid system means cycles from
more than one pmus


Yes, on hybrid system it means the cycles from two pmus. One cycle is from cpu_core pmu, another 
cycles is from cpu_atom pmu.

SNIP

@@ -1416,6 +1475,8 @@ int parse_events_add_numeric(struct parse_events_state 
*parse_state,
  {
        struct perf_event_attr attr;
        LIST_HEAD(config_terms);
+       bool hybrid;
+       int ret;
  
  	memset(&attr, 0, sizeof(attr));
        attr.type = type;
@@ -1430,6 +1491,18 @@ int parse_events_add_numeric(struct parse_events_state 
*parse_state,
                        return -ENOMEM;
        }
  
+	/*
+        * Skip the software dummy event.
+        */
+       if (type != PERF_TYPE_SOFTWARE) {
+               if (!perf_pmu__hybrid_exist())
+                       perf_pmu__scan(NULL);

this could be checked in the following add_hybrid_numeric call


Yes, that should be OK. I will move the check in the next version.

+
+               ret = add_hybrid_numeric(parse_state, list, &attr, &hybrid);
+               if (hybrid)
+                       return ret;
+       }

could we add this to separate object.. hybrid.c or maybe parse-events-hybrid.c,

there's already global __add_event wrapper - parse_events__add_event


jirka


Use a new parse-events-hybrid.c, hmm, well that's OK.

Thanks
Jin Yao

+
        return add_event(list, &parse_state->idx, &attr,
                         get_config_name(head_config), &config_terms);
  }
--
2.17.1