Messages in this thread | | | Subject | Re: [PATCH v2 09/27] perf parse-events: Create two hybrid hardware events | From | "Jin, Yao" <> | Date | Tue, 16 Mar 2021 11:13:03 +0800 |
| |
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 >> >
| |