On 07/08/2026 17:41, Athira Rajeev wrote: > The powerpc auxtrace dispatch lives entirely in > arch/powerpc/util/auxtrace.c. As new PMUs such as HTM are added, this > file would grow to contain the recording logic for all of them. > > Factor out the VPA-DTL recording initializer into its own file, > arch/powerpc/util/vpa-dtl.c, and reduce auxtrace.c to a thin dispatch > layer. auxtrace_record__init() now detects the PMU by name and calls > the appropriate per-PMU init function: > > - vpa_dtl_recording_init() for VPA-DTL events (unchanged behaviour) > - Further PMU entries will follow in subsequent patches > > This makes room in auxtrace_record__init() for the HTM recording path > added in the next patch without growing a single monolithic file. > > Signed-off-by: Athira Rajeev <[email protected]>
A couple of cosmetic / very minor comments. Nevertheless: Reviewed-by: Adrian Hunter <[email protected]> > --- > Changes in V5: > - Add forward declarations for struct evsel and struct auxtrace_record > in powerpc-vpadtl.h before the vpa_dtl_recording_init() prototype. > Without them, a translation unit that includes the header before the > full definitions are visible may produce implicit-declaration warnings > on strict compilers. > > Changes in V4: > - No changes from V3. > > Changes in V3: > Add #include <linux/zalloc.h> to vpa-dtl.c; without it the compiler > treats zalloc() as implicitly returning int, truncating the upper > 32 bits of the returned pointer on 64-bit PowerPC. > > Changes in V2: > - Renamed the destination file from arch/powerpc/util/vpa-dtl.c (same > name, unchanged) but the subject and commit message are reworded to > clearly state that the goal is to make auxtrace_record__init() a thin > per-PMU dispatcher, not merely to "allow multiple PMUs to use auxtrace". > - Handle failure from memory allocation > - Included stdlib and limits.h > - No functional change to the VPA-DTL path itself. > - Patch is now 1/6 instead of 1/9. > > tools/perf/arch/powerpc/util/Build | 1 + > tools/perf/arch/powerpc/util/auxtrace.c | 84 +++------------------- > tools/perf/arch/powerpc/util/vpa-dtl.c | 96 +++++++++++++++++++++++++ > tools/perf/util/powerpc-vpadtl.h | 3 + > 4 files changed, 108 insertions(+), 76 deletions(-) > create mode 100644 tools/perf/arch/powerpc/util/vpa-dtl.c > > diff --git a/tools/perf/arch/powerpc/util/Build > b/tools/perf/arch/powerpc/util/Build > index ae928050e07a..7819c8f5af2d 100644 > --- a/tools/perf/arch/powerpc/util/Build > +++ b/tools/perf/arch/powerpc/util/Build > @@ -7,3 +7,4 @@ perf-util-y += evsel.o > perf-util-$(CONFIG_LIBDW) += skip-callchain-idx.o > > perf-util-y += auxtrace.o > +perf-util-y += vpa-dtl.o > diff --git a/tools/perf/arch/powerpc/util/auxtrace.c > b/tools/perf/arch/powerpc/util/auxtrace.c > index 4600a1661b4f..e04a0bd61755 100644 > --- a/tools/perf/arch/powerpc/util/auxtrace.c > +++ b/tools/perf/arch/powerpc/util/auxtrace.c > @@ -13,63 +13,12 @@ > #include "../../util/auxtrace.h" > #include "../../util/powerpc-vpadtl.h" > #include "../../util/record.h" > -#include <internal/lib.h> // page_size > - > -#define KiB(x) ((x) * 1024) > - > -static int > -powerpc_vpadtl_recording_options(struct auxtrace_record *ar __maybe_unused, > - struct evlist *evlist __maybe_unused, > - struct record_opts *opts) > -{ > - opts->full_auxtrace = true; > - > - /* > - * Set auxtrace_mmap_pages to minimum > - * two pages > - */ > - if (!opts->auxtrace_mmap_pages) { > - opts->auxtrace_mmap_pages = KiB(128) / page_size; > - if (opts->mmap_pages == UINT_MAX) > - opts->mmap_pages = KiB(256) / page_size; > - } > - > - return 0; > -} > - > -static size_t powerpc_vpadtl_info_priv_size(struct auxtrace_record *itr > __maybe_unused, > - struct evlist *evlist __maybe_unused) > -{ > - return VPADTL_AUXTRACE_PRIV_SIZE; > -} > - > -static int > -powerpc_vpadtl_info_fill(struct auxtrace_record *itr __maybe_unused, > - struct perf_session *session __maybe_unused, > - struct perf_record_auxtrace_info *auxtrace_info, > - size_t priv_size __maybe_unused) > -{ > - auxtrace_info->type = PERF_AUXTRACE_VPA_DTL; > - > - return 0; > -} > - > -static void powerpc_vpadtl_free(struct auxtrace_record *itr) > -{ > - free(itr); > -} > - > -static u64 powerpc_vpadtl_reference(struct auxtrace_record *itr > __maybe_unused) > -{ > - return 0; > -} > > struct auxtrace_record *auxtrace_record__init(struct evlist *evlist, > int *err) > { > - struct auxtrace_record *aux; > struct evsel *pos; > - int found = 0; > + struct evsel *vpa_dtl_evsel = NULL; Ordering local definitions by descending line length is nicer e.g. struct evsel *vpa_dtl_evsel = NULL; struct evsel *pos; > > /* > * Set err value to zero here. Any fail later > @@ -78,33 +27,16 @@ struct auxtrace_record *auxtrace_record__init(struct > evlist *evlist, > *err = 0; > > evlist__for_each_entry(evlist, pos) { > - if (strstarts(pos->name, "vpa_dtl")) { > - found = 1; > + if (pos->name && strstarts(pos->name, "vpa_dtl")) { > pos->needs_auxtrace_mmap = true; > - break; > + /* Remember the first matching VPA DTL event */ > + if (!vpa_dtl_evsel) > + vpa_dtl_evsel = pos; > } > } > > - if (!found) > - return NULL; > - > - /* > - * To obtain the auxtrace buffer file descriptor, the auxtrace event > - * must come first. > - */ > - evlist__to_front(pos->evlist, pos); > - > - aux = zalloc(sizeof(*aux)); > - if (aux == NULL) { > - pr_debug("aux record is NULL\n"); > - *err = -ENOMEM; > - return NULL; > - } > + if (vpa_dtl_evsel) > + return vpa_dtl_recording_init(vpa_dtl_evsel, err); > > - aux->recording_options = powerpc_vpadtl_recording_options; > - aux->info_priv_size = powerpc_vpadtl_info_priv_size; > - aux->info_fill = powerpc_vpadtl_info_fill; > - aux->free = powerpc_vpadtl_free; > - aux->reference = powerpc_vpadtl_reference; > - return aux; > + return NULL; > } > diff --git a/tools/perf/arch/powerpc/util/vpa-dtl.c > b/tools/perf/arch/powerpc/util/vpa-dtl.c > new file mode 100644 > index 000000000000..2609b88f61d8 > --- /dev/null > +++ b/tools/perf/arch/powerpc/util/vpa-dtl.c > @@ -0,0 +1,96 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * VPA DTL AUX tracing support > + */ > + > +#include <linux/kernel.h> > +#include <linux/types.h> > +#include <linux/string.h> > +#include <linux/zalloc.h> > +#include <errno.h> > +#include <stdlib.h> > +#include <limits.h> > +#include "../../util/cpumap.h" Is cpumap.h needed? > +#include "../../util/evsel.h" > +#include "../../util/evlist.h" > +#include "../../util/session.h" > +#include "../../util/util.h" > +#include "../../util/debug.h" > +#include "../../util/auxtrace.h" > +#include "../../util/powerpc-vpadtl.h" > +#include "../../util/record.h" > +#include <internal/lib.h> // page_size > + > +#define KiB(x) ((x) * 1024) > + > +static int > +powerpc_vpadtl_recording_options(struct auxtrace_record *ar __maybe_unused, > + struct evlist *evlist __maybe_unused, > + struct record_opts *opts) > +{ > + opts->full_auxtrace = true; > + > + /* > + * Set auxtrace_mmap_pages to minimum > + * two pages > + */ > + if (!opts->auxtrace_mmap_pages) { > + opts->auxtrace_mmap_pages = KiB(128) / page_size; > + if (opts->mmap_pages == UINT_MAX) > + opts->mmap_pages = KiB(256) / page_size; > + } > + > + return 0; > +} > + > +static size_t powerpc_vpadtl_info_priv_size(struct auxtrace_record *itr > __maybe_unused, > + struct evlist *evlist __maybe_unused) > +{ > + return VPADTL_AUXTRACE_PRIV_SIZE; > +} > + > +static int > +powerpc_vpadtl_info_fill(struct auxtrace_record *itr __maybe_unused, > + struct perf_session *session __maybe_unused, > + struct perf_record_auxtrace_info *auxtrace_info, > + size_t priv_size __maybe_unused) > +{ > + auxtrace_info->type = PERF_AUXTRACE_VPA_DTL; > + > + return 0; > +} > + > +static void powerpc_vpadtl_free(struct auxtrace_record *itr) > +{ > + free(itr); > +} > + > +static u64 powerpc_vpadtl_reference(struct auxtrace_record *itr > __maybe_unused) > +{ > + return 0; > +} > + > +struct auxtrace_record *vpa_dtl_recording_init(struct evsel *pos, int *err) > +{ > + struct auxtrace_record *aux; > + > + /* > + * To obtain the auxtrace buffer file descriptor, the auxtrace event > + * must come first. > + */ > + evlist__to_front(pos->evlist, pos); > + > + aux = zalloc(sizeof(*aux)); > + if (aux == NULL) { > + pr_debug("aux record allocation failed (-ENOMEM)\n"); > + *err = -ENOMEM; > + return NULL; > + } > + > + aux->recording_options = powerpc_vpadtl_recording_options; > + aux->info_priv_size = powerpc_vpadtl_info_priv_size; > + aux->info_fill = powerpc_vpadtl_info_fill; > + aux->free = powerpc_vpadtl_free; > + aux->reference = powerpc_vpadtl_reference; > + return aux; > +} > diff --git a/tools/perf/util/powerpc-vpadtl.h > b/tools/perf/util/powerpc-vpadtl.h > index ca809660b9bb..68a780c63204 100644 > --- a/tools/perf/util/powerpc-vpadtl.h > +++ b/tools/perf/util/powerpc-vpadtl.h > @@ -20,4 +20,7 @@ struct perf_pmu; > int powerpc_vpadtl_process_auxtrace_info(union perf_event *event, > struct perf_session *session); > > +struct evsel; > +struct auxtrace_record; > +struct auxtrace_record *vpa_dtl_recording_init(struct evsel *pos, int *err); > #endif
