| Message ID | 20240823113306.2310957-5-leo.yan@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf auxtrace: Support multiple AUX events | expand |
On 23/08/24 14:33, Leo Yan wrote: > A prerequisite for multiple AUX events is that the AUX events cannot > overlap CPU maps. The reason is that every CPU has only one AUX trace > buffer and maps it to an unique buffer index for CPU and system tracing > mode. > > To prevent the case of CPU maps overlapping occurring within multiple > AUX events, the auxtrace_record__validate_events() function is > introduced. It iterates through all AUX events and returns failure if > it detects CPU maps overlapping. > > Signed-off-by: Leo Yan <leo.yan@arm.com> > --- > tools/perf/builtin-record.c | 4 +++ > tools/perf/util/auxtrace.c | 64 +++++++++++++++++++++++++++++++++++++ > tools/perf/util/auxtrace.h | 7 ++++ > 3 files changed, 75 insertions(+) > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index adbaf80b398c..2c618efba97d 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -862,6 +862,10 @@ static int record__auxtrace_init(struct record *rec) > > auxtrace_regroup_aux_output(rec->evlist); > > + err = auxtrace_validate_events(rec->evlist); > + if (err) > + return err; > + > return auxtrace_parse_filters(rec->evlist); > } > > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c > index ca8682966fae..87e4f21b6edf 100644 > --- a/tools/perf/util/auxtrace.c > +++ b/tools/perf/util/auxtrace.c > @@ -2828,6 +2828,70 @@ int auxtrace_parse_filters(struct evlist *evlist) > return 0; > } > > +int auxtrace_validate_events(struct evlist *evlist) 'auxtrace_validate_aux_events' would better indicate that it is looking only at AUX area events. > +{ > + struct evsel *evsel; > + struct perf_cpu_map *cpu_map = NULL; > + struct perf_cpu_map *cpu_map_intersect = NULL; > + struct perf_cpu_map *cpu_map_merged = NULL; > + int ret = 0; > + > + if (!evlist) > + return 0; Elsewhere we assume it is not NULL, might as well here too. > + > + /* > + * Currently the tool only supports multiple AUX events without > + * overlapping CPU maps and every CPU has its unique AUX buffer > + * for CPU or system mode tracing. > + * > + * Returns failure if detects CPU maps overlapping. > + */ > + evlist__for_each_entry(evlist, evsel) { > + if (!evsel__is_aux_event(evsel)) > + continue; > + > + if (perf_cpu_map__is_empty(evsel->pmu->cpus)) > + continue; Unless perf_cpu_map__intersect() is broken, the empty check should not be needed. Shouldn't we be looking at evsel->cpus ? Possibly need to consider the perf_cpu_map__has_any_cpu() case? e.g. if (cpu_map && (perf_cpu_map__has_any_cpu(evsel->cpus) || perf_cpu_map__has_any_cpu(cpu_map)) { ret = -EINVAL; break; } > + > + cpu_map_intersect = perf_cpu_map__intersect(cpu_map, evsel->pmu->cpus); > + if (cpu_map_intersect) { > + perf_cpu_map__put(cpu_map_intersect); > + pr_err("Doesn't support AUX events with overlapping CPU masks\n"); > + ret = -EINVAL; > + break; > + } > + perf_cpu_map__put(cpu_map_intersect); Maybe add a helper: static bool perf_cpu_map__do_maps_intersect(struct perf_cpu_map *a, struct perf_cpu_map *b) { struct perf_cpu_map *intersection = perf_cpu_map__intersect(a, b); bool ret = !perf_cpu_map__is_empty(intersection); perf_cpu_map__put(intersection); return ret; } > + > + cpu_map_merged = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); > + if (!cpu_map_merged) { > + ret = -ENOMEM; > + break; > + } > + > + /* Update the CPU maps after merging */ > + perf_cpu_map__put(cpu_map); > + cpu_map = cpu_map_merged; perf_cpu_map__merge() is a bit tricky - see its comments. This should probably all just be: cpu_map = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); > + } > + > + if (!ret) > + goto out; Could we put the error path last i.e. perf_cpu_map__put(cpu_map); if (ret) goto out_err; return 0; out_err: > + > + /* If fails, dump CPU maps for debugging */ > + evlist__for_each_entry(evlist, evsel) { > + char buf[200]; > + > + if (!evsel__is_aux_event(evsel)) > + continue; > + > + cpu_map__snprint(evsel->pmu->cpus, buf, sizeof(buf)); > + pr_debug("AUX event [%s]'s cpu map is: %s\n", evsel->pmu->name, buf); Could probably use cpu_map__fprintf(pmu->cpus, debug_file()) and not need buf. > + } > + > +out: > + perf_cpu_map__put(cpu_map); > + return ret; > +} > + > int auxtrace__process_event(struct perf_session *session, union perf_event *event, > struct perf_sample *sample, const struct perf_tool *tool) > { > diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h > index a1895a4f530b..67a74ad0c383 100644 > --- a/tools/perf/util/auxtrace.h > +++ b/tools/perf/util/auxtrace.h > @@ -636,6 +636,7 @@ void addr_filters__exit(struct addr_filters *filts); > int addr_filters__parse_bare_filter(struct addr_filters *filts, > const char *filter); > int auxtrace_parse_filters(struct evlist *evlist); > +int auxtrace_validate_events(struct evlist *evlist); > > int auxtrace__process_event(struct perf_session *session, union perf_event *event, > struct perf_sample *sample, const struct perf_tool *tool); > @@ -875,6 +876,12 @@ int auxtrace_parse_filters(struct evlist *evlist __maybe_unused) > return 0; > } > > +static inline > +int auxtrace_validate_events(struct evlist *evlist __maybe_unused) > +{ > + return 0; > +} > + > int auxtrace_mmap__mmap(struct auxtrace_mmap *mm, > struct auxtrace_mmap_params *mp, > void *userpg, int fd);
On 9/3/2024 4:26 PM, Adrian Hunter wrote: > On 23/08/24 14:33, Leo Yan wrote: >> A prerequisite for multiple AUX events is that the AUX events cannot >> overlap CPU maps. The reason is that every CPU has only one AUX trace >> buffer and maps it to an unique buffer index for CPU and system tracing >> mode. >> >> To prevent the case of CPU maps overlapping occurring within multiple >> AUX events, the auxtrace_record__validate_events() function is >> introduced. It iterates through all AUX events and returns failure if >> it detects CPU maps overlapping. >> >> Signed-off-by: Leo Yan <leo.yan@arm.com> >> --- >> tools/perf/builtin-record.c | 4 +++ >> tools/perf/util/auxtrace.c | 64 +++++++++++++++++++++++++++++++++++++ >> tools/perf/util/auxtrace.h | 7 ++++ >> 3 files changed, 75 insertions(+) >> >> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >> index adbaf80b398c..2c618efba97d 100644 >> --- a/tools/perf/builtin-record.c >> +++ b/tools/perf/builtin-record.c >> @@ -862,6 +862,10 @@ static int record__auxtrace_init(struct record *rec) >> >> auxtrace_regroup_aux_output(rec->evlist); >> >> + err = auxtrace_validate_events(rec->evlist); >> + if (err) >> + return err; >> + >> return auxtrace_parse_filters(rec->evlist); >> } >> >> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c >> index ca8682966fae..87e4f21b6edf 100644 >> --- a/tools/perf/util/auxtrace.c >> +++ b/tools/perf/util/auxtrace.c >> @@ -2828,6 +2828,70 @@ int auxtrace_parse_filters(struct evlist *evlist) >> return 0; >> } >> >> +int auxtrace_validate_events(struct evlist *evlist) > > 'auxtrace_validate_aux_events' would better indicate that it is > looking only at AUX area events. Will fix. >> +{ >> + struct evsel *evsel; >> + struct perf_cpu_map *cpu_map = NULL; >> + struct perf_cpu_map *cpu_map_intersect = NULL; >> + struct perf_cpu_map *cpu_map_merged = NULL; >> + int ret = 0; >> + >> + if (!evlist) >> + return 0; > > Elsewhere we assume it is not NULL, might as well here too. Sure, will drop this checking. >> + >> + /* >> + * Currently the tool only supports multiple AUX events without >> + * overlapping CPU maps and every CPU has its unique AUX buffer >> + * for CPU or system mode tracing. >> + * >> + * Returns failure if detects CPU maps overlapping. >> + */ >> + evlist__for_each_entry(evlist, evsel) { >> + if (!evsel__is_aux_event(evsel)) >> + continue; >> + >> + if (perf_cpu_map__is_empty(evsel->pmu->cpus)) >> + continue; > > Unless perf_cpu_map__intersect() is broken, the empty check > should not be needed. Perf's CPU map implementation is tricky. IIRC, if without this checking, it will break the tool. In below code, we invokes perf_cpu_map__merge() for merging maps. It does _not_ always allocate a new map for merging. Based on testing, it only allocates a new map if two passed map pointers are not NULL. If a passed CPU map pointer is NULL, then it will directly return the another map's pointer. This leads the difficulty for releasing the merged map. If the returned merged map is a new allocated one, it is safe us to release it. Otherwise, we might release a CPU map unexpectedly - though it is returned by perf_cpu_map__merge(), but the CPU map comes from a PMU and should not be released. Anyway, I will remove the empty check and see if fix the perf CPU map issue. > Shouldn't we be looking at evsel->cpus ? I don't find the field `evsel->cpus`, I assume you are referring to evsel__cpus(evsel)? If so, I will change to use the CPU map from evsel. > Possibly need to consider the perf_cpu_map__has_any_cpu() case? > e.g. > if (cpu_map && (perf_cpu_map__has_any_cpu(evsel->cpus) || > perf_cpu_map__has_any_cpu(cpu_map)) { > ret = -EINVAL; > break; > } Will add. >> + >> + cpu_map_intersect = perf_cpu_map__intersect(cpu_map, evsel->pmu->cpus); >> + if (cpu_map_intersect) { >> + perf_cpu_map__put(cpu_map_intersect); >> + pr_err("Doesn't support AUX events with overlapping CPU masks\n"); >> + ret = -EINVAL; >> + break; >> + } >> + perf_cpu_map__put(cpu_map_intersect); > > Maybe add a helper: > > static bool perf_cpu_map__do_maps_intersect(struct perf_cpu_map *a, struct perf_cpu_map *b) > { > struct perf_cpu_map *intersection = perf_cpu_map__intersect(a, b); > bool ret = !perf_cpu_map__is_empty(intersection); > > perf_cpu_map__put(intersection); > > return ret; > } Will do. >> + >> + cpu_map_merged = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); >> + if (!cpu_map_merged) { >> + ret = -ENOMEM; >> + break; >> + } >> + >> + /* Update the CPU maps after merging */ >> + perf_cpu_map__put(cpu_map); >> + cpu_map = cpu_map_merged; > > perf_cpu_map__merge() is a bit tricky - see its comments. This > should probably all just be: > > cpu_map = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); This might lead to memory leak for the 'old' cpu_map after merging. We cannot assume the `cpu_map` variable is extended from its old value, a new CPU map is allocated during the merging. This is why the patch always release the old cpu_map (perf_cpu_map__put(cpu_map)) and then assign the new merged CPU map. >> + } >> + >> + if (!ret) >> + goto out; > > Could we put the error path last i.e. > > perf_cpu_map__put(cpu_map); > > if (ret) > goto out_err; > > return 0; > > out_err: Makes sense. Will fix. >> + >> + /* If fails, dump CPU maps for debugging */ >> + evlist__for_each_entry(evlist, evsel) { >> + char buf[200]; >> + >> + if (!evsel__is_aux_event(evsel)) >> + continue; >> + >> + cpu_map__snprint(evsel->pmu->cpus, buf, sizeof(buf)); >> + pr_debug("AUX event [%s]'s cpu map is: %s\n", evsel->pmu->name, buf); > > Could probably use cpu_map__fprintf(pmu->cpus, debug_file()) and > not need buf. Will do. Thanks for suggestions! Leo >> + } >> + >> +out: >> + perf_cpu_map__put(cpu_map); >> + return ret; >> +} >> + >> int auxtrace__process_event(struct perf_session *session, union perf_event *event, >> struct perf_sample *sample, const struct perf_tool *tool) >> { >> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h >> index a1895a4f530b..67a74ad0c383 100644 >> --- a/tools/perf/util/auxtrace.h >> +++ b/tools/perf/util/auxtrace.h >> @@ -636,6 +636,7 @@ void addr_filters__exit(struct addr_filters *filts); >> int addr_filters__parse_bare_filter(struct addr_filters *filts, >> const char *filter); >> int auxtrace_parse_filters(struct evlist *evlist); >> +int auxtrace_validate_events(struct evlist *evlist); >> >> int auxtrace__process_event(struct perf_session *session, union perf_event *event, >> struct perf_sample *sample, const struct perf_tool *tool); >> @@ -875,6 +876,12 @@ int auxtrace_parse_filters(struct evlist *evlist __maybe_unused) >> return 0; >> } >> >> +static inline >> +int auxtrace_validate_events(struct evlist *evlist __maybe_unused) >> +{ >> + return 0; >> +} >> + >> int auxtrace_mmap__mmap(struct auxtrace_mmap *mm, >> struct auxtrace_mmap_params *mp, >> void *userpg, int fd); >
On 5/09/24 00:13, Leo Yan wrote: > On 9/3/2024 4:26 PM, Adrian Hunter wrote: >> On 23/08/24 14:33, Leo Yan wrote: >>> A prerequisite for multiple AUX events is that the AUX events cannot >>> overlap CPU maps. The reason is that every CPU has only one AUX trace >>> buffer and maps it to an unique buffer index for CPU and system tracing >>> mode. >>> >>> To prevent the case of CPU maps overlapping occurring within multiple >>> AUX events, the auxtrace_record__validate_events() function is >>> introduced. It iterates through all AUX events and returns failure if >>> it detects CPU maps overlapping. >>> >>> Signed-off-by: Leo Yan <leo.yan@arm.com> >>> --- >>> tools/perf/builtin-record.c | 4 +++ >>> tools/perf/util/auxtrace.c | 64 +++++++++++++++++++++++++++++++++++++ >>> tools/perf/util/auxtrace.h | 7 ++++ >>> 3 files changed, 75 insertions(+) >>> >>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c >>> index adbaf80b398c..2c618efba97d 100644 >>> --- a/tools/perf/builtin-record.c >>> +++ b/tools/perf/builtin-record.c >>> @@ -862,6 +862,10 @@ static int record__auxtrace_init(struct record *rec) >>> >>> auxtrace_regroup_aux_output(rec->evlist); >>> >>> + err = auxtrace_validate_events(rec->evlist); >>> + if (err) >>> + return err; >>> + >>> return auxtrace_parse_filters(rec->evlist); >>> } >>> >>> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c >>> index ca8682966fae..87e4f21b6edf 100644 >>> --- a/tools/perf/util/auxtrace.c >>> +++ b/tools/perf/util/auxtrace.c >>> @@ -2828,6 +2828,70 @@ int auxtrace_parse_filters(struct evlist *evlist) >>> return 0; >>> } >>> >>> +int auxtrace_validate_events(struct evlist *evlist) >> >> 'auxtrace_validate_aux_events' would better indicate that it is >> looking only at AUX area events. > > Will fix. > >>> +{ >>> + struct evsel *evsel; >>> + struct perf_cpu_map *cpu_map = NULL; >>> + struct perf_cpu_map *cpu_map_intersect = NULL; >>> + struct perf_cpu_map *cpu_map_merged = NULL; >>> + int ret = 0; >>> + >>> + if (!evlist) >>> + return 0; >> >> Elsewhere we assume it is not NULL, might as well here too. > > Sure, will drop this checking. > >>> + >>> + /* >>> + * Currently the tool only supports multiple AUX events without >>> + * overlapping CPU maps and every CPU has its unique AUX buffer >>> + * for CPU or system mode tracing. >>> + * >>> + * Returns failure if detects CPU maps overlapping. >>> + */ >>> + evlist__for_each_entry(evlist, evsel) { >>> + if (!evsel__is_aux_event(evsel)) >>> + continue; >>> + >>> + if (perf_cpu_map__is_empty(evsel->pmu->cpus)) >>> + continue; >> >> Unless perf_cpu_map__intersect() is broken, the empty check >> should not be needed. > > Perf's CPU map implementation is tricky. IIRC, if without this checking, it > will break the tool. > > In below code, we invokes perf_cpu_map__merge() for merging maps. It does > _not_ always allocate a new map for merging. Based on testing, it only > allocates a new map if two passed map pointers are not NULL. If a passed CPU > map pointer is NULL, then it will directly return the another map's pointer. > > This leads the difficulty for releasing the merged map. If the returned merged > map is a new allocated one, it is safe us to release it. Otherwise, we might > release a CPU map unexpectedly - though it is returned by > perf_cpu_map__merge(), but the CPU map comes from a PMU and should not be > released. If it returns a different map, it adjusts the reference counts accordingly. perf_cpu_map__merge() is still a problem though. See below. > > Anyway, I will remove the empty check and see if fix the perf CPU map issue. > >> Shouldn't we be looking at evsel->cpus ? > > I don't find the field `evsel->cpus`, I assume you are referring to > evsel__cpus(evsel)? If so, I will change to use the CPU map from evsel. > >> Possibly need to consider the perf_cpu_map__has_any_cpu() case? >> e.g. >> if (cpu_map && (perf_cpu_map__has_any_cpu(evsel->cpus) || >> perf_cpu_map__has_any_cpu(cpu_map)) { >> ret = -EINVAL; >> break; >> } > > Will add. > >>> + >>> + cpu_map_intersect = perf_cpu_map__intersect(cpu_map, evsel->pmu->cpus); >>> + if (cpu_map_intersect) { >>> + perf_cpu_map__put(cpu_map_intersect); >>> + pr_err("Doesn't support AUX events with overlapping CPU masks\n"); >>> + ret = -EINVAL; >>> + break; >>> + } >>> + perf_cpu_map__put(cpu_map_intersect); >> >> Maybe add a helper: >> >> static bool perf_cpu_map__do_maps_intersect(struct perf_cpu_map *a, struct perf_cpu_map *b) >> { >> struct perf_cpu_map *intersection = perf_cpu_map__intersect(a, b); >> bool ret = !perf_cpu_map__is_empty(intersection); >> >> perf_cpu_map__put(intersection); >> >> return ret; >> } > > Will do. > >>> + >>> + cpu_map_merged = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); >>> + if (!cpu_map_merged) { >>> + ret = -ENOMEM; >>> + break; >>> + } >>> + >>> + /* Update the CPU maps after merging */ >>> + perf_cpu_map__put(cpu_map); >>> + cpu_map = cpu_map_merged; >> >> perf_cpu_map__merge() is a bit tricky - see its comments. This >> should probably all just be: >> >> cpu_map = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); > > This might lead to memory leak for the 'old' cpu_map after merging. > > We cannot assume the `cpu_map` variable is extended from its old value, a new > CPU map is allocated during the merging. This is why the patch always release > the old cpu_map (perf_cpu_map__put(cpu_map)) and then assign the new merged > CPU map. I agree, perf_cpu_map__merge() is a bit broken. Maybe add another helper like: static int perf_cpu_map__merge_in(struct perf_cpu_map **orig, struct perf_cpu_map *other) { struct perf_cpu_map *merged; /* Avoid being unable to tell if perf_cpu_map__merge() failed */ if (perf_cpu_map__is_empty(*orig) && perf_cpu_map__is_empty(other)) return 0; merged = perf_cpu_map__merge(*orig, other); if (!merged) return -ENOMEM; *orig = merged; return 0; } Then it can be: ret = perf_cpu_map__merge_in(&cpu_map, evsel__cpus(evsel)); if (ret) break; > >>> + } >>> + >>> + if (!ret) >>> + goto out; >> >> Could we put the error path last i.e. >> >> perf_cpu_map__put(cpu_map); >> >> if (ret) >> goto out_err; >> >> return 0; >> >> out_err: > > Makes sense. Will fix. > >>> + >>> + /* If fails, dump CPU maps for debugging */ >>> + evlist__for_each_entry(evlist, evsel) { >>> + char buf[200]; >>> + >>> + if (!evsel__is_aux_event(evsel)) >>> + continue; >>> + >>> + cpu_map__snprint(evsel->pmu->cpus, buf, sizeof(buf)); >>> + pr_debug("AUX event [%s]'s cpu map is: %s\n", evsel->pmu->name, buf); >> >> Could probably use cpu_map__fprintf(pmu->cpus, debug_file()) and >> not need buf. > > Will do. > > Thanks for suggestions! > > Leo > >>> + } >>> + >>> +out: >>> + perf_cpu_map__put(cpu_map); >>> + return ret; >>> +} >>> + >>> int auxtrace__process_event(struct perf_session *session, union perf_event *event, >>> struct perf_sample *sample, const struct perf_tool *tool) >>> { >>> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h >>> index a1895a4f530b..67a74ad0c383 100644 >>> --- a/tools/perf/util/auxtrace.h >>> +++ b/tools/perf/util/auxtrace.h >>> @@ -636,6 +636,7 @@ void addr_filters__exit(struct addr_filters *filts); >>> int addr_filters__parse_bare_filter(struct addr_filters *filts, >>> const char *filter); >>> int auxtrace_parse_filters(struct evlist *evlist); >>> +int auxtrace_validate_events(struct evlist *evlist); >>> >>> int auxtrace__process_event(struct perf_session *session, union perf_event *event, >>> struct perf_sample *sample, const struct perf_tool *tool); >>> @@ -875,6 +876,12 @@ int auxtrace_parse_filters(struct evlist *evlist __maybe_unused) >>> return 0; >>> } >>> >>> +static inline >>> +int auxtrace_validate_events(struct evlist *evlist __maybe_unused) >>> +{ >>> + return 0; >>> +} >>> + >>> int auxtrace_mmap__mmap(struct auxtrace_mmap *mm, >>> struct auxtrace_mmap_params *mp, >>> void *userpg, int fd); >>
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index adbaf80b398c..2c618efba97d 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -862,6 +862,10 @@ static int record__auxtrace_init(struct record *rec) auxtrace_regroup_aux_output(rec->evlist); + err = auxtrace_validate_events(rec->evlist); + if (err) + return err; + return auxtrace_parse_filters(rec->evlist); } diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index ca8682966fae..87e4f21b6edf 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -2828,6 +2828,70 @@ int auxtrace_parse_filters(struct evlist *evlist) return 0; } +int auxtrace_validate_events(struct evlist *evlist) +{ + struct evsel *evsel; + struct perf_cpu_map *cpu_map = NULL; + struct perf_cpu_map *cpu_map_intersect = NULL; + struct perf_cpu_map *cpu_map_merged = NULL; + int ret = 0; + + if (!evlist) + return 0; + + /* + * Currently the tool only supports multiple AUX events without + * overlapping CPU maps and every CPU has its unique AUX buffer + * for CPU or system mode tracing. + * + * Returns failure if detects CPU maps overlapping. + */ + evlist__for_each_entry(evlist, evsel) { + if (!evsel__is_aux_event(evsel)) + continue; + + if (perf_cpu_map__is_empty(evsel->pmu->cpus)) + continue; + + cpu_map_intersect = perf_cpu_map__intersect(cpu_map, evsel->pmu->cpus); + if (cpu_map_intersect) { + perf_cpu_map__put(cpu_map_intersect); + pr_err("Doesn't support AUX events with overlapping CPU masks\n"); + ret = -EINVAL; + break; + } + perf_cpu_map__put(cpu_map_intersect); + + cpu_map_merged = perf_cpu_map__merge(cpu_map, evsel->pmu->cpus); + if (!cpu_map_merged) { + ret = -ENOMEM; + break; + } + + /* Update the CPU maps after merging */ + perf_cpu_map__put(cpu_map); + cpu_map = cpu_map_merged; + } + + if (!ret) + goto out; + + /* If fails, dump CPU maps for debugging */ + evlist__for_each_entry(evlist, evsel) { + char buf[200]; + + if (!evsel__is_aux_event(evsel)) + continue; + + cpu_map__snprint(evsel->pmu->cpus, buf, sizeof(buf)); + pr_debug("AUX event [%s]'s cpu map is: %s\n", evsel->pmu->name, buf); + } + +out: + perf_cpu_map__put(cpu_map); + return ret; +} + int auxtrace__process_event(struct perf_session *session, union perf_event *event, struct perf_sample *sample, const struct perf_tool *tool) { diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index a1895a4f530b..67a74ad0c383 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -636,6 +636,7 @@ void addr_filters__exit(struct addr_filters *filts); int addr_filters__parse_bare_filter(struct addr_filters *filts, const char *filter); int auxtrace_parse_filters(struct evlist *evlist); +int auxtrace_validate_events(struct evlist *evlist); int auxtrace__process_event(struct perf_session *session, union perf_event *event, struct perf_sample *sample, const struct perf_tool *tool); @@ -875,6 +876,12 @@ int auxtrace_parse_filters(struct evlist *evlist __maybe_unused) return 0; } +static inline +int auxtrace_validate_events(struct evlist *evlist __maybe_unused) +{ + return 0; +} + int auxtrace_mmap__mmap(struct auxtrace_mmap *mm, struct auxtrace_mmap_params *mp, void *userpg, int fd);
A prerequisite for multiple AUX events is that the AUX events cannot overlap CPU maps. The reason is that every CPU has only one AUX trace buffer and maps it to an unique buffer index for CPU and system tracing mode. To prevent the case of CPU maps overlapping occurring within multiple AUX events, the auxtrace_record__validate_events() function is introduced. It iterates through all AUX events and returns failure if it detects CPU maps overlapping. Signed-off-by: Leo Yan <leo.yan@arm.com> --- tools/perf/builtin-record.c | 4 +++ tools/perf/util/auxtrace.c | 64 +++++++++++++++++++++++++++++++++++++ tools/perf/util/auxtrace.h | 7 ++++ 3 files changed, 75 insertions(+)